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

Commit22f6f2c

Browse files
committed
Improve management of statement timeouts.
Commitf8e5f15 added private state in postgres.c to track whethera statement timeout is running. This seems like bad design to me;timeout.c's private state should be the single source of truth aboutthat. We already fixed one bug associated with failure to keep thosestates in sync (cf.be42015), and I've got little faith that wewon't find more in future. So get rid of postgres.c's local variableby exposing a way to ask timeout.c whether a timeout is running.(Obviously, such an inquiry is subject to race conditions, but itseems fine for the purpose at hand.)To make get_timeout_active() as cheap as possible, add a flag inthe per-timeout struct showing whether that timeout is active.This allows some small savings elsewhere in timeout.c, mainlyelimination of unnecessary searches of the active_timeouts array.While at it, fix enable_statement_timeout to not call disable_timeoutwhen statement_timeout is 0 and the timeout is not running. Thisavoids a useless deschedule-and-reschedule-timeouts cycle, whichrepresents a significant savings (at least one kernel call) whenthere is any other active timeout. Right now, there usually isn't,but there are proposals around to change that.Discussion:https://postgr.es/m/16035-456e6e69ebfd4374@postgresql.org
1 parent2b2bacd commit22f6f2c

File tree

3 files changed

+39
-33
lines changed

3 files changed

+39
-33
lines changed

‎src/backend/tcop/postgres.c

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -145,11 +145,6 @@ static bool DoingCommandRead = false;
145145
staticbooldoing_extended_query_message= false;
146146
staticboolignore_till_sync= false;
147147

148-
/*
149-
* Flag to keep track of whether statement timeout timer is active.
150-
*/
151-
staticboolstmt_timeout_active= false;
152-
153148
/*
154149
* If an unnamed prepared statement exists, it's stored here.
155150
* We keep it separate from the hashtable kept by commands/prepare.c
@@ -4029,7 +4024,6 @@ PostgresMain(int argc, char *argv[],
40294024
*/
40304025
disable_all_timeouts(false);
40314026
QueryCancelPending= false;/* second to avoid race condition */
4032-
stmt_timeout_active= false;
40334027

40344028
/* Not reading from the client anymore. */
40354029
DoingCommandRead= false;
@@ -4711,14 +4705,14 @@ enable_statement_timeout(void)
47114705

47124706
if (StatementTimeout>0)
47134707
{
4714-
if (!stmt_timeout_active)
4715-
{
4708+
if (!get_timeout_active(STATEMENT_TIMEOUT))
47164709
enable_timeout_after(STATEMENT_TIMEOUT,StatementTimeout);
4717-
stmt_timeout_active= true;
4718-
}
47194710
}
47204711
else
4721-
disable_timeout(STATEMENT_TIMEOUT, false);
4712+
{
4713+
if (get_timeout_active(STATEMENT_TIMEOUT))
4714+
disable_timeout(STATEMENT_TIMEOUT, false);
4715+
}
47224716
}
47234717

47244718
/*
@@ -4727,10 +4721,6 @@ enable_statement_timeout(void)
47274721
staticvoid
47284722
disable_statement_timeout(void)
47294723
{
4730-
if (stmt_timeout_active)
4731-
{
4724+
if (get_timeout_active(STATEMENT_TIMEOUT))
47324725
disable_timeout(STATEMENT_TIMEOUT, false);
4733-
4734-
stmt_timeout_active= false;
4735-
}
47364726
}

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

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ typedef struct timeout_params
2727
{
2828
TimeoutIdindex;/* identifier of timeout reason */
2929

30-
/* volatile because it may be changed from the signal handler */
30+
/* volatile because these may be changed from the signal handler */
31+
volatileboolactive;/* true if timeout is in active_timeouts[] */
3132
volatileboolindicator;/* true if timeout has occurred */
3233

3334
/* callback function for timeout, or NULL if timeout not registered */
@@ -105,6 +106,9 @@ insert_timeout(TimeoutId id, int index)
105106
elog(FATAL,"timeout index %d out of range 0..%d",index,
106107
num_active_timeouts);
107108

109+
Assert(!all_timeouts[id].active);
110+
all_timeouts[id].active= true;
111+
108112
for (i=num_active_timeouts-1;i >=index;i--)
109113
active_timeouts[i+1]=active_timeouts[i];
110114

@@ -125,6 +129,9 @@ remove_timeout_index(int index)
125129
elog(FATAL,"timeout index %d out of range 0..%d",index,
126130
num_active_timeouts-1);
127131

132+
Assert(active_timeouts[index]->active);
133+
active_timeouts[index]->active= false;
134+
128135
for (i=index+1;i<num_active_timeouts;i++)
129136
active_timeouts[i-1]=active_timeouts[i];
130137

@@ -147,9 +154,8 @@ enable_timeout(TimeoutId id, TimestampTz now, TimestampTz fin_time)
147154
* If this timeout was already active, momentarily disable it. We
148155
* interpret the call as a directive to reschedule the timeout.
149156
*/
150-
i=find_active_timeout(id);
151-
if (i >=0)
152-
remove_timeout_index(i);
157+
if (all_timeouts[id].active)
158+
remove_timeout_index(find_active_timeout(id));
153159

154160
/*
155161
* Find out the index where to insert the new timeout. We sort by
@@ -349,6 +355,7 @@ InitializeTimeouts(void)
349355
for (i=0;i<MAX_TIMEOUTS;i++)
350356
{
351357
all_timeouts[i].index=i;
358+
all_timeouts[i].active= false;
352359
all_timeouts[i].indicator= false;
353360
all_timeouts[i].timeout_handler=NULL;
354361
all_timeouts[i].start_time=0;
@@ -524,8 +531,6 @@ enable_timeouts(const EnableTimeoutParams *timeouts, int count)
524531
void
525532
disable_timeout(TimeoutIdid,boolkeep_indicator)
526533
{
527-
inti;
528-
529534
/* Assert request is sane */
530535
Assert(all_timeouts_initialized);
531536
Assert(all_timeouts[id].timeout_handler!=NULL);
@@ -534,9 +539,8 @@ disable_timeout(TimeoutId id, bool keep_indicator)
534539
disable_alarm();
535540

536541
/* Find the timeout and remove it from the active list. */
537-
i=find_active_timeout(id);
538-
if (i >=0)
539-
remove_timeout_index(i);
542+
if (all_timeouts[id].active)
543+
remove_timeout_index(find_active_timeout(id));
540544

541545
/* Mark it inactive, whether it was active or not. */
542546
if (!keep_indicator)
@@ -571,13 +575,11 @@ disable_timeouts(const DisableTimeoutParams *timeouts, int count)
571575
for (i=0;i<count;i++)
572576
{
573577
TimeoutIdid=timeouts[i].id;
574-
intidx;
575578

576579
Assert(all_timeouts[id].timeout_handler!=NULL);
577580

578-
idx=find_active_timeout(id);
579-
if (idx >=0)
580-
remove_timeout_index(idx);
581+
if (all_timeouts[id].active)
582+
remove_timeout_index(find_active_timeout(id));
581583

582584
if (!timeouts[i].keep_indicator)
583585
all_timeouts[id].indicator= false;
@@ -595,6 +597,8 @@ disable_timeouts(const DisableTimeoutParams *timeouts, int count)
595597
void
596598
disable_all_timeouts(boolkeep_indicators)
597599
{
600+
inti;
601+
598602
disable_alarm();
599603

600604
/*
@@ -613,15 +617,26 @@ disable_all_timeouts(bool keep_indicators)
613617

614618
num_active_timeouts=0;
615619

616-
if (!keep_indicators)
620+
for (i=0;i<MAX_TIMEOUTS;i++)
617621
{
618-
inti;
619-
620-
for (i=0;i<MAX_TIMEOUTS;i++)
622+
all_timeouts[i].active= false;
623+
if (!keep_indicators)
621624
all_timeouts[i].indicator= false;
622625
}
623626
}
624627

628+
/*
629+
* Return true if the timeout is active (enabled and not yet fired)
630+
*
631+
* This is, of course, subject to race conditions, as the timeout could fire
632+
* immediately after we look.
633+
*/
634+
bool
635+
get_timeout_active(TimeoutIdid)
636+
{
637+
returnall_timeouts[id].active;
638+
}
639+
625640
/*
626641
* Return the timeout's I've-been-fired indicator
627642
*

‎src/include/utils/timeout.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ extern void disable_timeouts(const DisableTimeoutParams *timeouts, int count);
8080
externvoiddisable_all_timeouts(boolkeep_indicators);
8181

8282
/* accessors */
83+
externboolget_timeout_active(TimeoutIdid);
8384
externboolget_timeout_indicator(TimeoutIdid,boolreset_indicator);
8485
externTimestampTzget_timeout_start_time(TimeoutIdid);
8586
externTimestampTzget_timeout_finish_time(TimeoutIdid);

0 commit comments

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