Andres,
>> I think the code needs a few clarifying comments around this, but
>> otherwise seems good. Not restarting the timeout in those cases
>> obviously isn't entirely "perfect"/"correct", but a tradeoff - the
>> comments should note that.
>>
>> Tatsuo-san, do you want to change those, and push? I can otherwise.
>
> Andres,
>
> If you don't mind, could you please fix the comments and push it.
I have changed the comments as you suggested. If it's ok, I can push
the patch myself (today I have time to work on this).
Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index a2282058..1fd93359 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -149,6 +149,11 @@ static bool doing_extended_query_message = false;
static bool ignore_till_sync = false;
/*
+ * Flag to keep track of whether we have started statement timeout timer.
+ */
+static bool stmt_timer_started = false;
+
+/*
* If an unnamed prepared statement exists, it's stored here.
* We keep it separate from the hashtable kept by commands/prepare.c
* in order to reduce overhead for short-lived queries.
@@ -188,6 +193,8 @@ static bool IsTransactionStmtList(List *pstmts);
static void drop_unnamed_stmt(void);
static void SigHupHandler(SIGNAL_ARGS);
static void log_disconnections(int code, Datum arg);
+static void enable_statement_timeout(void);
+static void disable_statement_timeout(void);
/* ----------------------------------------------------------------
@@ -1239,6 +1246,15 @@ exec_parse_message(const char *query_string, /* string to execute */
start_xact_command();
/*
+ * Set statement timeout running if it's not already set. The timer will
+ * be reset in a subsequent execute message. Ideally the timer should be
+ * reset in this function so that each parse/bind/execute message catches
+ * the timeout, but it needs gettimeofday() call for each, which is too
+ * expensive.
+ */
+ enable_statement_timeout();
+
+ /*
* Switch to appropriate context for constructing parsetrees.
*
* We have two strategies depending on whether the prepared statement is
@@ -1526,6 +1542,15 @@ exec_bind_message(StringInfo input_message)
*/
start_xact_command();
+ /*
+ * Set statement timeout running if it's not already set. The timer will
+ * be reset in a subsequent execute message. Ideally the timer should be
+ * reset in this function so that each parse/bind/execute message catches
+ * the timeout, but it needs gettimeofday() call for each, which is too
+ * expensive.
+ */
+ enable_statement_timeout();
+
/* Switch back to message context */
MemoryContextSwitchTo(MessageContext);
@@ -1942,6 +1967,11 @@ exec_execute_message(const char *portal_name, long max_rows)
start_xact_command();
/*
+ * Set statement timeout running if it's not already set.
+ */
+ enable_statement_timeout();
+
+ /*
* If we re-issue an Execute protocol request against an existing portal,
* then we are only fetching more rows rather than completely re-executing
* the query from the start. atStart is never reset for a v3 portal, so we
@@ -2014,6 +2044,11 @@ exec_execute_message(const char *portal_name, long max_rows)
* those that start or end a transaction block.
*/
CommandCounterIncrement();
+
+ /*
+ * We need to reset statement timeout if already set.
+ */
+ disable_statement_timeout();
}
/* Send appropriate CommandComplete to client */
@@ -2443,14 +2478,10 @@ 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;
+
+ /* Set statement timeout running, if any */
+ enable_statement_timeout();
}
}
@@ -2460,7 +2491,7 @@ finish_xact_command(void)
if (xact_started)
{
/* Cancel any active statement timeout before committing */
- disable_timeout(STATEMENT_TIMEOUT, false);
+ disable_statement_timeout();
CommitTransactionCommand();
@@ -4511,3 +4542,53 @@ log_disconnections(int code, Datum arg)
port->user_name, port->database_name, port->remote_host,
port->remote_port[0] ? " port=" : "", port->remote_port)));
}
+
+/*
+ * Set statement timeout if any.
+ */
+static void
+enable_statement_timeout(void)
+{
+ if (!stmt_timer_started)
+ {
+ if (StatementTimeout > 0)
+ {
+
+ /*
+ * Sanity check
+ */
+ if (!xact_started)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("Transaction must have been already started to set statement timeout")));
+ }
+
+ ereport(DEBUG3,
+ (errmsg_internal("Set statement timeout")));
+
+ enable_timeout_after(STATEMENT_TIMEOUT, StatementTimeout);
+ stmt_timer_started = true;
+ }
+ else
+ disable_timeout(STATEMENT_TIMEOUT, false);
+ }
+}
+
+/*
+ * Reset statement timeout if any.
+ */
+static void
+disable_statement_timeout(void)
+{
+ if (stmt_timer_started)
+ {
+ ereport(DEBUG3,
+ (errmsg_internal("Disable statement timeout")));
+
+ /* Cancel any active statement timeout */
+ disable_timeout(STATEMENT_TIMEOUT, false);
+
+ stmt_timer_started = false;
+ }
+}
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
