Here is oneliner:
$ gg 'LOG_\(DEBUG\|TRACE\|INFO\|TEST\|WARNING\|ERROR\|FATAL\)([^,]*, [a-zA-Z]' -- :*.cpp :*.h | cut -d: -f1 | sort -u | xargs -r sed -E -i 's#(LOG_[A-Z]*)\(([^,]*), ([A-Za-z][^,)]*)#\1(\2, fmt::runtime(\3)#'
Note, that I tried to do this with coccinelle (tool for semantic
patchin), but it cannot parse C++:
$ cat fmt.cocci
@@
expression log;
expression var;
@@
-LOG_DEBUG(log, var)
+LOG_DEBUG(log, fmt::runtime(var))
I've also tried to use some macros/templates magic to do this implicitly
in logger_useful.h, but I failed to do so, and apparently it is not
possible for now.
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
v2: manual fixes
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
This allows starting and stopping separately each protocol server
without restarting ClickHouse.
This also allows adding or removing `listen_host` entries, which
start and stops servers for all enabled ports.
When stopping a server, the listening socket is immediately closed
(and available for another server).
Protocols with persistent connections try to wait for any currently
running query to finish before closing the connection, but idle
connection are closed quickly (depending on how often the protocol
is polled).
An extra ProfileEvent is added, `MainConfigLoads`, it is
incremented every time the configuration is reloaded. This helps
when trying to assess whether the new configuration was applied.
This is not that important, but because of packets overlaps the client
may not render the fatal error.
You will find TSan report in `details`.
<details>
```
WARNING: ThreadSanitizer: data race (pid=13384)
Write of size 8 at 0x7b24000ad818 by thread T47:
0 DB::WriteBuffer::next() obj-x86_64-linux-gnu/../src/IO/WriteBuffer.h:43:15 (clickhouse-tsan+0x16124d61)
1 DB::TCPHandler::sendProgress() obj-x86_64-linux-gnu/../src/Server/TCPHandler.cpp:1602:10 (clickhouse-tsan+0x16124d61)
2 DB::TCPHandler::runImpl()::$_6::operator()() const obj-x86_64-linux-gnu/../src/Server/TCPHandler.cpp:318:25 (clickhouse-tsan+0x1612cf54)
Previous write of size 8 at 0x7b24000ad818 by thread T229:
0 DB::WriteBuffer::next() obj-x86_64-linux-gnu/../src/IO/WriteBuffer.h:43:15 (clickhouse-tsan+0x1612b793)
1 DB::TCPHandler::sendLogData(DB::Block const&) obj-x86_64-linux-gnu/../src/Server/TCPHandler.cpp:1561:10 (clickhouse-tsan+0x1612b793)
2 DB::TCPHandler::sendLogs() obj-x86_64-linux-gnu/../src/Server/TCPHandler.cpp:1632:9 (clickhouse-tsan+0x161228ab)
3 DB::TCPHandler::runImpl()::$_1::operator()() const obj-x86_64-linux-gnu/../src/Server/TCPHandler.cpp:229:62 (clickhouse-tsan+0x1612c4e1)
```
</details>
Before this patch INSERT via Buffer/Kafka may re-use previously set user
for that connection, while this is not correct, it should reset the
user, and use global context.
Note, before [1] there was a fallback to default user, but that code had
been removed, and now it got back.
[1]: 0159c74f21 ("Secure inter-cluster query execution (with initial_user as current query user) [v3]")
Also note, that context for Buffer table (and others) cannot be changed,
since they don't have any user only profile.
I've tested this patch manually using the following:
create table dist (key Int) engine=Distributed(test_cluster_two_shards_secure, default, data, key);
create table buffer (key Int) engine=Buffer(default, dist, 1, 0, 0, 0, 0, 0, 0);
create table data (key Int) engine=Memory();
# to start the connection with readonly user
$ clickhouse-client --user readonly -q 'select * from dist'
$ clickhouse-client -q 'insert into buffer values (1)'
# before this patch this produces errors like:
# 2021.09.27 23:46:48.384920 [ 19474 ] {} <Error> default.dist.DirectoryMonitor: Code: 164. DB::Exception: Received from 127.0.0.2:9000. DB::Exception: readonly: Cannot execute query in readonly mode. Stack trace:
v2: reset the authentication instead of using default user (as suggested by @vitlibar)
v3: reset Session::user and introduce ClientInfo::resetAuthentication (as suggested by @vitlibar)
v4: reset the session every time in interserver mode (suggested by @vitlibar)
As reported in #27171, the logic that generates the HTTP error
response didn't work in case the configuration hadn't specified a
`http_port`. The new logic checks whether the `http_port` was set in
the first place.
That may lead to uncaught exception [1].
[1]: https://github.com/ClickHouse/ClickHouse/pull/21624#issuecomment-808066747
v0: New method BlockOutputStream::finalize() -- bad (forgot some places
and superfluous)
v2: Flush BlockOutputStream in QueryStatus::releaseQueryStreams()
Safer query termination (report some uncaugh errors to user)
v3: Flush BlockOutputStream from TCPHandler only
Other places uses different executeQuery() implementation
v4: Do not call writeSuffix() twice