From 3540baa33c2d04788f152edb862888e66492e14a Mon Sep 17 00:00:00 2001 From: alesapin Date: Tue, 12 Oct 2021 11:58:33 +0300 Subject: [PATCH 1/9] Start server under gdb in functional tests --- docker/test/stateless/run.sh | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/docker/test/stateless/run.sh b/docker/test/stateless/run.sh index ed721690281..ebb72111e96 100755 --- a/docker/test/stateless/run.sh +++ b/docker/test/stateless/run.sh @@ -45,6 +45,23 @@ else sudo clickhouse start fi +echo " +set follow-fork-mode child +handle all noprint +handle SIGSEGV stop print +handle SIGBUS stop print +handle SIGABRT stop print +continue +thread apply all backtrace +detach +quit +" > script.gdb + +# FIXME Hung check may work incorrectly because of attached gdb +# 1. False positives are possible +# 2. We cannot attach another gdb to get stacktraces if some queries hung +gdb -batch -command script.gdb -p "$(cat /var/run/clickhouse-server/clickhouse-server.pid)" >> /test_output/gdb.log & + if [[ -n "$USE_DATABASE_REPLICATED" ]] && [[ "$USE_DATABASE_REPLICATED" -eq 1 ]]; then sudo -E -u clickhouse /usr/bin/clickhouse server --config /etc/clickhouse-server1/config.xml --daemon \ From d2dfbb5ab627ebe8f607fcc835e4a5a18737a303 Mon Sep 17 00:00:00 2001 From: alesapin Date: Wed, 13 Oct 2021 15:25:44 +0300 Subject: [PATCH 2/9] Remove trash from MergeTreeReadPool --- src/Storages/MergeTree/MergeTreeReadPool.cpp | 24 ------------------- src/Storages/MergeTree/MergeTreeReadPool.h | 3 --- .../MergeTreeThreadSelectProcessor.cpp | 11 ++++----- .../00167_read_bytes_from_fs.reference | 2 ++ .../1_stateful/00167_read_bytes_from_fs.sql | 7 ++++++ 5 files changed, 13 insertions(+), 34 deletions(-) create mode 100644 tests/queries/1_stateful/00167_read_bytes_from_fs.reference create mode 100644 tests/queries/1_stateful/00167_read_bytes_from_fs.sql diff --git a/src/Storages/MergeTree/MergeTreeReadPool.cpp b/src/Storages/MergeTree/MergeTreeReadPool.cpp index d08cec24184..4bb247f1369 100644 --- a/src/Storages/MergeTree/MergeTreeReadPool.cpp +++ b/src/Storages/MergeTree/MergeTreeReadPool.cpp @@ -142,30 +142,6 @@ MergeTreeReadTaskPtr MergeTreeReadPool::getTask(const size_t min_marks_to_read, prewhere_info && prewhere_info->remove_prewhere_column, per_part_should_reorder[part_idx], std::move(curr_task_size_predictor)); } -MarkRanges MergeTreeReadPool::getRestMarks(const IMergeTreeDataPart & part, const MarkRange & from) const -{ - MarkRanges all_part_ranges; - - /// Inefficient in presence of large number of data parts. - for (const auto & part_ranges : parts_ranges) - { - if (part_ranges.data_part.get() == &part) - { - all_part_ranges = part_ranges.ranges; - break; - } - } - if (all_part_ranges.empty()) - throw Exception("Trying to read marks range [" + std::to_string(from.begin) + ", " + std::to_string(from.end) + "] from part '" - + part.getFullPath() + "' which has no ranges in this query", ErrorCodes::LOGICAL_ERROR); - - auto begin = std::lower_bound(all_part_ranges.begin(), all_part_ranges.end(), from, [] (const auto & f, const auto & s) { return f.begin < s.begin; }); - if (begin == all_part_ranges.end()) - begin = std::prev(all_part_ranges.end()); - begin->begin = from.begin; - return MarkRanges(begin, all_part_ranges.end()); -} - Block MergeTreeReadPool::getHeader() const { return metadata_snapshot->getSampleBlockForColumns(column_names, data.getVirtuals(), data.getStorageID()); diff --git a/src/Storages/MergeTree/MergeTreeReadPool.h b/src/Storages/MergeTree/MergeTreeReadPool.h index 9949bdf86f8..380b132b806 100644 --- a/src/Storages/MergeTree/MergeTreeReadPool.h +++ b/src/Storages/MergeTree/MergeTreeReadPool.h @@ -85,9 +85,6 @@ public: */ void profileFeedback(const ReadBufferFromFileBase::ProfileInfo info); - /// This method tells which mark ranges we have to read if we start from @from mark range - MarkRanges getRestMarks(const IMergeTreeDataPart & part, const MarkRange & from) const; - Block getHeader() const; private: diff --git a/src/Storages/MergeTree/MergeTreeThreadSelectProcessor.cpp b/src/Storages/MergeTree/MergeTreeThreadSelectProcessor.cpp index 4eb6bc4b2e2..6a8ef860c87 100644 --- a/src/Storages/MergeTree/MergeTreeThreadSelectProcessor.cpp +++ b/src/Storages/MergeTree/MergeTreeThreadSelectProcessor.cpp @@ -68,18 +68,16 @@ bool MergeTreeThreadSelectProcessor::getNewTask() if (!reader) { - auto rest_mark_ranges = pool->getRestMarks(*task->data_part, task->mark_ranges[0]); - if (use_uncompressed_cache) owned_uncompressed_cache = storage.getContext()->getUncompressedCache(); owned_mark_cache = storage.getContext()->getMarkCache(); - reader = task->data_part->getReader(task->columns, metadata_snapshot, rest_mark_ranges, + reader = task->data_part->getReader(task->columns, metadata_snapshot, task->mark_ranges, owned_uncompressed_cache.get(), owned_mark_cache.get(), reader_settings, IMergeTreeReader::ValueSizeMap{}, profile_callback); if (prewhere_info) - pre_reader = task->data_part->getReader(task->pre_columns, metadata_snapshot, rest_mark_ranges, + pre_reader = task->data_part->getReader(task->pre_columns, metadata_snapshot, task->mark_ranges, owned_uncompressed_cache.get(), owned_mark_cache.get(), reader_settings, IMergeTreeReader::ValueSizeMap{}, profile_callback); } @@ -88,14 +86,13 @@ bool MergeTreeThreadSelectProcessor::getNewTask() /// in other case we can reuse readers, anyway they will be "seeked" to required mark if (part_name != last_readed_part_name) { - auto rest_mark_ranges = pool->getRestMarks(*task->data_part, task->mark_ranges[0]); /// retain avg_value_size_hints - reader = task->data_part->getReader(task->columns, metadata_snapshot, rest_mark_ranges, + reader = task->data_part->getReader(task->columns, metadata_snapshot, task->mark_ranges, owned_uncompressed_cache.get(), owned_mark_cache.get(), reader_settings, reader->getAvgValueSizeHints(), profile_callback); if (prewhere_info) - pre_reader = task->data_part->getReader(task->pre_columns, metadata_snapshot, rest_mark_ranges, + pre_reader = task->data_part->getReader(task->pre_columns, metadata_snapshot, task->mark_ranges, owned_uncompressed_cache.get(), owned_mark_cache.get(), reader_settings, reader->getAvgValueSizeHints(), profile_callback); } diff --git a/tests/queries/1_stateful/00167_read_bytes_from_fs.reference b/tests/queries/1_stateful/00167_read_bytes_from_fs.reference new file mode 100644 index 00000000000..05b54da2ac7 --- /dev/null +++ b/tests/queries/1_stateful/00167_read_bytes_from_fs.reference @@ -0,0 +1,2 @@ +468426149779992039 +1 diff --git a/tests/queries/1_stateful/00167_read_bytes_from_fs.sql b/tests/queries/1_stateful/00167_read_bytes_from_fs.sql new file mode 100644 index 00000000000..c3bdaea7abe --- /dev/null +++ b/tests/queries/1_stateful/00167_read_bytes_from_fs.sql @@ -0,0 +1,7 @@ +SELECT sum(cityHash64(*)) FROM test.hits SETTINGS max_threads=40 + +SYSTEM FLUSH LOGS; + +-- We had a bug which lead to additional compressed data read. hits compressed size if about 1.2G, but we read more then 3GB. +-- Small additional reads still possible, so we compare with about 1.5Gb. +SELECT ProfileEvents['ReadBufferFromFileDescriptorReadBytes'] < 1500000000 from system.query_log where query = 'SELECT sum(cityHash64(*)) FROM datasets.hits_v1 SETTINGS max_threads=40' and type = 'QueryFinish' From 7e85b7e407838e55ee290f5747684bb6c95b44bd Mon Sep 17 00:00:00 2001 From: alesapin Date: Wed, 13 Oct 2021 15:27:42 +0300 Subject: [PATCH 3/9] Remove accident change --- docker/test/stateless/run.sh | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/docker/test/stateless/run.sh b/docker/test/stateless/run.sh index ebb72111e96..ed721690281 100755 --- a/docker/test/stateless/run.sh +++ b/docker/test/stateless/run.sh @@ -45,23 +45,6 @@ else sudo clickhouse start fi -echo " -set follow-fork-mode child -handle all noprint -handle SIGSEGV stop print -handle SIGBUS stop print -handle SIGABRT stop print -continue -thread apply all backtrace -detach -quit -" > script.gdb - -# FIXME Hung check may work incorrectly because of attached gdb -# 1. False positives are possible -# 2. We cannot attach another gdb to get stacktraces if some queries hung -gdb -batch -command script.gdb -p "$(cat /var/run/clickhouse-server/clickhouse-server.pid)" >> /test_output/gdb.log & - if [[ -n "$USE_DATABASE_REPLICATED" ]] && [[ "$USE_DATABASE_REPLICATED" -eq 1 ]]; then sudo -E -u clickhouse /usr/bin/clickhouse server --config /etc/clickhouse-server1/config.xml --daemon \ From 886d10c3ea19c04a35ac43e175e0730a1148adc9 Mon Sep 17 00:00:00 2001 From: alesapin Date: Tue, 12 Oct 2021 11:58:33 +0300 Subject: [PATCH 4/9] Start server under gdb in functional tests --- docker/test/stateless/run.sh | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/docker/test/stateless/run.sh b/docker/test/stateless/run.sh index ce1d1b59a55..ec0af024b8b 100755 --- a/docker/test/stateless/run.sh +++ b/docker/test/stateless/run.sh @@ -45,6 +45,23 @@ else sudo clickhouse start fi +echo " +set follow-fork-mode child +handle all noprint +handle SIGSEGV stop print +handle SIGBUS stop print +handle SIGABRT stop print +continue +thread apply all backtrace +detach +quit +" > script.gdb + +# FIXME Hung check may work incorrectly because of attached gdb +# 1. False positives are possible +# 2. We cannot attach another gdb to get stacktraces if some queries hung +gdb -batch -command script.gdb -p "$(cat /var/run/clickhouse-server/clickhouse-server.pid)" >> /test_output/gdb.log & + if [[ -n "$USE_DATABASE_REPLICATED" ]] && [[ "$USE_DATABASE_REPLICATED" -eq 1 ]]; then sudo -E -u clickhouse /usr/bin/clickhouse server --config /etc/clickhouse-server1/config.xml --daemon \ From 2473bc5affadd1cb397ef810a262845fd8900220 Mon Sep 17 00:00:00 2001 From: alesapin Date: Wed, 13 Oct 2021 18:12:04 +0300 Subject: [PATCH 5/9] Fix test --- tests/queries/1_stateful/00167_read_bytes_from_fs.sql | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/queries/1_stateful/00167_read_bytes_from_fs.sql b/tests/queries/1_stateful/00167_read_bytes_from_fs.sql index c3bdaea7abe..341730bd82d 100644 --- a/tests/queries/1_stateful/00167_read_bytes_from_fs.sql +++ b/tests/queries/1_stateful/00167_read_bytes_from_fs.sql @@ -1,7 +1,7 @@ -SELECT sum(cityHash64(*)) FROM test.hits SETTINGS max_threads=40 +SELECT sum(cityHash64(*)) FROM test.hits SETTINGS max_threads=40; SYSTEM FLUSH LOGS; --- We had a bug which lead to additional compressed data read. hits compressed size if about 1.2G, but we read more then 3GB. +-- We had a bug which lead to additional compressed data read. test.hits compressed size is about 1.2Gb, but we read more then 3Gb. -- Small additional reads still possible, so we compare with about 1.5Gb. -SELECT ProfileEvents['ReadBufferFromFileDescriptorReadBytes'] < 1500000000 from system.query_log where query = 'SELECT sum(cityHash64(*)) FROM datasets.hits_v1 SETTINGS max_threads=40' and type = 'QueryFinish' +SELECT ProfileEvents['ReadBufferFromFileDescriptorReadBytes'] < 1500000000 from system.query_log where query = 'SELECT sum(cityHash64(*)) FROM datasets.hits_v1 SETTINGS max_threads=40' and databaser=currentDatabase() and type = 'QueryFinish'; From 98b555e7f77e0c134775fd9abb344977645f29de Mon Sep 17 00:00:00 2001 From: alesapin Date: Wed, 13 Oct 2021 19:21:11 +0300 Subject: [PATCH 6/9] Update run.sh --- docker/test/stateless/run.sh | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/docker/test/stateless/run.sh b/docker/test/stateless/run.sh index ec0af024b8b..ce1d1b59a55 100755 --- a/docker/test/stateless/run.sh +++ b/docker/test/stateless/run.sh @@ -45,23 +45,6 @@ else sudo clickhouse start fi -echo " -set follow-fork-mode child -handle all noprint -handle SIGSEGV stop print -handle SIGBUS stop print -handle SIGABRT stop print -continue -thread apply all backtrace -detach -quit -" > script.gdb - -# FIXME Hung check may work incorrectly because of attached gdb -# 1. False positives are possible -# 2. We cannot attach another gdb to get stacktraces if some queries hung -gdb -batch -command script.gdb -p "$(cat /var/run/clickhouse-server/clickhouse-server.pid)" >> /test_output/gdb.log & - if [[ -n "$USE_DATABASE_REPLICATED" ]] && [[ "$USE_DATABASE_REPLICATED" -eq 1 ]]; then sudo -E -u clickhouse /usr/bin/clickhouse server --config /etc/clickhouse-server1/config.xml --daemon \ From 386d47cb227b580a63635e1b94a9d8a765d5cb97 Mon Sep 17 00:00:00 2001 From: alesapin Date: Thu, 14 Oct 2021 10:49:12 +0300 Subject: [PATCH 7/9] Fix typo --- tests/queries/1_stateful/00167_read_bytes_from_fs.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/queries/1_stateful/00167_read_bytes_from_fs.sql b/tests/queries/1_stateful/00167_read_bytes_from_fs.sql index 341730bd82d..435bac85bc4 100644 --- a/tests/queries/1_stateful/00167_read_bytes_from_fs.sql +++ b/tests/queries/1_stateful/00167_read_bytes_from_fs.sql @@ -4,4 +4,4 @@ SYSTEM FLUSH LOGS; -- We had a bug which lead to additional compressed data read. test.hits compressed size is about 1.2Gb, but we read more then 3Gb. -- Small additional reads still possible, so we compare with about 1.5Gb. -SELECT ProfileEvents['ReadBufferFromFileDescriptorReadBytes'] < 1500000000 from system.query_log where query = 'SELECT sum(cityHash64(*)) FROM datasets.hits_v1 SETTINGS max_threads=40' and databaser=currentDatabase() and type = 'QueryFinish'; +SELECT ProfileEvents['ReadBufferFromFileDescriptorReadBytes'] < 1500000000 from system.query_log where query = 'SELECT sum(cityHash64(*)) FROM datasets.hits_v1 SETTINGS max_threads=40' and current_database = currentDatabase() and type = 'QueryFinish'; From 01ac2fea7991955ad68c8c0a4304fb0649ea84f5 Mon Sep 17 00:00:00 2001 From: alesapin Date: Thu, 14 Oct 2021 12:44:41 +0300 Subject: [PATCH 8/9] Update 00167_read_bytes_from_fs.sql --- tests/queries/1_stateful/00167_read_bytes_from_fs.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/queries/1_stateful/00167_read_bytes_from_fs.sql b/tests/queries/1_stateful/00167_read_bytes_from_fs.sql index 435bac85bc4..ee3e6b94537 100644 --- a/tests/queries/1_stateful/00167_read_bytes_from_fs.sql +++ b/tests/queries/1_stateful/00167_read_bytes_from_fs.sql @@ -4,4 +4,4 @@ SYSTEM FLUSH LOGS; -- We had a bug which lead to additional compressed data read. test.hits compressed size is about 1.2Gb, but we read more then 3Gb. -- Small additional reads still possible, so we compare with about 1.5Gb. -SELECT ProfileEvents['ReadBufferFromFileDescriptorReadBytes'] < 1500000000 from system.query_log where query = 'SELECT sum(cityHash64(*)) FROM datasets.hits_v1 SETTINGS max_threads=40' and current_database = currentDatabase() and type = 'QueryFinish'; +SELECT ProfileEvents['ReadBufferFromFileDescriptorReadBytes'] < 1500000000 from system.query_log where query = 'SELECT sum(cityHash64(*)) FROM test.hits SETTINGS max_threads=40' and current_database = currentDatabase() and type = 'QueryFinish'; From 4ab6f7d771a453eb4f098d5a666b8170eb565f76 Mon Sep 17 00:00:00 2001 From: alesapin Date: Fri, 15 Oct 2021 10:39:31 +0300 Subject: [PATCH 9/9] Finally fix test --- tests/queries/1_stateful/00167_read_bytes_from_fs.sql | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/queries/1_stateful/00167_read_bytes_from_fs.sql b/tests/queries/1_stateful/00167_read_bytes_from_fs.sql index ee3e6b94537..ac20e60b177 100644 --- a/tests/queries/1_stateful/00167_read_bytes_from_fs.sql +++ b/tests/queries/1_stateful/00167_read_bytes_from_fs.sql @@ -1,7 +1,7 @@ SELECT sum(cityHash64(*)) FROM test.hits SETTINGS max_threads=40; -SYSTEM FLUSH LOGS; - -- We had a bug which lead to additional compressed data read. test.hits compressed size is about 1.2Gb, but we read more then 3Gb. -- Small additional reads still possible, so we compare with about 1.5Gb. -SELECT ProfileEvents['ReadBufferFromFileDescriptorReadBytes'] < 1500000000 from system.query_log where query = 'SELECT sum(cityHash64(*)) FROM test.hits SETTINGS max_threads=40' and current_database = currentDatabase() and type = 'QueryFinish'; +SYSTEM FLUSH LOGS; + +SELECT ProfileEvents['ReadBufferFromFileDescriptorReadBytes'] < 1500000000 from system.query_log where query = 'SELECT sum(cityHash64(*)) FROM test.hits SETTINGS max_threads=40;' and current_database = currentDatabase() and type = 'QueryFinish';