From 22f6f2c1ccb56e0d6a159d4562418587e4b10e01 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 25 Oct 2019 11:41:16 -0400 Subject: Improve management of statement timeouts. Commit f8e5f156b added private state in postgres.c to track whether a statement timeout is running. This seems like bad design to me; timeout.c's private state should be the single source of truth about that. We already fixed one bug associated with failure to keep those states in sync (cf. be42015fc), and I've got little faith that we won't find more in future. So get rid of postgres.c's local variable by exposing a way to ask timeout.c whether a timeout is running. (Obviously, such an inquiry is subject to race conditions, but it seems fine for the purpose at hand.) To make get_timeout_active() as cheap as possible, add a flag in the per-timeout struct showing whether that timeout is active. This allows some small savings elsewhere in timeout.c, mainly elimination of unnecessary searches of the active_timeouts array. While at it, fix enable_statement_timeout to not call disable_timeout when statement_timeout is 0 and the timeout is not running. This avoids a useless deschedule-and-reschedule-timeouts cycle, which represents a significant savings (at least one kernel call) when there 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 --- src/backend/tcop/postgres.c | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) (limited to 'src/backend/tcop/postgres.c') diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 1ecaba0d574..4bec40aa287 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -145,11 +145,6 @@ static bool DoingCommandRead = false; static bool doing_extended_query_message = false; static bool ignore_till_sync = false; -/* - * Flag to keep track of whether statement timeout timer is active. - */ -static bool stmt_timeout_active = false; - /* * If an unnamed prepared statement exists, it's stored here. * We keep it separate from the hashtable kept by commands/prepare.c @@ -4029,7 +4024,6 @@ PostgresMain(int argc, char *argv[], */ disable_all_timeouts(false); QueryCancelPending = false; /* second to avoid race condition */ - stmt_timeout_active = false; /* Not reading from the client anymore. */ DoingCommandRead = false; @@ -4711,14 +4705,14 @@ enable_statement_timeout(void) if (StatementTimeout > 0) { - if (!stmt_timeout_active) - { + if (!get_timeout_active(STATEMENT_TIMEOUT)) enable_timeout_after(STATEMENT_TIMEOUT, StatementTimeout); - stmt_timeout_active = true; - } } else - disable_timeout(STATEMENT_TIMEOUT, false); + { + if (get_timeout_active(STATEMENT_TIMEOUT)) + disable_timeout(STATEMENT_TIMEOUT, false); + } } /* @@ -4727,10 +4721,6 @@ enable_statement_timeout(void) static void disable_statement_timeout(void) { - if (stmt_timeout_active) - { + if (get_timeout_active(STATEMENT_TIMEOUT)) disable_timeout(STATEMENT_TIMEOUT, false); - - stmt_timeout_active = false; - } } -- cgit v1.2.3