Re: [PATCH] Fix PITR pause bypass when initial XLOG_RUNNING_XACTS has subxid overflow

On Mon, Oct 26, 2026 at 9:45 PM Jan Nidzwetzki <jan(at)planetscale(dot)com> wrote:
>
> Hi Hackers,
>
> Attached is version 5 of the patch. It's a rebase against the
> current master branch, with no functional changes.

Thanks for updating the patch!

+ if (!LocalHotStandbyActive && !LocalPromoteIsTriggered)

Do we need to use !CheckForStandbyTrigger() instead of !LocalPromoteIsTriggered?
This would also handle the case where a promotion request has arrived but
LocalPromoteIsTriggered has not yet been set.

+ errdetail("The standby snapshots stayed overflowed and the
transactions responsible did not finish, so hot standby never
started."),
+ errhint("Set \"recovery_target_action\" to \"promote\", or choose a
recovery target at which hot standby can be enabled.")));

Are overflowed standby snapshots the only cause of this situation?
If not, mentioning them in the detail message could be misleading.

+ Likewise, if hot standby could not be started before the recovery
+ target was reached, for example because the replayed standby
+ snapshots remained overflowed, a setting of <literal>pause</literal>
+ will act the same as <literal>shutdown</literal>.

We should clarify that this applies only when recovery has already reached
a consistent state? If the recovery target is reached before that,
the preceding check raises FATAL regardless of recovery_target_action.
As currently worded, this could be read as covering that case too.

For example, how about the following description?

Likewise, if recovery has reached a consistent state but hot standby
has not yet become active when the recovery target is reached,
a setting of pause will act the same as shutdown. This can happen,
for example, when the replayed standby snapshots remain overflowed.

+# to pause and shut down instead of promoting. The server stops on its own, so
+# we drive it with pg_ctl directly (a regular ->start() would error out because
+# the server never becomes ready to accept connections).
+run_log(
+ [
+ 'pg_ctl',
+ '--pgdata' => $node_standby->data_dir,
+ '--log' => $node_standby->logfile,
+ 'start',
+ ]);

How about using $node_standby->start(fail_ok => 1) instead of invoking pg_ctl
directly? This would allow the expected startup failure while tracking
the postmaster PID, so teardown_node() could stop the server
if it unexpectedly promotes. If we do this, the comment needs to be updated
accordingly.

+my $until_lsn =
+ $node_primary->safe_psql('postgres', 'SELECT pg_current_wal_lsn()');

We can simplify this to "my $until_lsn = $node_primary->lsn('write');".

Regards,

--
Fujii Masao