pgsql: Additional tests for subtransactions in recovery

Started by Simon Riggsover 9 years ago7 messageshackerscomitters
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

won't retrysuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t36596
psql -h localhost -U postgres

Built from patchset v4 (message #4), July 28, 2026 at 05:55 AM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t36596_4 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t36596_4 && git checkout t36596_4

Patchset v4 (message #4) is on t36596_4

Jump to latest
#1Simon Riggs
simon@2ndQuadrant.com
hackerscomitters

Additional tests for subtransactions in recovery

Tests for normal and prepared transactions

Author: Nikhil Sontakke, placed in new test file by me

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/0352c15e5ab4f70c3ab2ac1d48d9f38a5dd11786

Modified Files
--------------
src/test/recovery/t/009_twophase.pl | 39 +-----
src/test/recovery/t/012_subtransactions.pl | 197 +++++++++++++++++++++++++++++
2 files changed, 198 insertions(+), 38 deletions(-)

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#2Michael Paquier
michael@paquier.xyz
In reply to: Simon Riggs (#1)
hackerscomitters
Re: pgsql: Additional tests for subtransactions in recovery

On Thu, Apr 27, 2017 at 9:28 PM, Simon Riggs <simon@2ndquadrant.com> wrote:

Additional tests for subtransactions in recovery

Tests for normal and prepared transactions

Author: Nikhil Sontakke, placed in new test file by me

+$node_slave->promote;
+$node_slave->poll_query_until('postgres',
+   "SELECT NOT pg_is_in_recovery()")
+  or die "Timed out while waiting for promotion of standby";

This reminds me that we should really switch PostgresNode::promote to
use the wait mode of pg_ctl promote, and remove all those polling
queries...
--
Michael

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#3Peter Eisentraut
peter_e@gmx.net
In reply to: Michael Paquier (#2)
hackerscomitters
pg_ctl wait exit code (was Re: [COMMITTERS] pgsql: Additional tests for subtransactions in recovery)

On 4/27/17 08:41, Michael Paquier wrote:

+$node_slave->promote;
+$node_slave->poll_query_until('postgres',
+   "SELECT NOT pg_is_in_recovery()")
+  or die "Timed out while waiting for promotion of standby";

This reminds me that we should really switch PostgresNode::promote to
use the wait mode of pg_ctl promote, and remove all those polling
queries...

I was going to say: This should all be obsolete already, because pg_ctl
promote waits by default.

However: Failure to complete promotion within the waiting time does not
lead to an error exit, so you will not get a failure if the promotion
does not finish. This is probably a mistake. Looking around pg_ctl, I
found that this was handled seemingly inconsistently in do_start(), but
do_stop() errors when it does not complete.

Possible patches for this attached.

Perhaps we need a separate exit code in pg_ctl to distinguish general
errors from did not finish within timeout?

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachments:

0001-pg_ctl-Make-failure-to-complete-operation-a-nonzero-.patchinvalid/octet-stream; name=0001-pg_ctl-Make-failure-to-complete-operation-a-nonzero-.patchDownload+13-3
0002-Remove-unnecessary-pg_is_in_recovery-calls-in-tests.patchinvalid/octet-stream; name=0002-Remove-unnecessary-pg_is_in_recovery-calls-in-tests.patchDownload+0-26
#4Peter Eisentraut
peter_e@gmx.net
In reply to: Peter Eisentraut (#3)
hackerscomitters
Re: pg_ctl wait exit code (was Re: [COMMITTERS] pgsql: Additional tests for subtransactions in recovery)

On 5/1/17 12:19, Peter Eisentraut wrote:

On 4/27/17 08:41, Michael Paquier wrote:

+$node_slave->promote;
+$node_slave->poll_query_until('postgres',
+   "SELECT NOT pg_is_in_recovery()")
+  or die "Timed out while waiting for promotion of standby";

This reminds me that we should really switch PostgresNode::promote to
use the wait mode of pg_ctl promote, and remove all those polling
queries...

I was going to say: This should all be obsolete already, because pg_ctl
promote waits by default.

However: Failure to complete promotion within the waiting time does not
lead to an error exit, so you will not get a failure if the promotion
does not finish. This is probably a mistake. Looking around pg_ctl, I
found that this was handled seemingly inconsistently in do_start(), but
do_stop() errors when it does not complete.

Possible patches for this attached.

Perhaps we need a separate exit code in pg_ctl to distinguish general
errors from did not finish within timeout?

I was going to hold this back for PG11, but since we're now doing some
other tweaks in pg_ctl, it might be useful to add this too. Thoughts?

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachments:

t36596_4
v2-0001-pg_ctl-Make-failure-to-complete-operation-a-nonze.patchtext/plain; charset=UTF-8; name=v2-0001-pg_ctl-Make-failure-to-complete-operation-a-nonze.patch; x-mac-creator=0; x-mac-type=0Download+13-3
v2-0002-Remove-unnecessary-pg_is_in_recovery-calls-in-tes.patchtext/plain; charset=UTF-8; name=v2-0002-Remove-unnecessary-pg_is_in_recovery-calls-in-tes.patch; x-mac-creator=0; x-mac-type=0Download+0-19
#5Michael Paquier
michael@paquier.xyz
In reply to: Peter Eisentraut (#4)
hackerscomitters
Re: pg_ctl wait exit code (was Re: [COMMITTERS] pgsql: Additional tests for subtransactions in recovery)

On Sat, Jul 1, 2017 at 4:47 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

On 5/1/17 12:19, Peter Eisentraut wrote:

However: Failure to complete promotion within the waiting time does not
lead to an error exit, so you will not get a failure if the promotion
does not finish. This is probably a mistake. Looking around pg_ctl, I
found that this was handled seemingly inconsistently in do_start(), but
do_stop() errors when it does not complete.

This inconsistency could be treated like a bug, though changing such
an old behavior in bacl-branches would be risky. So +1 for only HEAD
with such a change, and pg_ctl promote -w is new in 10.

Possible patches for this attached.

Perhaps we need a separate exit code in pg_ctl to distinguish general
errors from did not finish within timeout?

I would treat that as a separate item for 11, but that's as far as my
opinion goes. Per this link in pg_ctl.c the error code ought to be 4:
https://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB-Core-generic/iniscrptact.html

I was going to hold this back for PG11, but since we're now doing some
other tweaks in pg_ctl, it might be useful to add this too. Thoughts?

The use of 0 as exit code for the new promote -w if timeout is reached
looks like an open item to me. Cleaning up the pool queries after
promotion would be nice to see as well.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Peter Eisentraut
peter_e@gmx.net
In reply to: Michael Paquier (#5)
hackerscomitters
Re: Re: pg_ctl wait exit code (was Re: [COMMITTERS] pgsql: Additional tests for subtransactions in recovery)

On 7/2/17 20:28, Michael Paquier wrote:

I was going to hold this back for PG11, but since we're now doing some
other tweaks in pg_ctl, it might be useful to add this too. Thoughts?

The use of 0 as exit code for the new promote -w if timeout is reached
looks like an open item to me. Cleaning up the pool queries after
promotion would be nice to see as well.

committed

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Michael Paquier
michael@paquier.xyz
In reply to: Peter Eisentraut (#6)
hackerscomitters
Re: Re: pg_ctl wait exit code (was Re: [COMMITTERS] pgsql: Additional tests for subtransactions in recovery)

On Thu, Jul 6, 2017 at 2:41 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

On 7/2/17 20:28, Michael Paquier wrote:

I was going to hold this back for PG11, but since we're now doing some
other tweaks in pg_ctl, it might be useful to add this too. Thoughts?

The use of 0 as exit code for the new promote -w if timeout is reached
looks like an open item to me. Cleaning up the pool queries after
promotion would be nice to see as well.

committed

Thanks for finishing the cleanup.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers