From ebcf455f4a88043234fb19770bfc29958789c090 Mon Sep 17 00:00:00 2001 From: Igor Nikonov Date: Fri, 28 Jun 2024 22:17:34 +0000 Subject: [PATCH] Fix: progress bar for reading in order --- src/Processors/QueryPlan/ReadFromMergeTree.cpp | 9 +++++---- .../MergeTree/ParallelReplicasReadingCoordinator.cpp | 6 +++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/Processors/QueryPlan/ReadFromMergeTree.cpp b/src/Processors/QueryPlan/ReadFromMergeTree.cpp index e4e251b694e..08c6989242e 100644 --- a/src/Processors/QueryPlan/ReadFromMergeTree.cpp +++ b/src/Processors/QueryPlan/ReadFromMergeTree.cpp @@ -592,8 +592,10 @@ Pipe ReadFromMergeTree::readInOrder( /// Also do not count amount of read rows if we read in order of sorting key, /// because we don't know actual amount of read rows in case when limit is set. const UInt64 in_order_limit = query_info.input_order_info ? query_info.input_order_info->limit : 0; - const bool set_total_rows_approx - = !(is_parallel_reading_from_replicas && context->canUseParallelReplicasOnFollower()) && !in_order_limit; + const bool parallel_replicas_remote_plan_for_initiator = is_parallel_reading_from_replicas + && !context->getSettingsRef().parallel_replicas_local_plan && context->canUseParallelReplicasOnInitiator(); + const bool parallel_replicas_follower = is_parallel_reading_from_replicas && context->canUseParallelReplicasOnFollower(); + const bool set_total_rows_approx = !parallel_replicas_follower && !parallel_replicas_remote_plan_for_initiator && !in_order_limit; Pipes pipes; for (size_t i = 0; i < parts_with_ranges.size(); ++i) @@ -1968,8 +1970,7 @@ void ReadFromMergeTree::initializePipeline(QueryPipelineBuilder & pipeline, cons mode = CoordinationMode::ReverseOrder; break; case ReadFromMergeTree::ReadType::ParallelReplicas: - chassert(false); - UNREACHABLE(); + throw Exception(ErrorCodes::LOGICAL_ERROR, "Read type can't be ParallelReplicas on initiator"); } chassert(number_of_current_replica.has_value()); diff --git a/src/Storages/MergeTree/ParallelReplicasReadingCoordinator.cpp b/src/Storages/MergeTree/ParallelReplicasReadingCoordinator.cpp index 601ab3aeb70..f66cfdafa1a 100644 --- a/src/Storages/MergeTree/ParallelReplicasReadingCoordinator.cpp +++ b/src/Storages/MergeTree/ParallelReplicasReadingCoordinator.cpp @@ -888,9 +888,8 @@ void InOrderCoordinator::doHandleInitialAllRangesAnnouncement(InitialAllRa ++stats[announcement.replica_num].number_of_requests; - /// FIXME: this code updating total_rows_to_read but it needs to be done only once since we're taking working set from initiator - /// util I missing something, it seems this code is not necessary if working set is taken from initiator (todo: check it) - if (new_rows_to_read > 0 && progress_callback) + // progress_callback is not set when local plan is used for initiator + if (progress_callback && new_rows_to_read > 0) { Progress progress; progress.total_rows_to_read = new_rows_to_read; @@ -1052,6 +1051,7 @@ void ParallelReplicasReadingCoordinator::initialize(CoordinationMode mode) break; } + // progress_callback is not set when local plan is used for initiator if (progress_callback) pimpl->setProgressCallback(std::move(progress_callback));