Fixed `Unknown identifier (aggregate-function)` exception which appears when a user tries to calculate WINDOW ORDER BY/PARTITION BY expressions over aggregate functions
clang-tidy now also checks code in header files. Because the analyzer
finds tons of issues, activate the check only for directory "base/" (see
file ".clang-tidy"). All other directories, in particular "src/" are
left to future work.
While many findings were fixed, some were not (and suppressed instead).
Reasons for this include: a) the file is 1:1 copypaste of a 3rd-party
lib (e.g. pcg_extras.h) and fixing stuff would make upgrades/fixes more
difficult b) a fix would have broken lots of using code
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>
