pgsql: Rearm statement_timeout after each executed query.
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: /messages/by-id/20170222.115044.1665674502985097185.t-ishii@sraoss.co.jp
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/f8e5f156b30efee5d0038b03e38735773abcb7ed
Modified Files
--------------
src/backend/tcop/postgres.c | 77 ++++++++++++++++++++++++++++++++++++++-------
1 file changed, 65 insertions(+), 12 deletions(-)
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
On 9/18/17 22:41, Andres Freund wrote:
Rearm statement_timeout after each executed query.
This appears to have broken statement_timeout behavior in master such
that only every second query is affected by it. For example:
create table t1 as select * from generate_series(0, 100000000) as _(a);
set statement_timeout = '1s';
explain analyze select from t1 where a = 55; -- gets canceled
explain analyze select from t1 where a = 55; -- completes (>1s)
explain analyze select from t1 where a = 55; -- gets canceled
explain analyze select from t1 where a = 55; -- completes (>1s)
etc.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Tue, Feb 6, 2018 at 5:21 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
On 9/18/17 22:41, Andres Freund wrote:
Rearm statement_timeout after each executed query.
This appears to have broken statement_timeout behavior in master such
that only every second query is affected by it.
Yeah, I also just ran into this while testing a nearby complaint about
statement timeouts vs parallel query. In the error path
stmt_timeout_active remains true, so the next statement does nothing
in enable_statement_timeout(). I think we just need to clear that
flag in the error path, right where we call disable_all_timeouts().
See attached.
--
Thomas Munro
http://www.enterprisedb.com
Attachments:
fix-statement-timeout.patchapplication/octet-stream; name=fix-statement-timeout.patchDownload
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index ddc3ec860ae..eb3ca07df71 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -3913,6 +3913,9 @@ PostgresMain(int argc, char *argv[],
disable_all_timeouts(false);
QueryCancelPending = false; /* second to avoid race condition */
+ /* Forget about any active statement timeout. */
+ stmt_timeout_active = false;
+
/* Not reading from the client anymore. */
DoingCommandRead = false;
On Wed, Feb 7, 2018 at 7:04 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
On Tue, Feb 6, 2018 at 5:21 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:On 9/18/17 22:41, Andres Freund wrote:
Rearm statement_timeout after each executed query.
This appears to have broken statement_timeout behavior in master such
that only every second query is affected by it.Yeah, I also just ran into this while testing a nearby complaint about
statement timeouts vs parallel query. In the error path
stmt_timeout_active remains true, so the next statement does nothing
in enable_statement_timeout(). I think we just need to clear that
flag in the error path, right where we call disable_all_timeouts().
See attached.
Looks right. Committed, but I thought the comment was strange (forget
about?) so I just left that out.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On 2018-02-09 15:50:44 -0500, Robert Haas wrote:
On Wed, Feb 7, 2018 at 7:04 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:On Tue, Feb 6, 2018 at 5:21 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:On 9/18/17 22:41, Andres Freund wrote:
Rearm statement_timeout after each executed query.
This appears to have broken statement_timeout behavior in master such
that only every second query is affected by it.Yeah, I also just ran into this while testing a nearby complaint about
statement timeouts vs parallel query. In the error path
stmt_timeout_active remains true, so the next statement does nothing
in enable_statement_timeout(). I think we just need to clear that
flag in the error path, right where we call disable_all_timeouts().
See attached.Looks right. Committed, but I thought the comment was strange (forget
about?) so I just left that out.
Thanks Peter, Thomas, Robert!
- Andres