From de62095b98ae6305753f0e148b21d3dd45414f93 Mon Sep 17 00:00:00 2001 From: Alexander Gololobov Date: Thu, 12 Oct 2023 23:40:26 +0200 Subject: [PATCH 1/2] Test for query success with 'break' mode and timeout > 10 sec --- ...x_execution_time_with_break_overflow_mode.reference | 0 ...896_max_execution_time_with_break_overflow_mode.sql | 10 ++++++++++ 2 files changed, 10 insertions(+) create mode 100644 tests/queries/0_stateless/02896_max_execution_time_with_break_overflow_mode.reference create mode 100644 tests/queries/0_stateless/02896_max_execution_time_with_break_overflow_mode.sql diff --git a/tests/queries/0_stateless/02896_max_execution_time_with_break_overflow_mode.reference b/tests/queries/0_stateless/02896_max_execution_time_with_break_overflow_mode.reference new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/queries/0_stateless/02896_max_execution_time_with_break_overflow_mode.sql b/tests/queries/0_stateless/02896_max_execution_time_with_break_overflow_mode.sql new file mode 100644 index 00000000000..439b8b3f032 --- /dev/null +++ b/tests/queries/0_stateless/02896_max_execution_time_with_break_overflow_mode.sql @@ -0,0 +1,10 @@ +-- Tags: no-fasttest + +-- Query stops after timeout without an error +SELECT * FROM numbers(100000000) SETTINGS max_block_size=1, max_execution_time=13, timeout_overflow_mode='break' FORMAT Null; + +-- Query returns an error when runtime is estimated after 10 sec of execution +SELECT * FROM numbers(100000000) SETTINGS max_block_size=1, max_execution_time=13, timeout_overflow_mode='throw' FORMAT Null; -- { serverError TOO_SLOW } + +-- Query returns timeout error before its full execution time is estimated +SELECT * FROM numbers(100000000) SETTINGS max_block_size=1, max_execution_time=2, timeout_overflow_mode='throw' FORMAT Null; -- { serverError TIMEOUT_EXCEEDED } From 1d7f04208fb1ecb59aec67b89e1ebd108b74404c Mon Sep 17 00:00:00 2001 From: Alexander Gololobov Date: Thu, 12 Oct 2023 23:42:01 +0200 Subject: [PATCH 2/2] Throw TOO_SLOW error only when overflow mode is 'throw' --- src/QueryPipeline/ExecutionSpeedLimits.cpp | 5 +++-- src/QueryPipeline/ExecutionSpeedLimits.h | 3 ++- src/QueryPipeline/ReadProgressCallback.cpp | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/QueryPipeline/ExecutionSpeedLimits.cpp b/src/QueryPipeline/ExecutionSpeedLimits.cpp index 111ba7c9a95..9ceaa4921c7 100644 --- a/src/QueryPipeline/ExecutionSpeedLimits.cpp +++ b/src/QueryPipeline/ExecutionSpeedLimits.cpp @@ -45,7 +45,8 @@ static void limitProgressingSpeed(size_t total_progress_size, size_t max_speed_i void ExecutionSpeedLimits::throttle( size_t read_rows, size_t read_bytes, - size_t total_rows_to_read, UInt64 total_elapsed_microseconds) const + size_t total_rows_to_read, UInt64 total_elapsed_microseconds, + OverflowMode timeout_overflow_mode) const { if ((min_execution_rps != 0 || max_execution_rps != 0 || min_execution_bps != 0 || max_execution_bps != 0 @@ -82,7 +83,7 @@ void ExecutionSpeedLimits::throttle( { double estimated_execution_time_seconds = elapsed_seconds * (static_cast(total_rows_to_read) / read_rows); - if (estimated_execution_time_seconds > max_execution_time.totalSeconds()) + if (timeout_overflow_mode == OverflowMode::THROW && estimated_execution_time_seconds > max_execution_time.totalSeconds()) throw Exception( ErrorCodes::TOO_SLOW, "Estimated query execution time ({} seconds) is too long. Maximum: {}. Estimated rows to process: {}", diff --git a/src/QueryPipeline/ExecutionSpeedLimits.h b/src/QueryPipeline/ExecutionSpeedLimits.h index 63658462c9f..eed8b5c3248 100644 --- a/src/QueryPipeline/ExecutionSpeedLimits.h +++ b/src/QueryPipeline/ExecutionSpeedLimits.h @@ -25,7 +25,8 @@ public: Poco::Timespan timeout_before_checking_execution_speed = 0; /// Pause execution in case if speed limits were exceeded. - void throttle(size_t read_rows, size_t read_bytes, size_t total_rows_to_read, UInt64 total_elapsed_microseconds) const; + void throttle(size_t read_rows, size_t read_bytes, size_t total_rows_to_read, UInt64 total_elapsed_microseconds, + OverflowMode timeout_overflow_mode) const; bool checkTimeLimit(const Stopwatch & stopwatch, OverflowMode overflow_mode) const; }; diff --git a/src/QueryPipeline/ReadProgressCallback.cpp b/src/QueryPipeline/ReadProgressCallback.cpp index 4d7c7aa0f2a..59843d8791d 100644 --- a/src/QueryPipeline/ReadProgressCallback.cpp +++ b/src/QueryPipeline/ReadProgressCallback.cpp @@ -130,7 +130,7 @@ bool ReadProgressCallback::onProgress(uint64_t read_rows, uint64_t read_bytes, c /// TODO: Should be done in PipelineExecutor. for (const auto & limits : storage_limits) - limits.local_limits.speed_limits.throttle(progress.read_rows, progress.read_bytes, total_rows, total_stopwatch.elapsedMicroseconds()); + limits.local_limits.speed_limits.throttle(progress.read_rows, progress.read_bytes, total_rows, total_stopwatch.elapsedMicroseconds(), limits.local_limits.timeout_overflow_mode); if (quota) quota->used({QuotaType::READ_ROWS, value.read_rows}, {QuotaType::READ_BYTES, value.read_bytes});