Re: Statement timeout behavior in extended queries

From: "Tsunakawa, Takayuki" <tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com>
To: 'Tatsuo Ishii' <ishii(at)sraoss(dot)co(dot)jp>, "david(at)fetter(dot)org" <david(at)fetter(dot)org>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Statement timeout behavior in extended queries
Date: 2017-04-03 07:00:13
Message-ID: 0A3221C70F24FB45833433255569204D1F6BF355@G01JPEXMBYT05
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


I've reviewed the patch. My understanding is as follows. Please correct me if I'm wrong.

* The difference is that the Execute message stops the statement_timeout timer, so that the execution of one statement doesn't shorten the timeout period of subsequent statements when they are run in batch followed by a single Sync message.

* This patch is also necessary (or desirable) for the feature "Pipelining/batch mode support for libpq," which is being developed for PG 10. This patch enables correct timeout handling for each statement execution in a batch. Without this patch, the entire batch of statements is subject to statement_timeout. That's different from what the manual describes about statement_timeout. statement_timeout should measure execution of each statement.

Below are what I found in the patch.

+static bool st_timeout = false;

I think the variable name would better be stmt_timeout_enabled or stmt_timer_started, because other existing names use stmt to abbreviate statement, and thhis variable represents a state (see xact_started for example.)

+static void enable_statement_timeout(void)

+static void disable_statement_timeout(void)

"static void" and the remaining line should be on different lines, as other functions do.

+ /*
+ * Sanity check
+ */
+ if (!xact_started)
+ {
+ ereport(ERROR,
+ errmsg("Transaction must have been already started to set statement timeout")));
+ }

I think this fragment can be deleted, because enable/disable_timeout() is used only at limited places in postgres.c, so I don't see the chance of misuse.

Takayuki Tsunakawa

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Banck 2017-04-03 07:17:44 Re: gitlab post-mortem: pg_basebackup waiting for checkpoint
Previous Message Ashutosh Bapat 2017-04-03 06:49:30 Re: Foreign tables don't enforce the partition constraint