Repository navigation
Conversation
added 18 commits
February 12, 2022 16:05
…reSettings to storages.
…database_has_different_definition".
and use a BackupEntriesBatch for Memort engine to improve performance. A lot of minor corrections.
…not only by name.
vitlibar
pushed a commit
that referenced
this pull request
Jul 7, 2022
This use-after-free can be reproduced with distributed queries.
Also note, that this is not sumMappedArray() and friends (that
previously called sumMap()) but Map combinator.
You will find ASan report in details.
<details>
READ of size 8 at 0x62d00012d218 thread T186 (QueryPipelineEx)
2022.07.03 05:09:40.000234 [ 31956 ] {} <Trace> AsynchronousMetrics: MemoryTracking: was 1.23 GiB, peak 1.23 GiB, will set to 1.25 GiB (RSS), difference: 19.51 MiB
2022.07.03 05:09:41.000137 [ 31956 ] {} <Trace> AsynchronousMetrics: MemoryTracking: was 1.25 GiB, peak 1.25 GiB, will set to 1.26 GiB (RSS), difference: 3.76 MiB
#0 0x1233a0d8 in DB::AggregateFunctionSumData<>::get() const build_docker/../src/AggregateFunctions/AggregateFunctionSum.h:245:16
#1 0x1233a0d8 in DB::AggregateFunctionSum<>::insertResultInto(char*, DB::IColumn&, DB::Arena*) const build_docker/../src/AggregateFunctions/AggregateFunctionSum.h:536:70
#2 0x1470f910 in DB::AggregateFunctionMap<char8_t>::insertResultInto() const build_docker/../src/AggregateFunctions/AggregateFunctionMap.h:236:26
#3 0x147110ce in DB::IAggregateFunctionHelper<>::insertResultIntoBatch() const build_docker/../src/AggregateFunctions/IAggregateFunction.h:618:53
#4 0x2c4269d7 in void DB::Aggregator::convertToBlockImplFinal<>() const build_docker/../src/Interpreters/Aggregator.cpp:1878:49
#5 0x2c403b9f in void DB::Aggregator::convertToBlockImpl<>() const build_docker/../src/Interpreters/Aggregator.cpp:1714:13
ClickHouse#6 0x2be09b53 in DB::Aggregator::prepareBlockAndFillSingleLevel() const::$_2::operator()() const build_docker/../src/Interpreters/Aggregator.cpp:2144:9
ClickHouse#7 0x2be09b53 in DB::Block DB::Aggregator::prepareBlockAndFill<>() const build_docker/../src/Interpreters/Aggregator.cpp:2000:5
ClickHouse#8 0x2be09b53 in DB::Aggregator::prepareBlockAndFillSingleLevel() const build_docker/../src/Interpreters/Aggregator.cpp:2150:12
ClickHouse#9 0x2be37de3 in DB::Aggregator::mergeBlocks() build_docker/../src/Interpreters/Aggregator.cpp:3032:17
ClickHouse#10 0x308c27f8 in DB::MergingAggregatedBucketTransform::transform() build_docker/../src/Processors/Transforms/MergingAggregatedMemoryEfficientTransform.cpp:360:37
0x62d00012d218 is located 3608 bytes inside of 32768-byte region [0x62d00012c400,0x62d000134400)
freed by thread T186 (QueryPipelineEx) here:
#0 0xd701312 in free (/work1/azat/tmp/upstream/clickhouse-asan+0xd701312) (BuildId: b7977aef37e9f720)
...
ClickHouse#8 0x2e3c22eb in DB::ColumnAggregateFunction::~ColumnAggregateFunction() build_docker/../src/Columns/ColumnAggregateFunction.cpp:89:1
...
ClickHouse#18 0xd9fcdd4 in std::__1::vector<DB::ColumnWithTypeAndName, std::__1::allocator<DB::ColumnWithTypeAndName> >::~vector() build_docker/../contrib/libcxx/include/vector:401:9
ClickHouse#19 0x2be373f4 in DB::Aggregator::mergeBlocks() build_docker/../contrib/libcxx/include/__memory/unique_ptr.h
ClickHouse#20 0x308c27f8 in DB::MergingAggregatedBucketTransform::transform() build_docker/../src/Processors/Transforms/MergingAggregatedMemoryEfficientTransform.cpp:360:37
previously allocated by thread T186 (QueryPipelineEx) here:
#0 0xd7015be in malloc (/work1/azat/tmp/upstream/clickhouse-asan+0xd7015be) (BuildId: b7977aef37e9f720)
#1 0xd85190a in Allocator<false, false>::allocNoTrack(unsigned long, unsigned long) build_docker/../src/Common/Allocator.h:227:27
#2 0xd988d45 in Allocator<false, false>::alloc(unsigned long, unsigned long) build_docker/../src/Common/Allocator.h:96:16
#3 0xd988d45 in DB::Arena::MemoryChunk::MemoryChunk(unsigned long, DB::Arena::MemoryChunk*) build_docker/../src/Common/Arena.h:54:64
#4 0xd98904b in DB::Arena::addMemoryChunk(unsigned long) build_docker/../src/Common/Arena.h:122:20
#5 0xec9542c in DB::Arena::alignedAlloc(unsigned long, unsigned long) build_docker/../src/Common/Arena.h:171:13
ClickHouse#6 0x1470f123 in DB::AggregateFunctionMap<char8_t>::deserialize() const build_docker/../src/AggregateFunctions/AggregateFunctionMap.h:205:35
</details>
P.S. Thanks to @den-crane for the reproducer.
Fixes: ClickHouse#35359 (cc @den-crane @dongxiao-yang)
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
vitlibar
pushed a commit
that referenced
this pull request
Sep 16, 2022
…yFunc()
clang-15 reports [1]:
<details>
<summary>ASan report</summary>
```
==1==ERROR: AddressSanitizer: stack-use-after-return on address 0x7f1d04c4eb20 at pc 0x000031c4803c bp 0x7f1d05e19a00 sp 0x7f1d05e199f8
READ of size 8 at 0x7f1d04c4eb20 thread T200 (QueryPullPipeEx)
#0 0x31c4803b in DB::GetPriorityForLoadBalancing::getPriorityFunc(DB::LoadBalancing, unsigned long, unsigned long) const::$_3::operator()(unsigned long) const build_docker/../src/Common/GetPriorityForLoadBalancing.cpp:42:40
#1 0x31c4803b in decltype(static_cast<DB::GetPriorityForLoadBalancing::getPriorityFunc(DB::LoadBalancing, unsigned long, unsigned long) const::$_3&>(fp)(static_cast<unsigned long>(fp0))) std::__1::__invoke<DB::GetPriorityForLoadBalancing::getPriorityFunc(DB::LoadBalancing, unsigned long, unsigned long) const::$_3&, unsigned long>(DB::GetPriorityForLoadBalancing::getPriorityFunc(DB::LoadBalancing, unsigned long, unsigned long) const::$_3&, unsigned long&&) build_docker/../contrib/libcxx/include/type_traits:3640:23
#2 0x31c4803b in unsigned long std::__1::__invoke_void_return_wrapper<unsigned long, false>::__call<DB::GetPriorityForLoadBalancing::getPriorityFunc(DB::LoadBalancing, unsigned long, unsigned long) const::$_3&, unsigned long>(DB::GetPriorityForLoadBalancing::getPriorityFunc(DB::LoadBalancing, unsigned long, unsigned long) const::$_3&, unsigned long&&) build_docker/../contrib/libcxx/include/__functional/invoke.h:30:16
#3 0x31c4803b in std::__1::__function::__default_alloc_func<DB::GetPriorityForLoadBalancing::getPriorityFunc(DB::LoadBalancing, unsigned long, unsigned long) const::$_3, unsigned long (unsigned long)>::operator()(unsigned long&&) build_docker/../contrib/libcxx/include/__functional/function.h:230:12
#4 0x31c4803b in unsigned long std::__1::__function::__policy_invoker<unsigned long (unsigned long)>::__call_impl<std::__1::__function::__default_alloc_func<DB::GetPriorityForLoadBalancing::getPriorityFunc(DB::LoadBalancing, unsigned long, unsigned long) const::$_3, unsigned long (unsigned long)>>(std::__1::__function::__policy_storage const*, unsigned long) build_docker/../contrib/libcxx/include/__functional/function.h:711:16
#5 0x31c38b07 in std::__1::__function::__policy_func<unsigned long (unsigned long)>::operator()(unsigned long&&) const build_docker/../contrib/libcxx/include/__functional/function.h:843:16
ClickHouse#6 0x31c38b07 in std::__1::function<unsigned long (unsigned long)>::operator()(unsigned long) const build_docker/../contrib/libcxx/include/__functional/function.h:1184:12
ClickHouse#7 0x31c38b07 in PoolWithFailoverBase<DB::IConnectionPool>::getShuffledPools(unsigned long, std::__1::function<unsigned long (unsigned long)> const&) build_docker/../src/Common/PoolWithFailoverBase.h:174:39
This frame has 2 object(s):
[32, 40) 'pool_size.addr' <== Memory access at offset 32 is inside this variable
[64, 88) 'ref.tmp' (line 18)
```
</details>
[1]: https://s3.amazonaws.com/clickhouse-test-reports/41046/adea92f847373d1fcfd733d8979c63024f9b80bf/integration_tests__asan__[1/3].html
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
vitlibar
pushed a commit
that referenced
this pull request
Sep 27, 2022
…s and parallel KILL
Right now it is possible to call QueryStatus::addPipelineExecutor() when
the executors_mutex already acquired, it is possible when the query was
cancelled via KILL QUERY.
Here I will show some traces from debugger from a real example, where
tons of ProcessList::insert() got deadlocked.
Let's look at the lock owner for one of the threads that was deadlocked
in ProcessList::insert():
(gdb) p *mutex
$2 = {
__data = {
__owner = 46899,
},
}
And now let's see the stack trace of the 46899:
#0 __lll_lock_wait () at ../sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:103
#1 0x00007fb65569b714 in __GI___pthread_mutex_lock (mutex=0x7fb4a9d15298) at ../nptl/pthread_mutex_lock.c:80
#2 0x000000001b6edd91 in pthread_mutex_lock (arg=0x7fb4a9d15298) at ../src/Common/ThreadFuzzer.cpp:317
#3 std::__1::__libcpp_mutex_lock (__m=0x7fb4a9d15298) at ../contrib/libcxx/include/__threading_support:303
#4 std::__1::mutex::lock (this=0x7fb4a9d15298) at ../contrib/libcxx/src/mutex.cpp:33
#5 0x0000000014c7ae63 in std::__1::lock_guard<std::__1::mutex>::lock_guard (__m=..., this=<optimized out>) at ../contrib/libcxx/include/__mutex_base:91
ClickHouse#6 DB::QueryStatus::addPipelineExecutor (this=0x7fb4a9d14f90, e=0x80) at ../src/Interpreters/ProcessList.cpp:372
ClickHouse#7 0x0000000015bee4a7 in DB::PipelineExecutor::PipelineExecutor (this=0x7fb4b1e53618, processors=..., elem=<optimized out>) at ../src/Processors/Executors/PipelineExecutor.cpp:54
ClickHouse#12 std::__1::make_shared<DB::PipelineExecutor, std::__1::vector<std::__1::shared_ptr<DB::IProcessor>, std::__1::allocator<std::__1::shared_ptr<DB::IProcessor> > >&, DB::QueryStatus*&, void> (__args=@0x7fb63095b9b0: 0x7fb4a9d14f90, __args=@0x7fb63095b9b0: 0x7fb4a9d14f90) at ../contrib/libcxx/include/__memory/shared_ptr.h:963
ClickHouse#13 DB::QueryPipelineBuilder::execute (this=0x7fb63095b8b0) at ../src/QueryPipeline/QueryPipelineBuilder.cpp:552
ClickHouse#14 0x00000000158c6c27 in DB::Connection::sendExternalTablesData (this=0x7fb6545e9d98, data=...) at ../src/Client/Connection.cpp:797
ClickHouse#27 0x0000000014043a81 in DB::RemoteQueryExecutorRoutine::operator() (this=0x7fb63095bf20, sink=...) at ../src/QueryPipeline/RemoteQueryExecutorReadContext.cpp:46
ClickHouse#32 0x000000000a16dd4f in make_fcontext () at ../contrib/boost/libs/context/src/asm/make_x86_64_sysv_elf_gas.S:71
And also in the logs you can see very strange things for this thread:
2022.09.13 14:14:51.228979 [ 51145 ] {1712D4E914EC7C99} <Debug> Connection (localhost:9000): Sent data for 1 external tables, total 11 rows in 0.00046389 sec., 23688 rows/sec., 3.84 KiB (8.07 MiB/sec.), compressed 1.1070121092649958 times to 3.47 KiB (7.29 MiB/sec.)
...
2022.09.13 14:14:51.719402 [ 46899 ] {7c90ffa4-1dc8-42fd-938c-4e307c244394} <Debug> executeQuery: (from 10.101.15.181:42478) KILL QUERY WHERE query_id = '1712D4E914EC7C99' (stage: Complete)
2022.09.13 14:14:51.719488 [ 46899 ] {7c90ffa4-1dc8-42fd-938c-4e307c244394} <Debug> executeQuery: (internal) SELECT query_id, user, query FROM system.processes WHERE query_id = '1712D4E914EC7C99' (stage: Complete)
2022.09.13 14:14:51.719754 [ 46899 ] {7c90ffa4-1dc8-42fd-938c-4e307c244394} <Trace> ContextAccess (default): Access granted: SELECT(user, query_id, query) ON system.processes
2022.09.13 14:14:51.720544 [ 46899 ] {7c90ffa4-1dc8-42fd-938c-4e307c244394} <Trace> InterpreterSelectQuery: FetchColumns -> Complete
2022.09.13 14:14:53.228964 [ 46899 ] {7c90ffa4-1dc8-42fd-938c-4e307c244394} <Debug> Connection (localhost:9000): Sent data for 2 scalars, total 2 rows in 2.6838e-05 sec., 73461 rows/sec., 68.00 B (2.38 MiB/sec.), compressed 0.4594594594594595 times to 148.00 B (5.16 MiB/sec.)
How is this possible? The answer is fibers and query cancellation
routine. During cancellation of async queries it going into fibers again
and try to do this gracefully. However because of this during canceling
query it may call QueryStatus::addPipelineExecutor() from
QueryStatus::cancelQuery().
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
vitlibar
pushed a commit
that referenced
this pull request
Oct 26, 2022
…verflow
UBSAN report:
$ UBSAN_SYMBOLIZER_PATH=/usr/bin/llvm-symbolizer-14 UBSAN_OPTIONS=print_stacktrace=1 ./unit_tests_dbms --gtest_filter=*Gorilla*
../src/Compression/tests/gtest_compressionCodec.cpp:1216:47: runtime error: signed integer overflow: 23 * 100000000 cannot be represented in type 'int'
#0 0x14f67fd1 in auto (anonymous namespace)::$_6::operator()(int) const::'lambda'(auto)::operator()<int>(auto) const build_docker/../src/Compression/tests/gtest_compressionCodec.cpp:1216:47
#1 0x14f67fd1 in (anonymous namespace)::CodecTestSequence (anonymous namespace)::generateSeq<long, (anonymous namespace)::$_6::operator()(int) const::'lambda'(auto), int, int>((anonymous namespace)::$_6::operator()(int) const::'lambda'(auto), char const*, int, int) build_docker/../src/Compression/tests/gtest_compressionCodec.cpp:394:36
#2 0x14f67fd1 in auto (anonymous namespace)::GCompatibilityTestSequence<long>() build_docker/../src/Compression/tests/gtest_compressionCodec.cpp:1224:12
#3 0x14f3c7f3 in (anonymous namespace)::gtest_GorillaCodecTestCompatibility_EvalGenerator_() build_docker/../src/Compression/tests/gtest_compressionCodec.cpp:1227:1
#4 0x14f6bdb5 in testing::internal::ParameterizedTestSuiteInfo<(anonymous namespace)::CodecTestCompatibility>::RegisterTests() build_docker/../contrib/googletest/googletest/include/gtest/internal/gtest-param-util.h:553:45
#5 0x27d87988 in testing::internal::ParameterizedTestSuiteRegistry::RegisterTests() build_docker/../contrib/googletest/googletest/include/gtest/internal/gtest-param-util.h:726:24
ClickHouse#6 0x27d87988 in testing::internal::UnitTestImpl::RegisterParameterizedTests() build_docker/../contrib/googletest/googletest/src/gtest.cc:2805:34
ClickHouse#7 0x27d87988 in testing::internal::UnitTestImpl::PostFlagParsingInit() build_docker/../contrib/googletest/googletest/src/gtest.cc:5492:5
ClickHouse#8 0x27d9d002 in void testing::internal::InitGoogleTestImpl<char>(int*, char**) build_docker/../contrib/googletest/googletest/src/gtest.cc:6499:22
ClickHouse#9 0x14fd5495 in main build_docker/../src/Coordination/tests/gtest_coordination.cpp:2189:5
ClickHouse#10 0x7f8c29005209 (/lib/x86_64-linux-gnu/libc.so.6+0x29209) (BuildId: 71a7c7b97bc0b3e349a3d8640252655552082bf5)
ClickHouse#11 0x7f8c290052bb in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x292bb) (BuildId: 71a7c7b97bc0b3e349a3d8640252655552082bf5)
ClickHouse#12 0x14ce356d in _start (/work1/azat/tmp/42190/unit_tests_dbms+0x14ce356d) (BuildId: 482550e3f8d45f06e8c7f8147f427ee798c1f645)
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../src/Compression/tests/gtest_compressionCodec.cpp:1216:47 in
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
nikitamikhaylov
pushed a commit
that referenced
this pull request
Dec 2, 2022
Parallel grace hash join
nikitamikhaylov
pushed a commit
that referenced
this pull request
Dec 2, 2022
…reated
ASAN report:
Code: 586. DB::ErrnoException: Cannot create file: /src/.clickhouse_history, errno: 2, strerror: No such file or directory. (CANNOT_CREATE_FILE)
=================================================================
==1==ERROR: AddressSanitizer: heap-use-after-free on address 0x6240000208f0 at pc 0x000030d22ade bp 0x7ffff2ff3f70 sp 0x7ffff2ff3f68
READ of size 8 at 0x6240000208f0 thread T2
#0 0x30d22add in DB::ProcessList::insert() build_docker/../src/Interpreters/ProcessList.cpp:89:36
#1 0x31411018 in DB::executeQueryImpl() build_docker/../src/Interpreters/executeQuery.cpp:516:60
#2 0x3140e1ab in DB::executeQuery() build_docker/../src/Interpreters/executeQuery.cpp:1083:30
#3 0x3364391e in DB::LocalConnection::sendQuery() build_docker/../src/Client/LocalConnection.cpp:119:21
#4 0x3367bab0 in DB::Suggest::fetch() build_docker/../src/Client/Suggest.cpp:141:16
#5 0x336820eb in void DB::Suggest::load<DB::LocalConnection>()::'lambda'()::operator()() const build_docker/../src/Client/Suggest.cpp:118:17
0x6240000208f0 is located 2032 bytes inside of 7056-byte region [0x624000020100,0x624000021c90)
freed by thread T0 here:
#0 0xe381ef2 in operator delete(void*, unsigned long) (/wrk/clickhouse-asan+0xe381ef2) (BuildId: 6ea6d1a5d2d5a164f60f0fd8230936305bc8d9d0)
#1 0x335509fe in DB::ClientBase::~ClientBase() build_docker/../src/Client/ClientBase.cpp:293:25
#2 0x1f809bd5 in mainEntryClickHouseLocal(int, char**) build_docker/../programs/local/LocalServer.cpp:804:5
#3 0xe3856ad in main build_docker/../programs/main.cpp:482:12
#4 0x7ffff7dc0082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16
previously allocated by thread T0 here:
#0 0xe38128d in operator new(unsigned long) (/wrk/clickhouse-asan+0xe38128d) (BuildId: 6ea6d1a5d2d5a164f60f0fd8230936305bc8d9d0)
#1 0x2f34a7f3 in std::__1::__unique_if<DB::ContextSharedPart>::__unique_single std::__1::make_unique[abi:v15003]<DB::ContextSharedPart>() build_docker/../contrib/libcxx/include/__memory/unique_ptr.h:714:28
#2 0x2f34a7f3 in DB::Context::createShared() build_docker/../src/Interpreters/Context.cpp:603:32
#3 0x1f7f901d in DB::LocalServer::processConfig() build_docker/../programs/local/LocalServer.cpp:535:22
#4 0x1f7f4d92 in DB::LocalServer::main() build_docker/../programs/local/LocalServer.cpp:419:5
#5 0x3af24ffe in Poco::Util::Application::run() build_docker/../contrib/poco/Util/src/Application.cpp:334:8
ClickHouse#6 0x1f809bca in mainEntryClickHouseLocal(int, char**) build_docker/../programs/local/LocalServer.cpp:803:20
ClickHouse#7 0xe3856ad in main build_docker/../programs/main.cpp:482:12
ClickHouse#8 0x7ffff7dc0082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16
Thread T2 created by T0 here:
#0 0xe32fedc in pthread_create (/wrk/clickhouse-asan+0xe32fedc) (BuildId: 6ea6d1a5d2d5a164f60f0fd8230936305bc8d9d0)
#1 0x336806df in std::__1::__libcpp_thread_create[abi:v15003](unsigned long*, void* (*)(void*), void*) build_docker/../contrib/libcxx/include/__threading_support:376:10
#2 0x336806df in std::__1::thread::thread<void DB::Suggest::load<DB::LocalConnection>()::'lambda'(), void>() build_docker/../contrib/libcxx/include/thread:311:16
#3 0x3367ff5b in void DB::Suggest::load<DB::LocalConnection>(std::__1::shared_ptr<DB::Context const>, DB::ConnectionParameters const&, int) build_docker/../src/Client/Suggest.cpp:110:22
#4 0x3357fee9 in DB::ClientBase::runInteractive() build_docker/../src/Client/ClientBase.cpp:2066:22
#5 0x1f7f5264 in DB::LocalServer::main() build_docker/../programs/local/LocalServer.cpp
ClickHouse#6 0x3af24ffe in Poco::Util::Application::run() build_docker/../contrib/poco/Util/src/Application.cpp:334:8
ClickHouse#7 0x1f809bca in mainEntryClickHouseLocal(int, char**) build_docker/../programs/local/LocalServer.cpp:803:20
ClickHouse#8 0xe3856ad in main build_docker/../programs/main.cpp:482:12
ClickHouse#9 0x7ffff7dc0082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
vitlibar
pushed a commit
that referenced
this pull request
Jan 25, 2023
…ceptors)
Recently I noticed that clickhouse compiled with ASan does not work with
newer glibc 2.36+, before I though that this was only about compiling
with old but using new, however that was not correct, ASan simply does
not work with glibc 2.36+.
Here is a simple reproducer [1]:
$ cat > test-asan.cpp <<EOL
#include <pthread.h>
int main()
{
// something broken in ASan in interceptor for __pthread_mutex_lock
// and only since glibc 2.36, and for pthread_mutex_lock everything is OK
pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
return __pthread_mutex_lock(&mutex);
}
EOL
$ clang -g3 -o test-asan test-asan.cpp -fsanitize=address
$ ./test-asan
AddressSanitizer:DEADLYSIGNAL
=================================================================
==15659==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x000000000000 bp 0x7fffffffccb0 sp 0x7fffffffcb98 T0)
==15659==Hint: pc points to the zero page.
==15659==The signal is caused by a READ memory access.
==15659==Hint: address points to the zero page.
#0 0x0 (<unknown module>)
#1 0x7ffff7cda28f (/usr/lib/libc.so.6+0x2328f) (BuildId: 1e94beb079e278ac4f2c8bce1f53091548ea1584)
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (<unknown module>)
==15659==ABORTING
[1]: https://gist.github.com/azat/af073e57a248e04488b21068643f079e
I've started observing glibc code, there was some changes in glibc, that
moves pthread functions out from libpthread.so.0 into libc.so.6
(somewhere between 2.31 and 2.35), but
the problem pops up only with 2.36, 2.35 works fine.
After this I've looked into changes between 2.35 and 2.36, and found
this patch [2] - "dlsym: Make RTLD_NEXT prefer default version
definition [BZ ClickHouse#14932]", that fixes this bug [3].
[2]: https://sourceware.org/git/?p=glibc.git;a=commit;h=efa7936e4c91b1c260d03614bb26858fbb8a0204
[3]: https://sourceware.org/bugzilla/show_bug.cgi?id=14932
The problem with using DL_LOOKUP_RETURN_NEWEST flag for RTLD_NEXT is
that it does not resolve hidden symbols (and __pthread_mutex_lock is
indeed hidden).
Here is a sample that will show the difference [4]:
$ cat > test-dlsym.c <<EOL
#define _GNU_SOURCE
#include <dlfcn.h>
#include <stdio.h>
int main()
{
void *p = dlsym(RTLD_NEXT, "__pthread_mutex_lock");
printf("__pthread_mutex_lock: %p (via RTLD_NEXT)\n", p);
return 0;
}
EOL
# glibc 2.35: __pthread_mutex_lock: 0x7ffff7e27f70 (via RTLD_NEXT)
# glibc 2.36: __pthread_mutex_lock: (nil) (via RTLD_NEXT)
[4]: https://gist.github.com/azat/3b5f2ae6011bef2ae86392cea7789eb7
But ThreadFuzzer uses internal symbols to wrap
pthread_mutex_lock/pthread_mutex_unlock, which are intercepted by ASan
and this leads to NULL dereference.
The fix was obvious - just use dlsym(RTLD_NEXT), however on older
glibc's this leads to endless recursion (see commits in the code). But
only for jemalloc [5], and even though sanitizers does not uses jemalloc
the code of ThreadFuzzer is generic and I don't want to guard it with
more preprocessors macros.
[5]: https://gist.github.com/azat/588d9c72c1e70fc13ebe113197883aa2
So we have to use RTLD_NEXT only for ASan.
There is also one more interesting issue, if you will compile with clang
that itself had been compiled with newer libc (i.e. 2.36), you will get
the following error:
$ podman run --privileged -v $PWD/.cmake-asan/programs:/root/bin -e PATH=/bin:/root/bin -e --rm -it ubuntu-dev-v3 clickhouse
==1==ERROR: AddressSanitizer failed to allocate 0x0 (0) bytes of SetAlternateSignalStack (error code: 22)
...
==1==End of process memory map.
AddressSanitizer: CHECK failed: sanitizer_common.cpp:53 "((0 && "unable to mmap")) != (0)" (0x0, 0x0) (tid=1)
<empty stack>
The problem is that since GLIBC_2.31, `SIGSTKSZ` is a call to
`getconf(_SC_MINSIGSTKSZ)`, but older glibc does not have it, so `-1`
will be returned and used as `SIGSTKSZ` instead.
The workaround to disable alternative stack:
$ podman run --privileged -v $PWD/.cmake-asan/programs:/root/bin -e PATH=/bin:/root/bin -e ASAN_OPTIONS=use_sigaltstack=0 --rm -it ubuntu-dev-v3 clickhouse client --version
ClickHouse client version 22.13.1.1.
Fixes: ClickHouse#43426
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
vitlibar
pushed a commit
that referenced
this pull request
May 14, 2023
Reordering of the static variables should be enough to fix this:
[ RUN ] IOResourceStaticResourceManager.Prioritization
=================================================================
==13==ERROR: AddressSanitizer: stack-use-after-scope on address 0x7f7d8d621970 at pc 0x5636b80dcbb2 bp 0x7f7c48e47dd0 sp 0x7f7c48e47dc8
READ of size 8 at 0x7f7d8d621970 thread T3975 (ThreadPool)
#0 0x5636b80dcbb1 in IOResourceStaticResourceManager_Prioritization_Test::TestBody()::$_1::operator()(long) const build_docker/./src/IO/Resource/tests/gtest_resource_manager_static.cpp:78:13
#1 0x5636b80dcbb1 in IOResourceStaticResourceManager_Prioritization_Test::TestBody()::$_0::operator()() const build_docker/./src/IO/Resource/tests/gtest_resource_manager_static.cpp:95:21
#2 0x5636b80dcbb1 in decltype(std::declval<IOResourceStaticResourceManager_Prioritization_Test::TestBody()::$_0&>()()) std::__1::__invoke[abi:v15000]<IOResourceStaticResourceManager_Prioritization_Test::TestBody()::$_0&>(IOResourceStaticResourceManager_Prioritization_Test::TestBody()::$_0&) build_docker/./contrib/llvm-project/libcxx/include/__functional/invoke.h:394:23
#3 0x5636b80dcbb1 in decltype(auto) std::__1::__apply_tuple_impl[abi:v15000]<IOResourceStaticResourceManager_Prioritization_Test::TestBody()::$_0&, std::__1::tuple<>&>(IOResourceStaticResourceManager_Prioritization_Test::TestBody()::$_0&, std::__1::tuple<>&, std::__1::__tuple_indices<>) build_docker/./contrib/llvm-project/libcxx/include/tuple:1789:1
#4 0x5636b80dcbb1 in decltype(auto) std::__1::apply[abi:v15000]<IOResourceStaticResourceManager_Prioritization_Test::TestBody()::$_0&, std::__1::tuple<>&>(IOResourceStaticResourceManager_Prioritization_Test::TestBody()::$_0&, std::__1::tuple<>&) build_docker/./contrib/llvm-project/libcxx/include/tuple:1798:1
#5 0x5636b80dcbb1 in ThreadFromGlobalPoolImpl<true>::ThreadFromGlobalPoolImpl<IOResourceStaticResourceManager_Prioritization_Test::TestBody()::$_0>(IOResourceStaticResourceManager_Prioritization_Test::TestBody()::$_0&&)::'lambda'()::operator()() build_docker/./src/Common/ThreadPool.h:228:13
ClickHouse#6 0x5636b80dcbb1 in decltype(std::declval<IOResourceStaticResourceManager_Prioritization_Test::TestBody()::$_0>()()) std::__1::__invoke[abi:v15000]<ThreadFromGlobalPoolImpl<true>::ThreadFromGlobalPoolImpl<IOResourceStaticResourceManager_Prioritization_Test::TestBody()::$_0>(IOResourceStaticResourceManager_Prioritization_Test::TestBody()::$_0&&)::'lambda'()&>(IOResourceStaticResourceManager_Prioritization_Test::TestBody()::$_0&&) build_docker/./contrib/llvm-project/libcxx/include/__functional/invoke.h:394:23
ClickHouse#7 0x5636b80dcbb1 in void std::__1::__invoke_void_return_wrapper<void, true>::__call<ThreadFromGlobalPoolImpl<true>::ThreadFromGlobalPoolImpl<IOResourceStaticResourceManager_Prioritization_Test::TestBody()::$_0>(IOResourceStaticResourceManager_Prioritization_Test::TestBody()::$_0&&)::'lambda'()&>(ThreadFromGlobalPoolImpl<true>::ThreadFromGlobalPoolImpl<IOResourceStaticResourceManager_Prioritization_Test::TestBody()::$_0>(IOResourceStaticResourceManager_Prioritization_Test::TestBody()::$_0&&)::'lambda'()&) build_docker/./contrib/llvm-project/libcxx/include/__functional/invoke.h:479:9
ClickHouse#8 0x5636b80dcbb1 in std::__1::__function::__default_alloc_func<ThreadFromGlobalPoolImpl<true>::ThreadFromGlobalPoolImpl<IOResourceStaticResourceManager_Prioritization_Test::TestBody()::$_0>(IOResourceStaticResourceManager_Prioritization_Test::TestBody()::$_0&&)::'lambda'(), void ()>::operator()[abi:v15000]() build_docker/./contrib/llvm-project/libcxx/include/__functional/function.h:235:12
ClickHouse#9 0x5636b80dcbb1 in void std::__1::__function::__policy_invoker<void ()>::__call_impl<std::__1::__function::__default_alloc_func<ThreadFromGlobalPoolImpl<true>::ThreadFromGlobalPoolImpl<IOResourceStaticResourceManager_Prioritization_Test::TestBody()::$_0>(IOResourceStaticResourceManager_Prioritization_Test::TestBody()::$_0&&)::'lambda'(), void ()>>(std::__1::__function::__policy_storage const*) build_docker/./contrib/llvm-project/libcxx/include/__functional/function.h:716:16
ClickHouse#10 0x5636ea219310 in std::__1::__function::__policy_func<void ()>::operator()[abi:v15000]() const build_docker/./contrib/llvm-project/libcxx/include/__functional/function.h:848:16
ClickHouse#11 0x5636ea219310 in std::__1::function<void ()>::operator()() const build_docker/./contrib/llvm-project/libcxx/include/__functional/function.h:1187:12
ClickHouse#12 0x5636ea219310 in ThreadPoolImpl<std::__1::thread>::worker(std::__1::__list_iterator<std::__1::thread, void*>) build_docker/./src/Common/ThreadPool.cpp:416:13
ClickHouse#13 0x5636ea2258ac in void ThreadPoolImpl<std::__1::thread>::scheduleImpl<void>(std::__1::function<void ()>, long, std::__1::optional<unsigned long>, bool)::'lambda0'()::operator()() const build_docker/./src/Common/ThreadPool.cpp:180:73
ClickHouse#14 0x5636ea2258ac in decltype(std::declval<void>()()) std::__1::__invoke[abi:v15000]<void ThreadPoolImpl<std::__1::thread>::scheduleImpl<void>(std::__1::function<void ()>, long, std::__1::optional<unsigned long>, bool)::'lambda0'()>(void&&) build_docker/./contrib/llvm-project/libcxx/include/__functional/invoke.h:394:23
ClickHouse#15 0x5636ea2258ac in void std::__1::__thread_execute[abi:v15000]<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, void ThreadPoolImpl<std::__1::thread>::scheduleImpl<void>(std::__1::function<void ()>, long, std::__1::optional<unsigned long>, bool)::'lambda0'()>(std::__1::tuple<void, void ThreadPoolImpl<std::__1::thread>::scheduleImpl<void>(std::__1::function<void ()>, long, std::__1::optional<unsigned long>, bool)::'lambda0'()>&, std::__1::__tuple_indices<>) build_docker/./contrib/llvm-project/libcxx/include/thread:284:5
ClickHouse#16 0x5636ea2258ac in void* std::__1::__thread_proxy[abi:v15000]<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, void ThreadPoolImpl<std::__1::thread>::scheduleImpl<void>(std::__1::function<void ()>, long, std::__1::optional<unsigned long>, bool)::'lambda0'()>>(void*) build_docker/./contrib/llvm-project/libcxx/include/thread:295:5
ClickHouse#17 0x7f7d8fcf1608 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x8608) (BuildId: 7b4536f41cdaa5888408e82d0836e33dcf436466)
ClickHouse#18 0x7f7d8fc16132 in __clone (/lib/x86_64-linux-gnu/libc.so.6+0x11f132) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee)
Address 0x7f7d8d621970 is located in stack of thread T0 at offset 368 in frame
#0 0x5636b80d9bef in IOResourceStaticResourceManager_Prioritization_Test::TestBody() build_docker/./src/IO/Resource/tests/gtest_resource_manager_static.cpp:46
This frame has 10 object(s):
[32, 36) 'requests_per_thead' (line 48)
[48, 208) 't' (line 49)
[272, 296) 'ref.tmp' (line 51)
[336, 352) 'last_priority' (line 74)
[368, 376) 'check' (line 75) <== Memory access at offset 368 is inside this variable
[400, 424) 'name' (line 83)
[464, 512) 'ref.tmp13' (line 87)
[544, 560) 'c' (line 101)
[576, 600) 'ref.tmp32' (line 101)
[640, 664) 'ref.tmp41' (line 102)
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
vitlibar
pushed a commit
that referenced
this pull request
Jun 12, 2023
Fixed convertFieldToType case of converting Date and Date32 to DateTime64 Field
alexey-milovidov
pushed a commit
that referenced
this pull request
Jun 6, 2026
ClickHouse#106278 tried to remove the LIMIT-without-ORDER-BY non-determinism by sampling with WHERE cityHash64(WatchID) % N = 0. Post-merge master data showed it did not work: it made the measured queries scan the whole table on every run, so instead of reducing the noise it regressed it badly: classification #0: 6.5% -> 76% noisy classification #1: 10% -> 90% noisy (#2/#3/#4 were already clean.) Materialize the deterministic subset once into its own single-part table (single-threaded insert + OPTIMIZE FINAL) and measure over that, so the determinism lives in the setup (like ClickHouse#106249 for reading_from_file) and the measured queries read only the subset instead of rescanning the source.
vitlibar
pushed a commit
that referenced
this pull request
Jul 13, 2026
- Resolve setting query parameters on a copy of the changes instead of mutating the AST, so the placeholders are preserved for the old-server compatibility rewrite in ClientBase::processOrdinaryQuery (review major #1). - Pass the changes to checkSettingsConstraints as const: the non-const overload rewrites the list and drops no-op changes, which lost the "changed" flag for a setting explicitly set to its current value (e.g. SET log_queries = 1) and broke 03595_set_query_no_eq_set_to_one. - Teach QueryParameterVisitor / analyzeReceiveQueryParams to discover query parameters used as setting values (stored inside FieldFromAST), so predefined HTTP handlers accept param_<name> for them (review major #2). - Renumber the stateless test to 04416 to satisfy the test-numbers style check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
vitlibar
pushed a commit
that referenced
this pull request
Aug 1, 2026
Binary operators between instant vectors without any tags, e.g. 'vector(1) + vector(2)', as well as 'topk(1, vector(1))', 'label_replace(vector(1), ...)' and binary operators with 'on()' failed with an error like 'Argument #1 of function timeSeriesGroupToTags has wrong type UInt8, it must be UInt64'. The group #0 constant (meaning a group with no tags) was generated as a plain literal 0 in several places, and ClickHouse infers type UInt8 for such a literal while all timeSeries* functions working with groups require UInt64. Generate CAST(0, 'UInt64') instead, the same way transformGroupASTForAggregationOperator already does, and teach the group #0 short-circuit in transformGroupASTForBinaryOperator to recognize the CAST form.
vitlibar
pushed a commit
that referenced
this pull request
Aug 1, 2026
The comment on the outer-join control claimed that no `__join_result_dummy` marker is minted for those shapes. That is not what the code does: both mint sites are join-kind-unconditional (`JoinStepLogical.cpp:142-148` gates only on `outputs.empty()`, `:437-445` only on `required_nodes.empty()`); the sole join-kind-dependent code nearby is the nullability conversion, which does not gate minting. Measured with `EXPLAIN input_headers = 1, keep_logical_steps = 1` on the fix binary, the enclosing join's per-side input headers are: primary (aborts without the fix) #0 __join_result_dummy #1 __join_result_dummy outer joins #0 __join_result_dummy, __table3.a #1 __table4.a explicit CROSS JOIN #0 __join_result_dummy, __table3.a #1 __table4.a derived tables #0 __table1.c #1 __table4.c So the outer-join and explicit-cross shapes DO mint the marker, into the left child header only. What makes them negative controls is the property the defect actually needs: the two child headers of the enclosing join share no column name, so each name's queue holds exactly one input node and the mapping resolves to the same node whether it is rebuilt per child or hoisted. Only the primary shape has both children reduced to the identical marker name. The derived-table comment named `x.c` / `y.c` as the resulting column names; the measurement shows the table-expression-qualified `__table1.c` / `__table4.c`. Reworded to the measured names. Comment-only: the non-comment content of the test is unchanged (sha256 8d02af43049db436 before and after), `clickhouse format --multiquery --oneline` is identical across both revisions, and the `.reference` file has zero diff lines.
vitlibar
pushed a commit
that referenced
this pull request
Aug 17, 2026
The assertion added in the previous commit checked the join output header for __join_result_dummy. That column is synthesized by JoinStepLogical whenever the join has no required output columns (JoinStepLogical.cpp addToNullableIfNeeded, the `if (outputs.empty())` branch), which is a property of the SELECT list, not of the right side. It happened to discriminate the constant-folded `a >= 0` shape only because that shape leaves `b.size` as the single surviving output column, so the join output header names it instead of the dummy. That is an incidental property of this query, not a robust one: any shape where the surviving column comes from the left side would keep the dummy in the output header while the right side still fed a non-empty block into the join. Assert on the right pre-join input header instead, via EXPLAIN input_headers = 1. `#1 Empty header` is printed exactly when the right input header has no columns (QueryPlan.cpp explainStep, the `input_header->empty()` branch), which is the representation under test. The new form discriminates four mutations where the old one discriminated two: the constant-folded `a >= 0` shape, a swapped join order, selecting a right column so PREWHERE no longer consumes all of them, and dropping PREWHERE entirely. Reported by clickhouse-gh[bot].
vitlibar
pushed a commit
that referenced
this pull request
Aug 17, 2026
Casting a tag-less vector to a vector grid generated the group #0 constant as a plain literal 0 which ClickHouse types as UInt8, while timeSeries* functions require UInt64. The clamp() empty-result path for non-constant bounds converts scalar-backed vectors through toVectorGrid, so clamp(vector(5), time() * 0 + 1, -1) failed with 'Argument #1 of function timeSeriesGroupToTags has wrong type UInt8'. Same fix as in ClickHouse#111872 (which covers the remaining converter sites); the identical hunks will merge cleanly whichever PR lands first.
vitlibar
pushed a commit
that referenced
this pull request
Aug 17, 2026
The UInt64 casts for the no-tags group #0 constant (toVectorGrid, label manipulation functions, empty on() in binary operators) had no PromQL test exercising them, so the original "Argument #1 of function timeSeriesGroupToTags has wrong type UInt8, it must be UInt64" failure could regress without being noticed. Cover binary operators (including an empty on() over tag-less vectors), label_replace(), label_join(), topk() and the SCALAR_GRID range paths.
vitlibar
pushed a commit
that referenced
this pull request
Aug 27, 2026
`AggregatingTransform::initGenerate` used `many_data->variants.size()` as the
number of producers to wait for on the finish barrier. That vector is sized at
construction and used to be immutable afterwards, but the adaptive aggregation
made the last finisher append the early-drain routing table to it, so the
vector grows while the producers that are still arriving read its size:
Write of size 8 ... by thread T1033:
ClickHouse#7 DB::AggregatingTransform::initGenerate() src/Processors/Transforms/AggregatingTransform.cpp:1375
Previous read of size 8 ... by thread T1187:
#1 DB::AggregatingTransform::initGenerate() src/Processors/Transforms/AggregatingTransform.cpp:1319
Count the producers with `ManyAggregatedData::num_producers`, fixed at
construction time, so the barrier never reads the vector that the last
finisher grows. The barrier's outcome is unchanged: only the finisher whose
`fetch_add` returns `num_producers - 1` proceeds, and it is the only thread
that touches `variants` afterwards.
CI report:
https://s3.amazonaws.com/clickhouse-test-reports/json.html?REF=master&sha=ed4df189fc08473dd6cee5c84b4fce89d49ca640&name_0=MasterCI&name_1=Stress%20test%20%28arm_tsan%29
Caused by: ClickHouse#111459
vitlibar
pushed a commit
that referenced
this pull request
Sep 10, 2026
…ex comment Three of Copilot's four comments on ClickHouse#112309. `deserialize` took the row count straight from the serialized state and resized `samples` to it. States are user-transportable via `hierarchicalKMeansState`, so that count is untrusted input about to become an allocation. Bounded against `sample_cap`, which is the most the reservoir could ever have held. The comparison divides rather than multiplies, since `sample_cap * dim` overflows for large caps. `ReservoirSampler::read` guards the same way with a fixed 1 GiB cap, which would be wrong here - a legitimate 5M-vector reservoir at dim 768 is 15 GB, so the bound has to come from the configured cap instead. Verified with a crafted blob claiming 1M vectors of dimension 3 against a `sample_cap` of 5: rejected with `INCORRECT_DATA` before allocating, while the same blob under a large enough cap gets past the bound and fails later on the truncated payload. Note a plain SQL `CAST` between differently-parameterised states cannot reach this path at all - the type system rejects the conversion first - so the test goes through `unhex` to exercise it. `memcpy` was used with no `#include <cstring>`, relying on a transitive include. The `useDefaultImplementationForConstants` comment in `assignCentroid` said "arg #1" while the constant it means is argument index 1, the second one. Copilot's fourth comment, on the dictionary path allocating a vector per centroid before sorting, is not addressed: see the PR discussion. It is real but runs once per dictionary version, not per block or per row. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
alexey-milovidov
pushed a commit
that referenced
this pull request
Sep 19, 2026
…t covers a slow reject test_native_default_session_user_with_password fails only on the amd_llvm_coverage integration shards, with "TimeoutError: timed out" at native_hello's sock.recv(1): 3 failures over 7,621 coverage rows in 30 days, across three unrelated PRs (one of them documentation-only), and 0 master failures in 90 days. A rejected native login is answered only after two symbolized stack traces are rendered. The first comes from the catch in AccessControl::authenticate (src/Access/AccessControl.cpp:629); the second from writeException (src/IO/WriteHelpers.cpp:77-85), reached through TCPHandler::runImpl's sendException (src/Server/TCPHandler.cpp:507, :531) with the server context's calculate_text_stack_trace, and the packet reaches the wire only after it. Since StackTrace::toStringCached keys on the exact frame-pointer triple, the two distinct exceptions each pay a full DWARF symbolization. In the failing job's log (PR ClickHouse#115680 / 4ee5eff, coverage 2/8) those renders took 8.59s and 10.35s, so the client's first byte arrived 18.9s after connect; a second case in the same run took 16.6s. The helper allowed 10s. Nothing here is a server defect: coverage binaries are never shipped, the same renders cost 0.04s once the DWARF is parsed, and the logging is deliberate (AccessControl.cpp:661-663). The server's handshake_timeout_milliseconds is checked only before a read (src/IO/ReadBufferFromPocoSocket.cpp:106), which is why the answer does arrive at 18.9s instead of the connection being dropped. 60s is not an arbitrary pick: 60000ms is the value this repository already uses for a handshake read deadline on a slow lane, in six places. One is 00563_shard_insert_into_remote.sql (SET handshake_timeout_ms = 60000, by Alexey Milovidov, d4fed63); four are my own merged CI fixes (01600, 02040, 03630, 03443), each commented that the default 10s handshake_timeout_ms is not enough on slow lanes; stress_tests.lib:362 uses 30s. Two samples fix the order of magnitude of a first-time symbolization on a loaded runner, not its tail, so a deadline just above the observed maximum would re-flake. The cost is bounded: this is a deadline rather than a sleep, so a passing call pays nothing, native_hello has no retry loop, and pytest.ini's timeout = 900 still catches a genuine hang. The client's own handshake_timeout_ms default is also 10s, but that setting is an SLA for a real connection deciding when to fail over. This read only observes whether a login was rejected and asserts nothing about latency, so a client-realistic deadline adds no assertion value and only decides how often the observation is lost. Alternatives rejected: suppressing either render removes deliberate diagnostics on a security-relevant path; calculate_text_stack_trace=0 in the test profile would leave render #1 at 8.59s, i.e. 86% of a 10s budget, and weaken diagnostics for the module's other tests; skipping the case under coverage deletes reject-path coverage on the flavour whose purpose is measuring it. Two other hand-rolled native Hello sites are deliberately left alone. test_interserver_marker_requires_cluster_secret rethrows in interserver mode at TCPHandler.cpp:521-526 before sendException, so the packet-gating render is structurally absent and its oracle is a closed connection; it has never failed with this signature. In test_tcp_hello_string_limits a client timeout is part of the oracle (a rejection arm returns False on socket.timeout), and that module shortens the server's handshake timeout to 3000ms on purpose. Validation: with the coverage binary from the failing job the module passes 30/30 and the three native_hello callers pass 10 times each. The failure is reproduced deterministically against a stub listener answering after 15s: before this change the helper raises TimeoutError at sock.recv(1) in 10.01s, the exact CI frame; after it, the helper returns the Exception packet at 15.00s. A never-answering stub fails at 10s before and 60.00s after, so the constant is the operative variable, and an arm answering ServerHello instead still reddens the assertion in 0.05s. The 18.9s magnitude is measured in CI and is not reproducible on an idle box, where the same binary answers a reject in 0.105s. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
vitlibar
pushed a commit
that referenced
this pull request
Sep 26, 2026
The fixture fills its tables with min_insert_block_size_rows = 1, which disables squashing, so every block numbers(1000000) emits becomes its own part: 16 parts, 15 x 65409 + 18865 rows. Nothing stops merges, so the background scheduler collapses those 16 tiny single-partition parts while the harness is timing. The data is inserted in ascending order and the table is ORDER BY (), so each part covers a contiguous value range, and filterPartsByStatistics keeps only the parts whose range holds one of the three sought values: 3 of the 16 parts the fill writes, but both of 2 merged parts. Since total_marks_pk is summed over the survivors only, the granules the skip index has to evaluate are a step function of how far merging got before timing started, 39246 marks at 16 parts and up to 200003 at 2. The two sides of a comparison run set up concurrently on two servers, reach different points of that collapse, and the harness reports the difference as a slowdown of the binary under test. Over 10 days on Performance Comparison (arm_release, master_head, N/6), query #0 measures at two separated modes with an empty 16..24 ms bucket in 2493 samples per side, and the two sides' distributions match to within 2 samples. A mode flip reads as up to +291%, the job's own report calls the query slower, and the dashboard rates it confirmed_regression, which reds the shard on PRs whose diff cannot reach the query, and on master. SelectedMarks, SelectedRanges and SelectedRows are identical in both modes on both architectures, so no engine path misbehaves: the read is 3 granules either way and only the amount of index analysis moves. The fix therefore belongs in the fixture, which manufactures the non-determinism in its own fill. Stopping merges per table mirrors set_index_analysis.xml, the sibling fixture for this same feature; the per-table form, as in adaptive_codec_merge.xml, cannot reach another test's tables, and teardown restores merges and drops both tables, where the old file left test_minmax_index behind. Validated with an exact count oracle instead of timings, which makes it arch- and build-independent: with the pin, query #0 reports SelectedPartsTotal = 16 and SelectedMarksTotal = 39246, and query #1 reports 200003, on all 315 probes including after repeated sleeps. Unpinned, the same probe sees 11 parts / 91574 marks and then 6 / 156984 within half a second, and 6 / 156984 in the settled state. Removing only the two SYSTEM STOP MERGES lines brings the drift back. A co-tenant table filled while the pin is active still merges 16 parts down to 6, and after the drop_query block a fresh table of the same name merges again. Determinizing necessarily picks one mode, so the series steps once: amd from ~31 ms to ~14 ms, while the arm median stays at 8 ms. Related: ClickHouse#100759 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
vitlibar
pushed a commit
that referenced
this pull request
Sep 26, 2026
Holding merges keeps the parts the fill writes, but it does not determine how they are cut. min_insert_block_size_rows = 1 disables squashing, so one part is written per source block, and the source block size for numbers(1000000) is max_block_size, which this fixture never set. The existing max_insert_block_size = 65536 does not supply it: use_strict_insert_block_limits defaults to false, and in that state the max_insert_block_size settings are not enforced for an INSERT. So the 16-part layout every figure here depends on was a property of the compiled DEFAULT_BLOCK_SIZE rather than of the fixture, and the arithmetic closes only on that value: 15 x 65409 + 18865 = 16 parts, 3 x 13082 = 39246 selected marks, 15 x 13082 + 3773 = 200003 total. Setting max_block_size to 65409 in both fills makes the cut a property of the settings and keeps those measured values unchanged, which is what join_convert_full_to_left_or_right.xml already does for the same reason. The pin could also fail silently. SYSTEM STOP MERGES <table> does not error when the table is not found: the statement resolves the StorageID without an existence check, and the action lock is added only if the catalog returns a table. A rename or a typo would therefore turn the pin back into a no-op, and because the fixture asserted nothing about its own layout the only symptom would have been confirmed_regression reds reappearing weeks later on unrelated pull requests, which is the failure this fixture change exists to end. One throwIf over system.parts per table, in the shape group_by_low_cardinality_array_join.xml uses, turns that into an immediate job failure: a setup query that raises is fatal, since perf.py re-raises it out of the worker thread. Validated with the same exact count oracle: query #0 reports SelectedPartsTotal = 16 and SelectedMarksTotal = 39246, and query #1 reports 200003, on all 574 tight probes and every settled iteration. The assertion was proven live in both directions it has to catch. With only the two SYSTEM STOP MERGES lines removed, test_set_index is down to 11 active parts by the time the fills finish and the assertion fires on every run; with merges still held but max_block_size set to 70000, both tables land at 15 parts and it fires again. SelectedMarks, SelectedRanges and SelectedRows are unchanged throughout, so the fixture still analyses the same 3 granules it always read. Corrects the previous commit message's disclosure of the one-time step in the amd series: measured at the pinned layout it is ~20 ms, the median over 174 job-sides in 30 days, not ~14 ms, which rested on 4 samples. The arm median is unchanged at ~8 ms. Related: ClickHouse#100759 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
vitlibar
pushed a commit
that referenced
this pull request
Sep 26, 2026
Review asked to keep the block sizes always a power of two, so the fill's `max_block_size` moves from 65409 to 65536. It now matches the `max_insert_block_size` beside it and tests/performance/join_convert_full_to_left_or_right.xml, which pins the same pair at 65536. 65409 was the current default of the setting (`DEFAULT_BLOCK_SIZE` in src/Core/Defines.h, 65536 minus the SIMD padding), so the pinned value is now a deliberate round number instead of a restatement of that default. The comment dropped the sentence naming 65409 as the default, which the swap makes false, and states the invariant the pin buys instead. Nothing the test asserts or measures changes. Both tables still write 16 active parts, so the `throwIf` oracles stand; the cut moves from 15 x 65409 rows + 18865 to 15 x 65536 + 16960. Measured with `EXPLAIN indexes = 1` on all three queries, 65409 -> 65536: #0 parts 3/16 unchanged, index range 39246 -> 39321 granules, read 3 -> 3 #1 parts 1/16 unchanged, read 1 -> 1 #2 parts 12/16 unchanged, 156984 -> 157284 granules, read 133114 -> 133110 The +0.19% is the range the skip index walks, not the data read. With both sides at one layout the worst arm spread measured on #0 is +9.6% over 5537 jobs, against the check's 15% floor, so the shift is far below either.
vitlibar
pushed a commit
that referenced
this pull request
Sep 28, 2026
These queries were flagged as slower/faster/unstable on master runs, or scored as confirmed regressions by the performance dashboard on unrelated PRs, because the two compared servers did not measure the same work: - `and_compare_chain_derived`: both tables are created with `numbers_mt`, which writes a varying number of parts, and the measured runs stop merges, so the servers read different layouts (the join query took 34 ms over 17 parts and 82 ms over one part on the same binary). Merge each table to one part and check it. Query 2 now keeps 2M rows per side (`8000000 < l.v`) and runs at about 150 ms instead of 20 ms. - `set_index`: the IN set was built from `rand`, so every run looked up a different set, selected different ranges and read a different number of rows before `read_overflow_mode = 'break'`. Use a fixed pseudo-random sample (`toUInt32(cityHash64(number))`) and merge the 500M-row table to one part. - `formats_columns_nullable`: `SELECT * ... LIMIT 1000` read 6-10 ms, and parallel parsing parsed a varying amount ahead before the LIMIT cancelled it (`LIMIT 30000` of CSV took 197 ms with it and 38 ms without). Disable parallel parsing for that query and read 100000 rows, about 200 ms for text formats. - `serialization_pool_lookup`: the fixed `max_threads = 16` oversubscribed the 8 pinned cores of the amd runners; query 0 was flagged in 9.3% of amd master runs against 0.4% on arm. Use the harness default of one thread per pinned CPU. Master flag rates over 14 days (slower/faster/unstable share of master runs): `and_compare_chain_derived #2` 5-8%, `set_index #0` 14% (arm), `formats_columns_nullable #1` 19% (arm), `serialization_pool_lookup #0` 9% (amd).
vitlibar
pushed a commit
that referenced
this pull request
Sep 28, 2026
…ce test Each fill INSERT squashes the numbers_mt stream into parts of 256 MiB (min_insert_block_size_bytes), so a part holds the next ~3 blocks in the order the SELECT threads deliver them. perf.py fills both servers at the same time and merges stay stopped, so the two servers of a comparison measure different layouts for the whole test. On ClickHouse#104431 the right server read 16% more compressed bytes for query #1 with the same CPU time, and the dashboard gate reported +19.8% as a confirmed regression. Setting min_insert_block_size_rows = 1000000 (above the 500000 rows of an INSERT) and min_insert_block_size_bytes = 0 on both INSERTs makes each of them one block, sorted by the key when written, so both servers get the same two intersecting parts. The SELECT's SETTINGS reach the INSERT's squashing through InsertQuerySettingsPushDownVisitor. A throwIf fill query fails the test if the table does not have exactly 2 active parts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
vitlibar
pushed a commit
that referenced
this pull request
Sep 28, 2026
test_complex_table_schema and test_varchar_char_types_via_unity_catalog fail with
"subprocess.TimeoutExpired: ... bin/spark-sql ... timed out after 600 seconds".
CIDB over 30 days: 5 rows across 5 distinct PRs for this signature, 0 on master.
The Unity Catalog server accepts GET /schemas/{name} and never answers it, and the
UC Java client issues that request through java.net.http.HttpClient with no
per-request timeout, so the Spark driver parks for the whole subprocess budget.
The jstack that the test's own hang diagnostics capture 600s into the stall puts
it in the batch's first statement:
"main" #1 cpu=11625.96ms elapsed=600.24s WAITING (parking)
jdk.internal.net.http.HttpClientImpl.send
io.unitycatalog.client.api.SchemasApi.getSchemaWithHttpInfo(SchemasApi.java:273)
io.unitycatalog.spark.UCProxy.loadNamespaceMetadata(UCSingleCatalog.scala:399)
SupportsNamespaces.namespaceExists(SupportsNamespaces.java:98)
CreateNamespaceExec.run(CreateNamespaceExec.scala:43)
CreateNamespaceExec.run probes namespaceExists before creating anything. In the
same diagnostics both socket ends are ESTAB with Recv-Q = Send-Q = 0 for 600s
(the request was fully delivered and UC produced no response byte) while a
concurrent UC liveness probe answered in 10.7ms, so the server was serving other
requests and a fresh client converges.
Two alternatives are excluded by measurement. Ivy package resolution is not
involved: the failing run reports "0 artifacts copied, 33 already retrieved
(0kB/16ms)", because the image pre-resolves the same --packages list at build
time. Derby metastore init is not involved either: the JVM got well past
ObjectStore init.
UC is a third-party server pinned in the test image and its client constructs its
own HttpClient, so neither the handler nor a request timeout is reachable from the
spark-sql command line. The only lever the test owns is to survive the stall,
which is what ClickHouse#107133 established and ClickHouse#109641 extended to the read path.
Hence the same two-part remedy at the two measured write sites: rewrite the batch
into the idempotent form already used at the precedent site (CREATE SCHEMA IF NOT
EXISTS, CREATE TABLE IF NOT EXISTS, INSERT OVERWRITE), then pass
retry_on_timeout=True so one fresh-JVM replay is allowed. 2 x 300s fits the 900s
per-test cap from tests/integration/pytest.ini with roughly 300s of headroom; no
timeout constant changes. Both tables are named with a fresh uuid4() and hold
exactly one row, so a replay converges to identical content.
test_snapshot_version is deliberately left alone. It asserts the Delta commit
history (get_table_versions() == ["0","1","2"] and _commit_version), so INSERT
OVERWRITE would add a commit and break the assertions; making it replay-safe
needs a real restructure. That is the same boundary ClickHouse#109641 drew when it retried
only that test's read-only DESCRIBE HISTORY.
Validation. Since the UC stall cannot be forced, a temporary local injection
reported a timeout after the batch had really executed and committed, which is
the worst case for a replay: 4/4 test executions passed and each logged the
retry. Reverting exactly one edit at a time then reddens the run for the
predicted reason (SCHEMA_ALREADY_EXISTS, TABLE_OR_VIEW_ALREADY_EXISTS, and a
duplicate row breaking the column assertions), so no edit is decoration. Without
injection: 4/4 on the pristine tree and 4/4 with the fix in the same wall-clock
band, 21 passed / 3 skipped for the whole module, and 40/40 over --count 10 on
all four parameter ids.
Failing job:
https://github.com/ClickHouse/ClickHouse/actions/runs/35763632624/job/107009114425
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
vitlibar
pushed a commit
that referenced
this pull request
Sep 28, 2026
Query #0 groups 5M rows by a single-type Dynamic key. For every row the
aggregation looks up the variant's default serialization in
SerializationObjectPool: a shared lock on the key's shard mutex,
weak_ptr::lock, and the release of the returned SerializationPtr. These
are atomic read-modify-writes on two cache lines that every aggregation
thread writes for every row. One key means one shard, so the pool's
sharding cannot spread the load, and the query is bound by how fast those
lines move between cores. That speed is fixed per server process.
CI data (query_metric_runs_v1, arm, 1177 runs since 2026-09-17): the 5-7
samples of one server agree within 0.7% (p50), but the per-process medians
span 0.643-0.840 s (p05-p95). Fast and slow processes do the same work
(same arena bytes, memory, two-level hash tables and page faults) and
differ only in user CPU time (9.5 vs 12.1 s). The wall time does not
depend on the thread count: arm 0.754 s with 16 threads vs 0.775 s with
12, amd 0.669 s with 16 threads on 8 pinned cores vs 0.686 s with 8. So
each A/B run compares two random draws of that per-process speed. Runs
whose new/old median moved by more than 15% (raw samples):
arm PR arm master amd PR amd master
serialization_pool_lookup #0 123/789 54/558 68/176 220/560
serialization_pool_lookup #1 41/789 11/558 5/176 20/560
serialization_pool_lookup #2 0/789 0/558 0/176 0/560
group_by_dynamic_keys #0 0/782 0/559 0/173 0/559
group_by_dynamic_keys #0 runs the same per-row lookup path with
max_threads = 1. On PR ClickHouse#117533 the performance dashboard scored #0 as a
confirmed regression (arm 5/6, 0.670 -> 0.799 s) that the shard's own
report did not flag.
Run #0 in one thread, from numbers instead of numbers_mt, so it times the
pool's hit path itself, which does not depend on the process. Queries #1
(two keys, where sharding spreads the load) and #2 (the multi-threaded
MergeTree read) keep the contention coverage.
Local check on x86 with the official 26.10.1.820 master build, perf.py,
7 runs: old #0 1.76 s with 12 threads (15.4 s user CPU per run), new #0
0.57 s (0.56 s user CPU), same result. Ten fresh servers pinned to 8
cores with max_threads = 8: old #0 per-process medians 0.68-1.08 s; new
#0 0.56-0.58 s in 8 of them and 0.72-0.73 s in the two whose table
fills were also about 25% slower (9.2-10.0 s vs 7.4-8.6 s).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
vitlibar
pushed a commit
that referenced
this pull request
Sep 28, 2026
Pointing `contrib/nlohmann-json` at the ClickHouse fork makes the copy of `nlohmann::json` that `ai-sdk-cpp` vendors in `third_party/nlohmann_json_patched` an ODR violation rather than a duplicate. Both are version 3.12.0, so both define the same `nlohmann::json_abi_v3_12_0` symbols, and the two definitions no longer match: the fork holds a `number_buffer` member in `lexer` that the vendored copy has no field for, and drops the `decimal_point_char` the vendored copy keeps, so the fork's `parser` is the larger of the two. The linker resolves this by keeping one definition of each symbol, so a `parser` laid out by the vendored header is handed to a `parse` compiled from the fork header, which reads and destroys members past the end of it. In a sanitizer build this aborts on the first response of a model, which is how it was found - every AI client test of ClickHouse#114043 failed this way: ERROR: AddressSanitizer: stack-buffer-overflow #0 nlohmann::json_abi_v3_12_0::detail::parser<...>::parse(...) contrib/nlohmann-json/single_include/nlohmann/json.hpp:13073 #1 nlohmann::json_abi_v3_12_0::basic_json<...>::parse<...>(...) contrib/ai-sdk-cpp/third_party/nlohmann_json_patched/include/nlohmann/json.hpp:4053 #2 ai::providers::BaseProviderClient::generate_text_single_step(...) [32, 232) 'ref.tmp' (line 4053) <== Memory access at offset 240 overflows this variable and the `MemorySanitizer` build reports the same defect from the other side, the `lexer` destructor of that call destroying a `std::string` that was never constructed. In a release build the same code reads and writes past the end of a stack object without anything catching it, on the path of the `??` command of the client and of the server-side `aiGenerate` function. Nothing on this branch drives a model, which is why its own CI does not show this, so the fix goes here rather than only in the pull request that found it: this branch is the one that introduces the second definition. Build `ai-sdk-cpp` against `contrib/nlohmann-json` instead, so there is one. The vendored copy exists to keep the number parsing away from the locale, which the fork does properly, so there is nothing left to keep it for. `nlohmann-json` is now added unconditionally rather than only for `google-cloud-cpp` - the two are enabled independently of each other - and its include directory is marked as a system one, because it now reaches ClickHouse's own translation units through `ai-sdk-cpp`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vitlibar
pushed a commit
that referenced
this pull request
Oct 1, 2026
Each fill INSERT squashes the numbers_mt stream in the order the SELECT threads deliver its 65409-row blocks and cuts a part once min_insert_block_size_rows (1048449) rows have accumulated, i.e. every 17 blocks, so each INSERT writes 8 parts whose key ranges depend on thread timing. perf.py fills both servers at the same time and merges stay stopped, so the two servers of a comparison measure different 16-part tables for the whole test. Queries #0 and #1 stop at their LIMIT and read a layout-dependent number of rows: on ClickHouse#120186 the new server read 31% more rows for both, and the dashboard gate reported +17.9% and +19.3% as confirmed regressions. Both INSERTs now set min_insert_block_size_rows = 10000000 (above the 8000000 rows of an INSERT) and min_insert_block_size_bytes = 0 (an INSERT is ~350 MB, so the 256 MiB default would still cut it in two). Each INSERT becomes one block, sorted by the key when written, so both servers get the same two intersecting parts, the fixture the test was measured with. A throwIf fill query fails the test if the table does not have exactly 2 active parts. max_insert_threads cannot do this: it only fans the squashed blocks out to the insert sinks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
vitlibar
pushed a commit
that referenced
this pull request
Oct 1, 2026
…ion over a `Nullable` argument The `-If` null adapters merged the null map with the condition into a buffer sized by the absolute end row of the call and filled only `[row_begin, row_end)`. Aggregation in order calls `addBatchSinglePlace` once per key interval, and window frames once per frame update, so with 8-row groups every call asked for up to 64 KiB to use 8 bytes. The merges the nested functions do for themselves (`SingleValueData`, `groupBit*`, the simple moments, `regr_*`, `timeSeriesLastTwoSamples`) were sized the same way. On x86_64 every such request above 32 KiB exceeds jemalloc's thread cache limit (`opt_tcache_max`) and takes the arena large-extent path, whose cost depends on the state of the process. On aarch64 (64 KiB pages) these sizes are small size classes served from the thread cache. That is why queries #2 to #5 of `tests/performance/aggregation_in_order_if_null_map.xml` are unstable on the amd runners only: in the last 14 days CIDB has 2008 of the test's 2174 amd `unstable` verdicts on #2 to #5, and 8 on arm in total. With a single `Nullable` argument no merged map is needed: `addBatchSinglePlaceNotNull` takes the null map and the condition column separately, and every implementation handles `if_argument_pos` (`AggregateFunctionIf` already calls them all that way). So `AggregateFunctionIfNullUnary` with a non-`Nullable` condition passes the value's null map and the condition at position 1, and `AggregateFunctionIfNullVariadic` with exactly one `Nullable` argument passes that argument's null map to the nested `-If` function, which applies the condition. The merges those calls reach allocate `row_end - row_begin` bytes and call their kernels with pointers rebased by `row_begin`, as `sum` and `avg` already do. The same rows are added in the same order, so the results do not change. Two or more `Nullable` arguments (query #5, `corr(x, w)`), a `Nullable` condition of a one-argument `-If` function and the TimeSeries array merges keep their merged buffer. Measured on a release build (x86_64, `max_threads` 8), base / this change, per run of each query of the test: - jemalloc large allocations above 32 KiB: #0 703 / 703, #1 714 / 714, #2 302182 / 703, #3 302142 / 663, #4 229046 / 1718, #5 115413 / 115413; - CPU profiler samples in the jemalloc large-extent path: #2 33% / 0.9%, #3 31% / 0.5%; - perf.py median of 13 runs, two alternating rounds (ms): #2 38.2, 39.8 / 24.6, 21.7; #3 39.0, 40.2 / 21.3, 19.8; #4 44.3, 45.4 / 28.3, 25.9; #0, #1 and #5 unchanged; - `minIf(x, cond) OVER (ORDER BY k ROWS BETWEEN 100 PRECEDING AND CURRENT ROW)` over 1M rows: 340 / 166 ms, and `sumIf(y, cn)` over a `Nullable` condition: 301 / 89 ms; - aggregation without keys over the same table: 12.2 / 12.3 ms. Query #0, flagged on ClickHouse#118490, never had such a buffer and is not changed here. CI report: https://s3.amazonaws.com/clickhouse-test-reports/praktika.html?PR=118490&sha=8ded98e22889e53dd536b141b564e136c76482a9&name_0=PR&name_1=Performance%20Comparison%20(amd_release%2C%20master_head%2C%203%2F6) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
vitlibar
pushed a commit
that referenced
this pull request
Oct 5, 2026
…nce per block `mortonEncode` with two arguments ran one row loop for every argument type and mask: per row and argument it tested the mask and `is_const`, reloaded the ratio, switched on the column width and, with a mask, called the out-of-line `expand` (36 call sites in the arm `executeImpl`). On arm the speed of the branch-dense paths depends on the binary's layout, so `space_filling_curve` #1 (constant mask) and #4 (`UInt32` arguments) have per-build speed modes and flag unrelated PRs in Performance Comparison (ClickHouse#122482), while #2 (two `UInt64` columns) is stable. Now each argument is resolved once per call into a contiguous `UInt64` array with the range expansion applied: a `UInt64` column that needs no expansion is read in place, other widths are widened with one type dispatch, and a constant ratio is applied in one loop per ratio value (`expand` is `ALWAYS_INLINE` and called with a template-constant ratio, so its switch folds). A per-row mask keeps the per-row `expand`. `nd == 1` writes the result column directly. The encode loops only encode. x86, one core, median fix/base of 5 interleaved rounds over the queries of `space_filling_curve.xml`: #1 0.57 (BMI2) / 0.55 (lookup-table path, built with `-mno-bmi2`), #2/#3/ClickHouse#6 0.45-0.64, #4 0.86-0.88, untouched functions 0.99-1.03. Results are unchanged: 545 checksums over every width, mask and ratio are byte-identical before and after on both code paths, and the new test fails on both paths under two mutations of the change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
vitlibar
pushed a commit
that referenced
this pull request
Oct 5, 2026
…test The two compared servers each built a different copy of the fixture: - `rand64() % 5000000` makes the number of rows with key 5, which queries #1 and #3 look up, a fresh draw per server (0 to 3 over 10 local fills); - each INSERT is squashed into ~5 parts (cut at min_insert_block_size_rows), which merge until perf.py's --stop-merges freezes them at a per-server point. #1 and #3 read about one granule per part, so each server read its own number of granules: in CI the servers read different parts in 80% of runs, and time follows the rows read (correlation 0.85-0.88). This redded unrelated PRs (e.g. ClickHouse#118662, arm 4/6, #1 +37.5% confirmed_regression) and inflated the learned threshold of #1/#3 to 0.41-0.53 (0.04-0.09 for #0/#2/#4 on the same tables); the dashboard now caps both at likely_regression as "Flaky lately", so a real regression there cannot red. Use deterministic data, cityHash64(number, 7) and cityHash64(number, 11) modulo 5000000 (same key range and duplicate profile; the seeds put key 5 once in each table, the modal case of the random draw, so #1/#3 keep measuring a non-empty point join), and write each table as one part with min_insert_block_size_rows = 10000000, min_insert_block_size_bytes = 0 (5M rows give one squashed block). A throwIf on system.parts fails the fill if the layout ever drifts. Same shape as ClickHouse#122479 and ClickHouse#122660. Validated through perf.py --stop-merges on two servers, 5 fresh fills per arm: new fixture identical on all 10 server fills (one part per table, same data, #1/#3 read 2 granules on both, #0/#2/#4 read 10M rows); old fixture different on all 5 iterations. Without the insert settings the throwIf fails the fill. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
vitlibar
pushed a commit
that referenced
this pull request
Oct 5, 2026
Since ClickHouse#122458 the first query of the test (#0-ClickHouse#7, one per format) is SELECT * ... LIMIT 100000 with input_format_parallel_parsing = 0, so the text formats are parsed in one thread. On ARM the speed of that parse is fixed per server process: the 7 runs of one server agree within 1-2%, while different servers take 0.26-0.55 s for #2 (JSONEachRow). In the failing job on ClickHouse#101022 (+54.9%, 0.338 -> 0.524 s) the confirmation restart drew new levels for both servers (0.293 and 0.345 s), and the slow side read the same rows and bytes with 1.58x the user CPU. #1 and #2 share the factor within a process (r = 0.86); it does not reach the other 80 queries of the same processes, which read the same files with parallel parsing. Why the single-threaded parse is per process on ARM is not established. Runs whose two servers differ by more than 15% (query_metric_runs_v1, raw samples, since 2026-09-27): arm PR arm master amd PR amd master #0 TSVWithNames 38/200 5/33 0/48 0/42 #1 CSVWithNames 111/191 19/35 0/42 0/30 #2 JSONEachRow 112/199 15/31 0/34 0/32 #3-ClickHouse#7 binary 1/982 0/188 0/180 0/185 The 80 parallel-parsing queries on the text formats: 0 of 7232 ARM runs. Read the whole file with the default parallel parsing instead: without a LIMIT the amount parsed does not depend on read-ahead (why ClickHouse#122458 disabled parallel parsing), and no text format is parsed in one thread. Avro has no parallel parsing, so #3 now takes about 2.7 s on ARM (report-only "too long"; ClickHouse#75 of this test is already 2.16 s). The text of #0-ClickHouse#7 changes, ClickHouse#8-ClickHouse#87 are unchanged. Local check on x86 with the official 26.10.1.1403 master build, server pinned to 16 cores with max_parsing_threads = 16, perf.py, 7 runs: new #0-#2 0.43, 0.47 and 0.96 s with 3% spread, #3 2.15 s, #4-ClickHouse#7 0.09-0.53 s; every query reads all 2000000 rows of the file, #0-#2 in 7-15 threads. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
vitlibar
pushed a commit
that referenced
this pull request
Oct 11, 2026
…ce test Query #1 cast a String to JSON inside the measured query, so most of its CPU (about 80% in the arm CI profile, 72% on x86 locally) went to the JSON parser rather than to the serializing hash the test exists for. The parse has a per-server-process slow mode, so a slow draw on the tested server read as a hashing regression and turned unrelated PRs red (Performance Comparison on ClickHouse#117318: +15.8%, gone after a server restart). Build the JSON column once at fill time in a Memory table and hash the stored column, single-threaded as before and as in query #2. JSON parsing itself stays measured by new_json_type. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
vitlibar
pushed a commit
that referenced
this pull request
Oct 11, 2026
The sub-range branch in `HashMethodKeysFixed::getKeyHolder` was compiled into every fixed-key state, including those that only ever cover whole blocks (`Set` / `IN`, `LIMIT BY`, `JOIN`, whole-block `GROUP BY`). It merges a key returned in general-purpose registers into the hot loop, so the 128-bit key leaves `%xmm0` there too. Performance Comparison on the arm release build flagged it as blocking: `set_packed_keys #1` +15.8% and `limit_by_in_pk_order #1` +21.5%, both slower on this PR only in 14 days of CIDB. The sub-range logic now sits behind a `for_sub_range` template parameter. Only `ColumnsHashing::SubRangeState<State>` turns it on, and only for states that can batch-pack their keys. Its users are the three callers that build one state per run of a block: `executeOnBlockSmall` and `mergeOnBlockSmall` (in-order aggregation, single-level variants only), `DistinctSortedStreamTransform::buildFilterForRange` and `MergeTreeIndexAggregatorSet::buildFilter`. Every other state compiles master's code; the adaptive aggregator's slices go back to master's whole-block state too. Verified on the release configuration (clang-22, -O2, no ThinLTO) by compiling Set.cpp, LimitByTransform.cpp and Aggregator.cpp against master's headers and against this tree, then comparing every fixed-key function: all whole-block hot loops (`Set::insertFromBlockImplCase` / `executeImplCase`, `LimitByTransform::consumeImpl`, `Aggregator::executeImplBatch*`) are instruction-identical to master except two instructions per state construction that pass the default `RowRange`. The previous head differed in 36 of 48 `Set` and 21 of 37 `LIMIT BY` functions. The in-order copies add 443 KB to Aggregator.o .text. The headline query still runs in 0.049 s (6.2 s on master).
vitlibar
pushed a commit
that referenced
this pull request
Oct 12, 2026
…erformance test Queries #0 and #1 selected every 64th row (`number % 64`). On arm their timing is bimodal per server process: in CIDB, 264 / 271 of ~3570 arm runs since the test was added moved more than 10% old-vs-new, while the six other queries of the same test moved in 2-4 runs and amd in 5-6 runs. Queries ClickHouse#6 / ClickHouse#7 run the same `extractMaskNumericImpl` instantiations with an 8-row period, and #4 / #5 with a dense 64-row mask, and are stable, so the noise depends on the 64-row period of a sparse mask, not on the cost of the mask loop. Select the sparse rows by `intHash32(number) % 64` instead: the density stays 1/64 and the queries still take the compact short-circuit path the test was added for, but the mask has no period. `cityHash64` of an integer is `intHash64`, so the selector uses a different hash to stay independent of the other two arguments: about half of the selected rows still reach the third argument, as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
vitlibar
pushed a commit
that referenced
this pull request
Oct 13, 2026
The cache-policy randomization joined two `s|...|...|` substitutions into a single `sed` expression with `;`, but dropped the closing `|` of the first substitution, producing `sed: -e expression #1, char 94: unknown option to 's'` whenever a non-default policy (`LRU` or `SIEVE`) was selected for the mark and uncompressed caches. Split them into two `-e` expressions in both `ci/jobs/functional_tests.py` and `tests/docker_scripts/stress_runner.sh`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
vitlibar
pushed a commit
that referenced
this pull request
Oct 15, 2026
…ance test With `optimize_rewrite_like_perfect_affix=0`, every row of the fixture matches, so `MatchImpl` calls the dispatched StringZilla `sz_find` once per row, 10M times per query. On amd the cost of that call varies by 5-10% between binaries and between server processes (the red run's extra CPU samples are all in `sz_find_skylake`), so over 30 days the old/new ratio of `#0` crossed 15% in 29 of 1,193 master-vs-master runs and 26 of 1,316 PR runs (median 1.001), and reddened `Performance Comparison (amd_release, master_head, 2/6)` on unrelated ClickHouse#122420 (+20.9%, +17.2% after restart). `#2` behaves the same. The rewrite-on arms never crossed 15% on amd (0 of 2,518, p99 1.109 / 1.081). The harness compares the same query index across builds, never one arm against another, so the `=1` arms are the detector for the rewrite: if it stopped applying they would get 1.6x to 1.7x slower. The rewrite-off LIKE path stays measured by the two `optimize_rewrite_like_perfect_affix = 0` arms of `regexp_anchored_literal.xml` (0 runs above 12% in over 1,050 amd runs each), and correctness by `03594_like_perfect_affix_rewrite_pass`. Same shape as ClickHouse#122156. The survivors renumber to #0/#1, so their learned thresholds reset to the 0.15 floor, which is above their p99. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
vitlibar
pushed a commit
that referenced
this pull request
Oct 19, 2026
Since ClickHouse#110009 each perf server rebuilds hits_10m_single, hits_100m_single and datasets.hits_v1 -> test.hits itself: a 16-thread INSERT ... SELECT, then OPTIMIZE FINAL. The INSERT splits the rows into parts nondeterministically, and the vertical merge with variable adaptive granularity (blocks_are_granules_size in MergeTask) makes every merged granule follow the input parts' granules, so the left and right servers get different granule boundaries. order_by_read_in_order #1 reads in reverse order, only the last granule, whose size is the remainder of that layout: 510 vs 6041 rows on PR 117972 arm 2/6 (+18%), 958 vs 7977 on PR 119090 arm 4/6 (+50.9%). The layout lasts for the whole job, so the difference is perfectly separated and reproducible, and it reds shards. With use_const_adaptive_granularity the merged part's rows per granule is one constant computed from its total rows and bytes, capped at index_granularity. hits rows are 553-713 bytes, far below 10 MiB / 8192, so both servers get 8192-row granules. The setting only affects parts written while it is set, so it is reset after OPTIMIZE FINAL: the part keeps its granules, and CREATE TABLE ... AS clones (trivial_insert_select_from_files measures an INSERT into one) keep the default insert path. The double-check-perf-tests mirror of rebuild_table gets the same two statements, so --populate keeps matching CI. Validated on two local servers with the official master build and the CI datasets, driving the harness function itself: before, every rebuild pair differed (hits_100m_single query #1 read 7977 vs 993 rows); after, all pairs are identical (256 rows). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
vitlibar
pushed a commit
that referenced
this pull request
Oct 19, 2026
…ight-table column The hash join probe loops called the out-of-line AddedColumns::appendFromBlock for every matched row. When the join outputs no column of the right table, record_row_refs is false and the callee returned immediately, but the call stayed in the loop, and its cost depended on where the linker placed the loop and the callee: join_squashing_blocks #0/#1 ran 17-20% slower in about one build in 64 (the amd_release reds on ClickHouse#122184 and ClickHouse#120758), and 1.4x slower in some process starts. Make appendFromBlock ALWAYS_INLINE so the record_row_refs test happens in the loop, and keep the recording part, LazyOutput::addRef, out of line with NO_INLINE: inlining its body into every probe loop made hashjoin_low_cardinality_key #0 5-10% slower and pushed processMatch out of line, even in loops where addRef never runs. Joins that record row refs keep one call per match, to the same body as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
...
Detailed description / Documentation draft:
...
By adding documentation, you'll allow users to try your new feature immediately, not when someone else will have time to document it later. Documentation is necessary for all features that affect user experience in any way. You can add brief documentation draft above, or add documentation right into your patch as Markdown files in docs folder.
If you are doing this for the first time, it's recommended to read the lightweight Contributing to ClickHouse Documentation guide first.
Information about CI checks: https://clickhouse.tech/docs/en/development/continuous-integration/