Fixes for query profiler.#6124
Conversation
|
Now we can get usable results: |
|
In gdb type to not stop on profiling signals. |
|
It still can cause segfault: |
|
|
|
It's also reproduced: in debug build; in build with clang; without jemalloc. |
|
It happens not only inside malloc, also in normal C++ code: |
|
|
Note that the address that libunwind wants to read (and it think that it's the value of #0 0x000000001402be70 in libunwind::LocalAddressSpace::get64 (this=0x14175fc0 libunwind::LocalAddressSpace::sThisAddressSpace, addr=524288) at ../contrib/libunwind/src/AddressSpace.hpp:203 |
|
Probably it will work if we limit DWARF version to 3 for ClickHouse build. |
Not true. |
|
Deadlock is possible if:
|
|
|
|
TODO: timer offset randomization. For example, we may want to set up timer interval of one second but be able to sample some queries with duration less than a second. |
This probably requires |
|
But it's by default. |
|
Backported changes to query profiler to 19.12, because it was in unusable state there. |
| throw; | ||
| } | ||
| #else | ||
| throw Exception("QueryProfiler cannot work with stock libunwind", ErrorCodes::NOT_IMPLEMENTED); |
There was a problem hiding this comment.
Can we make it a compile time error?
There was a problem hiding this comment.
There are alternative build variants for ClickHouse:
- build for Debian repository;
- build for Arcadia source code repository in Yandex;
These builds must work, and we simply disable query profiler in that cases.
There was a problem hiding this comment.
But maybe better to hide all QueryProfiler class altogether.
| throw Exception("QueryProfiler cannot be used without PHDR cache, that is not available for TSan build", ErrorCodes::NOT_IMPLEMENTED); | ||
|
|
||
| /// Too high frequency can introduce infinite busy loop of signal handlers. We will limit maximum frequency (with 1000 signals per second). | ||
| if (period < 1000000) |
There was a problem hiding this comment.
This check should be performed after period_rand is selected. Otherwise we still can get very small periods
There was a problem hiding this comment.
period_rand is the offset. It affects only the first signal. And it is needed for sampling of short queries.

For changelog. Remove if this is non-significant change.
Category (leave one):
Short description (up to few sentences):
Merging #4247