Merge pull request ClickHouse#55577 from ClickHouse/fix_max_execution…

…_time_and_break_mode Fix max execution time and 'break' overflow mode
chenziliang · Oct 16, 2023 · 8449e4c · 8449e4c
2 parents a4bd689 + 1d7f042
commit 8449e4c
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 4 deletions.
diff --git 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<double>(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
@@ -25,7 +25,8 @@ class ExecutionSpeedLimits
     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
@@ -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});

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
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
@@ -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 }