From 7e5b5420cb4270c424344ef4b33b40ed08370294 Mon Sep 17 00:00:00 2001 From: Nikita Taranov Date: Thu, 28 Mar 2024 14:44:28 +0000 Subject: [PATCH 1/3] impl --- src/Storages/StorageReplicatedMergeTree.cpp | 23 ++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/Storages/StorageReplicatedMergeTree.cpp b/src/Storages/StorageReplicatedMergeTree.cpp index c41403e312b..e0d041dc96c 100644 --- a/src/Storages/StorageReplicatedMergeTree.cpp +++ b/src/Storages/StorageReplicatedMergeTree.cpp @@ -5795,11 +5795,24 @@ bool StorageReplicatedMergeTree::optimize( if (select_decision != SelectPartsDecision::SELECTED) { - constexpr const char * message_fmt = "Cannot select parts for optimization: {}"; - assert(disable_reason != unknown_disable_reason); - if (!partition_id.empty()) - disable_reason += fmt::format(" (in partition {})", partition_id); - return handle_noop(message_fmt, disable_reason); + if (try_no + 1 < max_retries) + { + /// Here we trying to have a similar behaviour to ordinary MergeTree: if some merges are already in progress - let's wait for them to finish. + /// This way `optimize final` won't just silently be a noop (if also `optimize_throw_if_noop=false`), but will wait for the active merges and repeat an attempt to schedule final merge. + /// This guarantees are enough for tests, because there we have full control over insertions. + const auto wait_timeout = query_context->getSettingsRef().receive_timeout.totalMilliseconds() / max_retries; + /// DEFAULT (and not LIGHTWEIGHT) because merges are not condidered lightweight; empty `source_replicas` means "all replicas" + waitForProcessingQueue(wait_timeout, SyncReplicaMode::DEFAULT, {}); + continue; + } + else + { + constexpr const char * message_fmt = "Cannot select parts for optimization: {}"; + assert(disable_reason != unknown_disable_reason); + if (!partition_id.empty()) + disable_reason += fmt::format(" (in partition {})", partition_id); + return handle_noop(message_fmt, disable_reason); + } } ReplicatedMergeTreeLogEntryData merge_entry; From becedbdf4d79d3788f98e6eeb9faf3380c2b3d47 Mon Sep 17 00:00:00 2001 From: Nikita Taranov Date: Thu, 28 Mar 2024 18:22:58 +0000 Subject: [PATCH 2/3] add test --- .../03015_optimize_final_rmt.reference | 1 + .../0_stateless/03015_optimize_final_rmt.sh | 25 +++++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 tests/queries/0_stateless/03015_optimize_final_rmt.reference create mode 100755 tests/queries/0_stateless/03015_optimize_final_rmt.sh diff --git a/tests/queries/0_stateless/03015_optimize_final_rmt.reference b/tests/queries/0_stateless/03015_optimize_final_rmt.reference new file mode 100644 index 00000000000..d00491fd7e5 --- /dev/null +++ b/tests/queries/0_stateless/03015_optimize_final_rmt.reference @@ -0,0 +1 @@ +1 diff --git a/tests/queries/0_stateless/03015_optimize_final_rmt.sh b/tests/queries/0_stateless/03015_optimize_final_rmt.sh new file mode 100755 index 00000000000..96cb2ea22d1 --- /dev/null +++ b/tests/queries/0_stateless/03015_optimize_final_rmt.sh @@ -0,0 +1,25 @@ +#!/usr/bin/env bash +# Tags: long + +CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +# shellcheck source=../shell_config.sh +. "$CURDIR"/../shell_config.sh + +${CLICKHOUSE_CLIENT} -q "CREATE TABLE 03015_optimize_final_rmt(a UInt64) ENGINE=ReplicatedMergeTree('/clickhouse/tables/$CLICKHOUSE_TEST_ZOOKEEPER_PREFIX/03015_optimize_final_rmt', 'r1') ORDER BY a SETTINGS min_age_to_force_merge_seconds=1, merge_selecting_sleep_ms=100" + +for _ in {0..10}; do + ${CLICKHOUSE_CLIENT} --insert_deduplicate 0 -q "INSERT INTO 03015_optimize_final_rmt select * from numbers_mt(1e6)" +done + +# trigger a merge if it is not already running +${CLICKHOUSE_CLIENT} -q "OPTIMIZE TABLE 03015_optimize_final_rmt FINAL" & + +# this query should wait for the running merges, not just return immediately +${CLICKHOUSE_CLIENT} -q "OPTIMIZE TABLE 03015_optimize_final_rmt FINAL" + +# then at this point we should have a single part +${CLICKHOUSE_CLIENT} -q "SELECT COUNT() FROM system.parts WHERE database = currentDatabase() AND table = '03015_optimize_final_rmt' AND active" + +wait + +${CLICKHOUSE_CLIENT} --query "DROP TABLE 03015_optimize_final_rmt" From dd45151106c4f9c4bb25320c5fd6e7e46c56d21a Mon Sep 17 00:00:00 2001 From: Nikita Taranov Date: Wed, 10 Apr 2024 15:28:52 +0000 Subject: [PATCH 3/3] fix test --- tests/queries/0_stateless/03015_optimize_final_rmt.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/queries/0_stateless/03015_optimize_final_rmt.sh b/tests/queries/0_stateless/03015_optimize_final_rmt.sh index 96cb2ea22d1..f822a401c49 100755 --- a/tests/queries/0_stateless/03015_optimize_final_rmt.sh +++ b/tests/queries/0_stateless/03015_optimize_final_rmt.sh @@ -1,5 +1,5 @@ #!/usr/bin/env bash -# Tags: long +# Tags: long, no-random-settings, no-random-merge-tree-settings CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) # shellcheck source=../shell_config.sh