On Mon, Oct 26, 2026 at 9:45 PM Jan Nidzwetzki <[email protected]> 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


Reply via email to