While pushign to MVs, there is a low-level code that create
ThreadGroupStatus/ThreadStatus, it is required to gather some metrics
for system.query_views_log.
But, one should not use ThreadGroupStatus of the MainThreadStatus, since
this structure can hold some state, that may not be cleaned, plus this
may be racy, instead it is better to create new ThreadGroupStatus and
attach it instead.
Also this place misses detachQuery(), and because of this it leaks
ThreadGroupStatus::finished_threads_counters_memory. But it is only the
problem pushing to MVs is done w/o query context (i.e. from Kafka/...),
since when it has query context detachQuery() will be called eventually.
Before this patch series, when I've tried the reproducer with
500 MVs attached to Kafka engine (that @den-crane suggested), jemalloc
report looks like this:
$ ../jeprof --text ~/ch/tmp/upstream/clickhouse-binary --base jeprof.44384.0.i0.heap jeprof.44384.167.i167.heap
Using local file /home/azat/ch/tmp/upstream/clickhouse-binary.
Using local file jeprof.44384.167.i167.heap.
Total: 915.6 MB
910.7 99.5% 99.5% 910.7 99.5% Snapshot (inline)
9.5 1.0% 100.5% 9.5 1.0% std::__1::__libcpp_operator_new (inline)
0.5 0.1% 100.6% 0.5 0.1% DB::TasksStatsCounters::create
And with focus to this place:
$ ../jeprof --focus Snapshot --text ~/ch/tmp/upstream/clickhouse-binary --base jeprof.44384.0.i0.heap jeprof.44384.167.i167.heap
Using local file /home/azat/ch/tmp/upstream/clickhouse-binary.
Using local file jeprof.44384.167.i167.heap.
Total: 915.6 MB
910.7 100.0% 100.0% 910.7 100.0% Snapshot (inline)
0.0 0.0% 100.0% 910.7 100.0% DB::QueryPipeline::reset
0.0 0.0% 100.0% 910.7 100.0% DB::StorageKafka::streamToViews
0.0 0.0% 100.0% 910.7 100.0% DB::StorageKafka::threadFunc
0.0 0.0% 100.0% 910.7 100.0% ProfileEvents::Counters::getPartiallyAtomicSnapshot
0.0 0.0% 100.0% 910.7 100.0% ~ThreadStatus
0.0 0.0% 100.0% 910.7 100.0% ~ViewRuntimeData
0.0 0.0% 100.0% 910.7 100.0% ~ViewRuntimeStats (inline)
Actually this report does not looks great (you understand it because I
stripped it), because --text does not that smart, but if you will use
--pdf for the report you will see the stacktrace (will attach pdf to the
pull request).
But after this patch series the process RSS does not goes beyond
~700MiB.
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
This can create leaks, since detachQuery() responsible for cleaning,
i.e. ThreadGroupStatus::finished_threads_counters_memory
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
One should not use MainThreadStatus, since ThreadGroupStatus can hold
some states, and it is better not to play with this, since this may
create leaks.
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
I have to do this, since I have libraries compiled with DWARF5 (i.e.
glibc).
ClickHouse changes:
- use camel_case
- add NOLINT
- avoid using folly:: (use std:: instead)
- avoid using boost:: (use std:: instead)
Refs: 490b287ca3
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
The problem is that the buffer size validated after marks reading in
MergeTreeReaderStream::init(), since it requires to read marks first.
And later it is passed to AsynchronousReadBufferFromFileDescriptor,
which throws LOGICAL_ERROR because buffer_size < alignment.
Fix this my simply disallow such values for max_read_buffer_size (I
thougt about modifying createReadBufferFromFileBase(), but it is not
used for remote reads -- remote_fs_buffer_size).
Fixes: #40669
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
