Re: Disable startup progress timeout during standby WAL replay

> On Oct 7, 2026, at 12:54, Nitin Jadhav <nitinjadhavpostgres(at)gmail(dot)com> wrote:
>
>> Maybe I'm missing your point... Could you clarify what change you are
>> suggesting for EnableStandbyMode() itself?
>>
>>>> Wouldn't it be simpler to handle the standby case at the existing
>>>> check, like this?
>>>>
>>>> if (!StandbyMode)
>>>> begin_startup_progress_phase();
>>>> + else
>>>> + disable_startup_progress_timeout();
>>>
>>> I thought the same at first, too. But I just thought the timeout should not
>>> be enabled even while reading the first WAL record, so I placed
>>> the check before reading the first record.
>>
>> Similarly, if the timeout should be disabled before reading the first
>> WAL record, I wonder whether begin_startup_progress_phase() should
>> also be called at the same point.
>
> I think disable_startup_progress_timeout() is needed both in
> EnableStandbyMode() and near the beginning of PerformWalRecovery(),
> because the two calls cover different orderings. When standby mode is
> enabled during InitWalRecovery(), it happens before
> ResetUnloggedRelations(). Since ResetUnloggedRelations() calls
> begin_startup_progress_phase(), it can re-enable the timeout after
> EnableStandbyMode() has disabled it. Therefore, if StandbyMode is
> already true on entry to PerformWalRecovery(), the timeout needs to be
> disabled again before reading the first WAL record.
>
> On the other hand, recovery can initially start with StandbyMode ==
> false and later switch from crash recovery to archive/standby recovery
> after exhausting the WAL available in pg_wal. In that case,
> EnableStandbyMode() is called from ReadRecord(), and a WAL replay
> progress timeout may already be active. Therefore, the call in
> EnableStandbyMode() also needs to remain. This transition can
> potentially happen while trying to obtain the first replay record,
> before InRedo becomes true. Thus, making the call in
> EnableStandbyMode() conditional on replay having already started would
> not cover every case.
>
> Regarding the location of the new call, placing it before the first
> ReadRecord() looks correct to me. Reading the first record may involve
> restoring WAL from the archive or waiting for WAL to arrive. If the
> timeout was left active by ResetUnloggedRelations(), postponing the
> disable until after ReadRecord() would allow unnecessary periodic
> timeout activity throughout that wait.
>
> I do not think begin_startup_progress_phase() should be moved to the
> same location for symmetry. Its current placement after successfully
> obtaining the first record, near the "redo starts at ..." message,
> provides a natural boundary for the reported redo phase. Moving it
> before ReadRecord() would include time spent locating, restoring, or
> waiting for the first WAL record in the reported redo elapsed time.
> There is also no redo progress-reporting point inside that initial
> ReadRecord() call.
>
> Therefore, although the two disable calls look similar, they handle
> different transitions, and the code change in the patch looks correct
> to me.
>
> I have one minor suggestion for the documentation. The proposed wording:
>
>> On a standby server, however, it does not apply during WAL replay.
>
> might be slightly broader than the actual condition, because a server
> requested to become a standby can initially perform crash recovery
> with StandbyMode == false. Perhaps the following would map more
> directly to the implementation:
>
> "This setting is applied separately to each operation, such as WAL
> replay, syncing the data directory, and resetting unlogged relations.
> WAL replay progress is not reported while standby mode is active."
>
> With that minor documentation suggestion, the patch looks good to me.
>
> Best Regards,
> Nitin Jadhav
> Azure Database for PostgreSQL
> Microsoft
>
>

Hi!

Thanks for the patch. The problem is real, and I agree with Fujii's
analysis and Nitin's: the fix is correct, the placement is right, and
the call in EnableStandbyMode() still needs to stay.

I built master with and without the patch, with a temporary write() in
startup_progress_timeout_handler() to count firings, and
the timer re-armed by ResetUnloggedRelations() did keep firing for
the standby's whole lifetime, and the patch stops it.

I also checked the crash-recovery-first path (a crashed primary
restarted with standby.signal, no backup_label). There redo progress is
reported during the crash recovery part, and the timer is still running
when ReadRecord() calls EnableStandbyMode(). So the existing call in
EnableStandbyMode() is still needed; the new call only covers the case
where StandbyMode was already true when PerformWalRecovery() started.

Two non-blocking suggestions, no new patch needed for them:

- The comment says "earlier startup phases executed afterward", which
contradicts itself, and EnableStandbyMode() disables the timer, not
the logging (the !StandbyMode checks do that). Maybe:

EnableStandbyMode() disabled the startup progress timeout, but
startup operations performed since then, such as
ResetUnloggedRelations(), may have re-enabled it. Disable it again,
since recovery progress is not reported in standby mode.

- The doc sentence "On a standby server, however, it does not apply
during WAL replay" (and Nitin's "while standby mode is active") is
slightly off for the standby.signal-without-backup_label case: the
server logs "entering standby mode" and then still emits "redo in
progress" during the crash recovery part, because StandbyMode only
becomes true when ReadRecord() switches over. The exception only ever
adds messages and never hides any, so maybe a simple sentence is
safest, like:
"Progress of WAL replay is not reported on a standby
server."

Separately, while testing the conflict path I found a real bug next to
this one, and I'm attaching a small patch for it.

ResolveRecoveryConflictWithLock() and
ResolveRecoveryConflictWithBufferPin() both end with
disable_all_timeouts(false), under the assumption that the startup
process has no other timeouts. That stopped being true in PG 15, when
STARTUP_PROGRESS_TIMEOUT was added. So a single recovery conflict
cancels the progress timeout, and "redo in progress" goes quiet for the
rest of the redo, even though redo keeps going.

I reproduced it with PITR (recovery.signal, hot_standby=on): without a conflict
I got 24 messages of redo; with one lock conflict 7 messages, then silence
only. Nothing is lost but the log messages, so an admin
watching recovery might well think it is stuck. The attached patch
cancels only the two timeouts those functions enable; with it the same
run gives 24 messages.

That one is a separate and pre-existing: probably wants a backpatch
to 15. I exercised the lock-conflict path; the buffer-pin path is the
same pattern but I only read it, not ran it.

Regards,
Aidar Imamov