MDEV-35880: Fix incorrect error message when MAX_EXECUTION_TIME was e… #4525
Conversation
Olernov
force-pushed
the
12.2-MDEV-35880-max-time
branch
from
13ab85e to
21eda5d
Compare
…xceeded The error message shown when MAX_EXECUTION_TIME hint causes a timeout incorrectly referred to "max_statement_time", which is a different variable with different units (seconds vs milliseconds). This could confuse users. Before: "Query execution was interrupted (max_statement_time exceeded)" After: "Query was interrupted: execution time limit 1.5 sec exceeded" The new message is neutral and accurate for both max_statement_time system variable and MAX_EXECUTION_TIME optimizer hint, displaying the actual timeout value that was exceeded.
Olernov
force-pushed
the
12.2-MDEV-35880-max-time
branch
from
21eda5d to
9e49451
Compare
mysys/thr_timer.c
Outdated
|
|
||
| DBUG_ASSERT(timer_data->expired == 1); | ||
|
|
||
| timer_data->period= micro_seconds; |
There was a problem hiding this comment.
As far as I understand, this is done to remember how large the timeout value was.
But I also see this in mysys/thr_timer.c :
is_periodic= timer_data->period != 0;So, this patch also makes the timer to be "periodic", right? That is, it will fire every N microseconds. While before the patch it would fire only once. I'm not sure if this can have any impact on how/what executes. Any idea?
One thing that doesn't look nice is that timer_data->period now has two meanings... (if my above statements are correct).
There was a problem hiding this comment.
Good point, not the best idea to intermingle with the periodic logic. See the next commit introducing a separate variable for this
| but not for scientific notation | ||
| */ | ||
| if (!strchr(timeout_str, '.') && !strchr(timeout_str, 'e')) | ||
| len+= my_snprintf(timeout_str + len, sizeof(timeout_str) - len, ".0"); |
There was a problem hiding this comment.
This doesn't ever print "NaN" or "Inf" does it?
There was a problem hiding this comment.
That should not occur: timeout is ulonglong from user-set timeout values (which are already checked for valid ranges). Division by 1000000.0 always produces a valid positive double.
…xceeded
The error message shown when MAX_EXECUTION_TIME hint causes a timeout incorrectly referred to "max_statement_time", which is a different variable with different units (seconds vs milliseconds). This could confuse users.
Before: "Query execution was interrupted (max_statement_time exceeded)"
After: "Query was interrupted: execution time limit 1.5 sec exceeded"
The new message is neutral and accurate for both max_statement_time system variable and MAX_EXECUTION_TIME optimizer hint, displaying the actual timeout value that was exceeded.