Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit9dd4178

Browse files
committed
Be more predictable about reporting "lock timeout" vs "statement timeout".
If both timeout indicators are set when we arrive at ProcessInterrupts,we've historically just reported "lock timeout". However, some buildfarmmembers have been observed to fail isolationtester's timeouts test byreporting "lock timeout" when the statement timeout was expected to firefirst. The cause seems to be that the process is allowed to sleep longerthan expected (probably due to heavy machine load) so that the locktimeout happens before we reach the point of reporting the error, andthen this arbitrary tiebreak rule does the wrong thing. We can improvematters by comparing the scheduled timeout times to decide which errorto report.I had originally proposed greatly reducing the 1-second window betweenthe two timeouts in the test cases. On reflection that is a bad idea,at least for the case where the lock timeout is expected to fire first,because that would assume that it takes negligible time to get fromstatement start to the beginning of the lock wait. Thus, this patchdoesn't completely remove the risk of test failures on slow machines.Empirically, however, the case this handles is the one we are seeingin the buildfarm. The explanation may be that the other case requiresthe scheduler to take the CPU away from a busy process, whereas thecase fixed here only requires the scheduler to not give the CPU backright away to a process that has been woken from a multi-second sleep(and, perhaps, has been swapped out meanwhile).Back-patch to 9.3 where the isolationtester timeouts test was added.Discussion: <8693.1464314819@sss.pgh.pa.us>
1 parentd74048d commit9dd4178

File tree

3 files changed

+35
-5
lines changed

3 files changed

+35
-5
lines changed

‎src/backend/tcop/postgres.c

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2909,6 +2909,9 @@ ProcessInterrupts(void)
29092909

29102910
if (QueryCancelPending)
29112911
{
2912+
boollock_timeout_occurred;
2913+
boolstmt_timeout_occurred;
2914+
29122915
/*
29132916
* Don't allow query cancel interrupts while reading input from the
29142917
* client, because we might lose sync in the FE/BE protocol. (Die
@@ -2929,17 +2932,29 @@ ProcessInterrupts(void)
29292932

29302933
/*
29312934
* If LOCK_TIMEOUT and STATEMENT_TIMEOUT indicators are both set, we
2932-
*prefer toreport the former; but be sure to clear both.
2935+
*need toclear both, so always fetch both.
29332936
*/
2934-
if (get_timeout_indicator(LOCK_TIMEOUT, true))
2937+
lock_timeout_occurred=get_timeout_indicator(LOCK_TIMEOUT, true);
2938+
stmt_timeout_occurred=get_timeout_indicator(STATEMENT_TIMEOUT, true);
2939+
2940+
/*
2941+
* If both were set, we want to report whichever timeout completed
2942+
* earlier; this ensures consistent behavior if the machine is slow
2943+
* enough that the second timeout triggers before we get here. A tie
2944+
* is arbitrarily broken in favor of reporting a lock timeout.
2945+
*/
2946+
if (lock_timeout_occurred&&stmt_timeout_occurred&&
2947+
get_timeout_finish_time(STATEMENT_TIMEOUT)<get_timeout_finish_time(LOCK_TIMEOUT))
2948+
lock_timeout_occurred= false;/* report stmt timeout */
2949+
2950+
if (lock_timeout_occurred)
29352951
{
2936-
(void)get_timeout_indicator(STATEMENT_TIMEOUT, true);
29372952
LockErrorCleanup();
29382953
ereport(ERROR,
29392954
(errcode(ERRCODE_LOCK_NOT_AVAILABLE),
29402955
errmsg("canceling statement due to lock timeout")));
29412956
}
2942-
if (get_timeout_indicator(STATEMENT_TIMEOUT, true))
2957+
if (stmt_timeout_occurred)
29432958
{
29442959
LockErrorCleanup();
29452960
ereport(ERROR,

‎src/backend/utils/misc/timeout.c

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ typedef struct timeout_params
3434
timeout_handler_proctimeout_handler;
3535

3636
TimestampTzstart_time;/* time that timeout was last activated */
37-
TimestampTzfin_time;/*if active,time it is due to fire */
37+
TimestampTzfin_time;/* time it is, or was last, due to fire */
3838
}timeout_params;
3939

4040
/*
@@ -654,3 +654,17 @@ get_timeout_start_time(TimeoutId id)
654654
{
655655
returnall_timeouts[id].start_time;
656656
}
657+
658+
/*
659+
* Return the time when the timeout is, or most recently was, due to fire
660+
*
661+
* Note: will return 0 if timeout has never been activated in this process.
662+
* However, we do *not* reset the fin_time when a timeout occurs, so as
663+
* not to create a race condition if SIGALRM fires just as some code is
664+
* about to fetch the value.
665+
*/
666+
TimestampTz
667+
get_timeout_finish_time(TimeoutIdid)
668+
{
669+
returnall_timeouts[id].fin_time;
670+
}

‎src/include/utils/timeout.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,5 +82,6 @@ extern void disable_all_timeouts(bool keep_indicators);
8282
/* accessors */
8383
externboolget_timeout_indicator(TimeoutIdid,boolreset_indicator);
8484
externTimestampTzget_timeout_start_time(TimeoutIdid);
85+
externTimestampTzget_timeout_finish_time(TimeoutIdid);
8586

8687
#endif/* TIMEOUT_H */

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp