Rearm statement_timeout after each executed query.

Previously statement_timeout, in the extended protocol, affected all
messages till a Sync message.  For clients that pipeline/batch query
execution that's problematic.

Instead disable timeout after each Execute message, and enable, if
necessary, the timer in start_xact_command(). As that's done only for
Execute and not Parse / Bind, pipelining the latter two could still
cause undesirable timeouts. But a survey of protocol implementations
shows that all drivers issue Sync messages when preparing, and adding
timeout rearming to both is fairly expensive for the common parse /
bind / execute sequence.

Author: Tatsuo Ishii, editorialized by Andres Freund
Reviewed-By: Takayuki Tsunakawa, Andres Freund
Discussion: https://postgr.es/m/20170222.115044.1665674502985097185.t-ishii@sraoss.co.jp
This commit is contained in:
Andres Freund 2017-09-18 19:36:44 -07:00
parent 0fb9e4ace5
commit f8e5f156b3
1 changed files with 65 additions and 12 deletions

View File

@ -143,6 +143,11 @@ 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
@ -182,6 +187,8 @@ static bool IsTransactionExitStmtList(List *pstmts);
static bool IsTransactionStmtList(List *pstmts);
static void drop_unnamed_stmt(void);
static void log_disconnections(int code, Datum arg);
static void enable_statement_timeout(void);
static void disable_statement_timeout(void);
/* ----------------------------------------------------------------
@ -1241,7 +1248,8 @@ exec_parse_message(const char *query_string, /* string to execute */
/*
* Start up a transaction command so we can run parse analysis etc. (Note
* that this will normally change current memory context.) Nothing happens
* if we are already in one.
* if we are already in one. This also arms the statement timeout if
* necessary.
*/
start_xact_command();
@ -1529,7 +1537,8 @@ exec_bind_message(StringInfo input_message)
/*
* Start up a transaction command so we can call functions etc. (Note that
* this will normally change current memory context.) Nothing happens if
* we are already in one.
* we are already in one. This also arms the statement timeout if
* necessary.
*/
start_xact_command();
@ -2021,6 +2030,9 @@ exec_execute_message(const char *portal_name, long max_rows)
* those that start or end a transaction block.
*/
CommandCounterIncrement();
/* full command has been executed, reset timeout */
disable_statement_timeout();
}
/* Send appropriate CommandComplete to client */
@ -2450,25 +2462,27 @@ start_xact_command(void)
{
StartTransactionCommand();
/* Set statement timeout running, if any */
/* NB: this mustn't be enabled until we are within an xact */
if (StatementTimeout > 0)
enable_timeout_after(STATEMENT_TIMEOUT, StatementTimeout);
else
disable_timeout(STATEMENT_TIMEOUT, false);
xact_started = true;
}
/*
* Start statement timeout if necessary. Note that this'll intentionally
* not reset the clock on an already started timeout, to avoid the timing
* overhead when start_xact_command() is invoked repeatedly, without an
* interceding finish_xact_command() (e.g. parse/bind/execute). If that's
* not desired, the timeout has to be disabled explicitly.
*/
enable_statement_timeout();
}
static void
finish_xact_command(void)
{
/* cancel active statement timeout after each command */
disable_statement_timeout();
if (xact_started)
{
/* Cancel any active statement timeout before committing */
disable_timeout(STATEMENT_TIMEOUT, false);
CommitTransactionCommand();
#ifdef MEMORY_CONTEXT_CHECKING
@ -4537,3 +4551,42 @@ log_disconnections(int code, Datum arg)
port->user_name, port->database_name, port->remote_host,
port->remote_port[0] ? " port=" : "", port->remote_port)));
}
/*
* Start statement timeout timer, if enabled.
*
* If there's already a timeout running, don't restart the timer. That
* enables compromises between accuracy of timeouts and cost of starting a
* timeout.
*/
static void
enable_statement_timeout(void)
{
/* must be within an xact */
Assert(xact_started);
if (StatementTimeout > 0)
{
if (!stmt_timeout_active)
{
enable_timeout_after(STATEMENT_TIMEOUT, StatementTimeout);
stmt_timeout_active = true;
}
}
else
disable_timeout(STATEMENT_TIMEOUT, false);
}
/*
* Disable statement timeout, if active.
*/
static void
disable_statement_timeout(void)
{
if (stmt_timeout_active)
{
disable_timeout(STATEMENT_TIMEOUT, false);
stmt_timeout_active = false;
}
}