Thread: Re: Statement timeout behavior in extended queries
> The patch doesn't seem to behave like that. The Parse message calls start_xact_command() -> enable_statement_timeout()-> enable_timeout(), and set stmt_timer_started to true. Subsequent Bind and Execute messagescall enable_statement_timeout(), but enable_statement_timeout() doesn't call enable_timeout() because stmt_timer_startedis already true. > > >> > It looks like the patch does the following. I think this is desirable, >> because starting and stopping the timer for each message may be costly as >> Tom said. >> > Parse(statement1) >> > start timer >> > Bind(statement1, portal1) >> > Execute(portal1) >> > stop timer >> > Sync > > I ran one INSERT statement using JDBC with log_min_messages = DEBUG3, and the server log shows what I said. > > DEBUG: parse <unnamed>: insert into a values(2) > DEBUG: Set statement timeout > LOG: duration: 0.431 ms parse <unnamed>: insert into a values(2) > DEBUG: bind <unnamed> to <unnamed> > LOG: duration: 0.127 ms bind <unnamed>: insert into a values(2) > DEBUG: Disable statement timeout > LOG: duration: 0.184 ms execute <unnamed>: insert into a values(2) > DEBUG: snapshot of 1+0 running transaction ids (lsn 0/163BF28 oldest xid 561 latest complete 560 next xid 562) Check. >> This doesn't work in general use cases. Following pattern appears frequently >> in applications. >> >> Parse(statement1) >> Bind(statement1, portal1) >> Execute(portal1) >> Bind(statement1, portal1) >> Execute(portal1) >> : >> : >> Sync > > It works. The first Parse-Bind-Execute is measured as one unit, then subsequent Bind-Execute pairs are measured as otherunits. That's because each Execute ends the statement_timeout timer and the Bind message starts it again. I thinkthis is desirable, so the current patch looks good. May I mark this as ready for committer? FYI, make check-worldpassed successfully. It's too late. Someone has already moved the patch to the next CF (for PostgreSQL 11). >> Also what would happen if client just send a parse message and does nothing >> after that? > > It's correct to trigger the statement timeout in this case, because the first SQL statement started (with the Parse message)and its execution is not finished (with Execute message.) > > > Regards > Takayuki Tsunakawa > > >
From: Tatsuo Ishii [mailto:ishii@sraoss.co.jp] > It's too late. Someone has already moved the patch to the next CF (for > PostgreSQL 11). Yes, but this patch will be necessary by the final release of PG 10 if the libpq batch/pipelining is committed in PG 10. I marked this as ready for committer in the next CF, so that some committer can pick up this patch and consider putting itin PG 10. If you decide to modify the patch, please change the status. Regards Takayuki Tsunakawa
On 2017-04-04 06:18:04 +0000, Tsunakawa, Takayuki wrote: > From: Tatsuo Ishii [mailto:ishii@sraoss.co.jp] > > It's too late. Someone has already moved the patch to the next CF (for > > PostgreSQL 11). > > Yes, but this patch will be necessary by the final release of PG 10 if the libpq batch/pipelining is committed in PG 10. > > I marked this as ready for committer in the next CF, so that some committer can pick up this patch and consider puttingit in PG 10. If you decide to modify the patch, please change the status. Given the concern raised in http://archives.postgresql.org/message-id/12207.1491228316%40sss.pgh.pa.us I don't see this being ready for committer. - Andres
From: Andres Freund [mailto:andres@anarazel.de] Given the concern raised in > http://archives.postgresql.org/message-id/12207.1491228316%40sss.pgh.p > a.us > I don't see this being ready for committer. If what Tatsuo-san said to Tom is correct (i.e. each Parse/Bind/Execute starts and stops the timer), then it's a concernand the patch should not be ready for committer. However, the current patch is not like that -- it seems to do whatothers in this thread are expecting. Regards Takayuki Tsunakawa
On 2017-04-04 06:35:00 +0000, Tsunakawa, Takayuki wrote: > From: Andres Freund [mailto:andres@anarazel.de] > Given the concern raised in > > http://archives.postgresql.org/message-id/12207.1491228316%40sss.pgh.p > > a.us > > I don't see this being ready for committer. > > If what Tatsuo-san said to Tom is correct (i.e. each Parse/Bind/Execute starts and stops the timer), then it's a concernand the patch should not be ready for committer. However, the current patch is not like that -- it seems to do whatothers in this thread are expecting. Oh, interesting - I kind of took the author's statement as, uh, authoritative ;). A quick look over the patch confirms your understanding. 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. Unfortunately I can't move the patch back to the current CF, but I guess we can just mark it as committed in the next. - Andres
>> If what Tatsuo-san said to Tom is correct (i.e. each Parse/Bind/Execute starts and stops the timer), then it's a concernand the patch should not be ready for committer. However, the current patch is not like that -- it seems to do whatothers in this thread are expecting. > > Oh, interesting - I kind of took the author's statement as, uh, > authoritative ;). A quick look over the patch confirms your > understanding. Yes, Tsunakawa-san is correct. Sorry for confusion. > 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. > Unfortunately I can't move the patch back to the current CF, but I guess > we can just mark it as committed in the next. That will be great. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
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);staticvoid 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 onwhether 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 morerows 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 enda 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 beforecommitting */ - 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; + } +}
On 2017-04-04 16:10:32 +0900, Tatsuo Ishii wrote: > >> If what Tatsuo-san said to Tom is correct (i.e. each Parse/Bind/Execute starts and stops the timer), then it's a concernand the patch should not be ready for committer. However, the current patch is not like that -- it seems to do whatothers in this thread are expecting. > > > > Oh, interesting - I kind of took the author's statement as, uh, > > authoritative ;). A quick look over the patch confirms your > > understanding. > > Yes, Tsunakawa-san is correct. Sorry for confusion. > > > 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. Hm. I started to edit it, but I'm halfway coming back to my previous view that this isn't necessarily ready. If a client were to to prepare a large number of prepared statements (i.e. a lot of parse messages), this'll only start the timeout once, at the first statement sent. It's not an issue for libpq which sends a sync message after each PQprepare, nor does it look to be an issue for pgjdbc based on a quick look. Does anybody think there might be a driver out there that sends a bunch of 'parse' messages, without syncs? - Andres
On 2017-04-05 08:34:43 +0900, Tatsuo Ishii wrote: > 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). I'm working on the patch, and I've edited it more heavily, so please hold off. Changes: I don't think the debugging statements are a good idea, the !xact_started should be an assert, and disable_timeout should be called from within enable_statement_timeout independent of stmt_timer_started. But more importantly I had just sent a question that I think merits discussion. - Andres
On 2017-04-04 16:38:53 -0700, Andres Freund wrote: > On 2017-04-04 16:10:32 +0900, Tatsuo Ishii wrote: > > >> If what Tatsuo-san said to Tom is correct (i.e. each Parse/Bind/Execute starts and stops the timer), then it's a concernand the patch should not be ready for committer. However, the current patch is not like that -- it seems to do whatothers in this thread are expecting. > > > > > > Oh, interesting - I kind of took the author's statement as, uh, > > > authoritative ;). A quick look over the patch confirms your > > > understanding. > > > > Yes, Tsunakawa-san is correct. Sorry for confusion. > > > > > 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. > > Hm. I started to edit it, but I'm halfway coming back to my previous > view that this isn't necessarily ready. > > If a client were to to prepare a large number of prepared statements > (i.e. a lot of parse messages), this'll only start the timeout once, at > the first statement sent. It's not an issue for libpq which sends a > sync message after each PQprepare, nor does it look to be an issue for > pgjdbc based on a quick look. > > Does anybody think there might be a driver out there that sends a bunch > of 'parse' messages, without syncs? Looks to me like npgsql doesn't do that either. None of libpq, pgjdbs and npgsql doing it seems like some evidence that it's ok. - Andres
From: Andres Freund [mailto:andres@anarazel.de] > Looks to me like npgsql doesn't do that either. None of libpq, pgjdbs and > npgsql doing it seems like some evidence that it's ok. And psqlODBC now uses always libpq. Now time for final decision? Regards Takayuki Tsunakawa
On 2017-04-04 23:52:28 +0000, Tsunakawa, Takayuki wrote: > From: Andres Freund [mailto:andres@anarazel.de] > > Looks to me like npgsql doesn't do that either. None of libpq, pgjdbs and > > npgsql doing it seems like some evidence that it's ok. > > And psqlODBC now uses always libpq. > > Now time for final decision? I'll send an updated patch in a bit, and then will wait till tomorrow to push. Giving others a chance to chime in seems fair. - Andres
> Hm. I started to edit it, but I'm halfway coming back to my previous > view that this isn't necessarily ready. > > If a client were to to prepare a large number of prepared statements > (i.e. a lot of parse messages), this'll only start the timeout once, at > the first statement sent. It's not an issue for libpq which sends a > sync message after each PQprepare, nor does it look to be an issue for > pgjdbc based on a quick look. > > Does anybody think there might be a driver out there that sends a bunch > of 'parse' messages, without syncs? What's your point of the question? What kind of problem do you expect if the timeout starts only once at the first parse meesage out of bunch of parse messages? Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
On 2017-04-05 10:05:19 +0900, Tatsuo Ishii wrote: > > Hm. I started to edit it, but I'm halfway coming back to my previous > > view that this isn't necessarily ready. > > > > If a client were to to prepare a large number of prepared statements > > (i.e. a lot of parse messages), this'll only start the timeout once, at > > the first statement sent. It's not an issue for libpq which sends a > > sync message after each PQprepare, nor does it look to be an issue for > > pgjdbc based on a quick look. > > > > Does anybody think there might be a driver out there that sends a bunch > > of 'parse' messages, without syncs? > > What's your point of the question? What kind of problem do you expect > if the timeout starts only once at the first parse meesage out of > bunch of parse messages? It's perfectly valid to send a lot of Parse messages without interspersed Sync or Bind/Execute message. There'll be one timeout covering all of those Parse messages, which can thus lead to a timeout, even though nothing actually takes long individually. Greetings, Andres Freund
>> What's your point of the question? What kind of problem do you expect >> if the timeout starts only once at the first parse meesage out of >> bunch of parse messages? > > It's perfectly valid to send a lot of Parse messages without > interspersed Sync or Bind/Execute message. There'll be one timeout > covering all of those Parse messages, which can thus lead to a timeout, > even though nothing actually takes long individually. Hmm. IMO, that could happen even with the current statement timeout implementation as well. Or we could start/stop the timeout in exec_execute_message() only. This could avoid the problem above. Also this is more consistent with log_duration/log_min_duration_statement behavior than now. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
On 2017-04-04 16:56:26 -0700, 'Andres Freund' wrote: > On 2017-04-04 23:52:28 +0000, Tsunakawa, Takayuki wrote: > > From: Andres Freund [mailto:andres@anarazel.de] > > > Looks to me like npgsql doesn't do that either. None of libpq, pgjdbs and > > > npgsql doing it seems like some evidence that it's ok. > > > > And psqlODBC now uses always libpq. > > > > Now time for final decision? > > I'll send an updated patch in a bit, and then will wait till tomorrow to > push. Giving others a chance to chime in seems fair. Attached. I did not like that the previous patch had the timeout handling duplicated in the individual functions, I instead centralized it into start_xact_command(). Given that it already activated the timeout in the most common cases, that seems to make more sense to me. In your version we'd have called enable_statement_timeout() twice consecutively (which behaviourally is harmless). What do you think? I've not really tested this with the extended protocol, so I'd appreciate if you could rerun your test from the older thread. - Andres
> On 2017-04-04 16:56:26 -0700, 'Andres Freund' wrote: >> On 2017-04-04 23:52:28 +0000, Tsunakawa, Takayuki wrote: >> > From: Andres Freund [mailto:andres@anarazel.de] >> > > Looks to me like npgsql doesn't do that either. None of libpq, pgjdbs and >> > > npgsql doing it seems like some evidence that it's ok. >> > >> > And psqlODBC now uses always libpq. >> > >> > Now time for final decision? >> >> I'll send an updated patch in a bit, and then will wait till tomorrow to >> push. Giving others a chance to chime in seems fair. > > Attached. I did not like that the previous patch had the timeout > handling duplicated in the individual functions, I instead centralized > it into start_xact_command(). Given that it already activated the > timeout in the most common cases, that seems to make more sense to > me. In your version we'd have called enable_statement_timeout() twice > consecutively (which behaviourally is harmless). > > What do you think? I've not really tested this with the extended > protocol, so I'd appreciate if you could rerun your test from the > older thread. Ok, I will look into your patch and test it out using pgproto. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
From: pgsql-hackers-owner@postgresql.org > [mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Tatsuo Ishii > Hmm. IMO, that could happen even with the current statement timeout > implementation as well. > > Or we could start/stop the timeout in exec_execute_message() only. This > could avoid the problem above. Also this is more consistent with > log_duration/log_min_duration_statement behavior than now. I think it's better to include Parse and Bind as now. Parse or Bind could take long time when the table has many partitions,the query is complex and/or very long, some DDL statement is running against a target table, or the system loadis high. Firing statement timeout during or after many Parses is not a problem, because the first Parse started running some statementand it's not finished. Plus, Andres confirmed that major client drivers don't use such a pattern. There may bean odd behavior purely from the perspective of E.Q.P, that's a compromise, which Andres meant by "not perfect but." Regards Takayuki Tsunakawa
Andres Freund <andres@anarazel.de> writes:
> On 2017-04-05 10:05:19 +0900, Tatsuo Ishii wrote:
>> What's your point of the question? What kind of problem do you expect
>> if the timeout starts only once at the first parse meesage out of
>> bunch of parse messages?
> It's perfectly valid to send a lot of Parse messages without
> interspersed Sync or Bind/Execute message. There'll be one timeout
> covering all of those Parse messages, which can thus lead to a timeout,
> even though nothing actually takes long individually.
It might well be reasonable to redefine statement_timeout as limiting the
total time from the first client input to the response to Sync ... but
if that's what we're doing, let's make sure we do it consistently.
I haven't read the patch, but the comments in this thread make me fear
that it's introducing some ad-hoc, inconsistent behavior.
regards, tom lane
From: pgsql-hackers-owner@postgresql.org > [mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of 'Andres Freund' > Attached. I did not like that the previous patch had the timeout handling > duplicated in the individual functions, I instead centralized it into > start_xact_command(). Given that it already activated the timeout in the > most common cases, that seems to make more sense to me. In your version > we'd have called enable_statement_timeout() twice consecutively (which > behaviourally is harmless). > > What do you think? I've not really tested this with the extended protocol, > so I'd appreciate if you could rerun your test from the older thread. The patch looks good and cleaner. It looks like the code works as expected. As before, I ran one INSERT statement withPgJDBC, with gdb's breakpoints set on enable_timeout() and disable_timeout(). I confirmed that enable_timeout() is calledjust once at Parse message, and disable_timeout() is called just once at Execute message. I'd like to wait for Tatsuo-san's thorough testing with pgproto. Regards Takayuki Tsunakawa
On 2017-04-05 00:39:51 -0400, Tom Lane wrote: > Andres Freund <andres@anarazel.de> writes: > > On 2017-04-05 10:05:19 +0900, Tatsuo Ishii wrote: > >> What's your point of the question? What kind of problem do you expect > >> if the timeout starts only once at the first parse meesage out of > >> bunch of parse messages? > > > It's perfectly valid to send a lot of Parse messages without > > interspersed Sync or Bind/Execute message. There'll be one timeout > > covering all of those Parse messages, which can thus lead to a timeout, > > even though nothing actually takes long individually. > > It might well be reasonable to redefine statement_timeout as limiting the > total time from the first client input to the response to Sync ... That's actually the current behaviour. start_xact_command arms the timer in the !xact_started case, and only finish_xact_command() disarms it. finish_xact_command() is issued for Sync messsages and simple statements. I brought the case up because what we're discussing is changing things so Execute resets the clock (more precisely disables it, and have it reenabled later). So it'd be "fixed" for pipelined Execute statements, but not for pipelined Parses. Changing things so that Parse/Bind also reset the timing would increase overhead - and it'd not be useful in practice, because Sync's are sent by the drivers that support pipelining when statements are prepared. > but > if that's what we're doing, let's make sure we do it consistently. > I haven't read the patch, but the comments in this thread make me fear > that it's introducing some ad-hoc, inconsistent behavior. I'm a bit worried too due to the time constraints here, but I think resetting the clock at Execute too actually makes a fair amount sense. Greetings, Andres Freund
Andres Freund <andres@anarazel.de> writes:
> On 2017-04-05 00:39:51 -0400, Tom Lane wrote:
>> but
>> if that's what we're doing, let's make sure we do it consistently.
>> I haven't read the patch, but the comments in this thread make me fear
>> that it's introducing some ad-hoc, inconsistent behavior.
> I'm a bit worried too due to the time constraints here, but I think
> resetting the clock at Execute too actually makes a fair amount sense.
Meh. Two days before feature freeze is not the time to be introducing
a rushed redefinition of the wire protocol --- and let's not fool
ourselves, that is what this amounts to. Let's push this out to v11
and think about it more carefully.
regards, tom lane
On 2017-04-05 09:46:31 -0400, Tom Lane wrote: > Andres Freund <andres@anarazel.de> writes: > > On 2017-04-05 00:39:51 -0400, Tom Lane wrote: > >> but > >> if that's what we're doing, let's make sure we do it consistently. > >> I haven't read the patch, but the comments in this thread make me fear > >> that it's introducing some ad-hoc, inconsistent behavior. > > > I'm a bit worried too due to the time constraints here, but I think > > resetting the clock at Execute too actually makes a fair amount sense. > > Meh. Two days before feature freeze is not the time to be introducing > a rushed redefinition of the wire protocol --- and let's not fool > ourselves, that is what this amounts to. Let's push this out to v11 > and think about it more carefully. What I was trying to say is that I think the change makes sense and isn't particularly ad-hoc, but that I don't think we need to force this for v10. Greetings, Andres Freund