Re: POC: enable logical decoding when wal_level = 'replica' without a server restart - Mailing list pgsql-hackers

On Wed, Nov 19, 2025 at 1:21 AM shveta malik <shveta.malik@gmail.com> wrote:
>
> On Wed, Nov 19, 2025 at 1:52 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Mon, Nov 17, 2025 at 10:49 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > On Tue, Nov 18, 2025 at 1:02 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > > >
> > > > On Sun, Nov 16, 2025 at 9:51 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > >
> > > > >
> > > > > In v26-0002-FIXUP-remove-status_change_allowed-flag, by using
> > > > > status_change_inprogress, we ensure that no backend is allowed to
> > > > > toggle the logical_wal/decoding status till startup process marks the
> > > > > recovery state as recovery_done. I am trying to think what problem
> > > > > this part of design prevents. I have considered the following
> > > > > scenarios:
> > > > >
> > > > > Scenario-1:
> > > > > 1. Startup process enables logical_wal and logical_decoding. Writes
> > > > > WAL record for it
> > > > > 2. Backend disables logical_decoding, writes WAL for it, and disables
> > > > > logical_wal.
> > > > > 3. Startup process sets recovery_done and allows wal_writes
> > > > >
> > > > > Say, instead of using status_change_inprogress to prevent doing
> > > > > step-2, if we had used recovery_in_progress kind of flag then how is
> > > > > it possible for backends to create any problem for the current node or
> > > > > cascaded standbys? I think the only way a problem can happen is if we
> > > > > write the WAL to disable_logical decoding after any backend could have
> > > > > written a non-logical WAL information record. Can that happen if we
> > > > > use the recovery_in_progress flag to prevent disable of logical_wal?
> > > > > If so, how?
> > > >
> > > > The main idea of holding status_change_inprogress until the recovery
> > > > end is to prevent concurrent toggling the logical decoding status. In
> > > > your scenario, IIUC backends cannot write any WAL yet at step-2 since
> > > > it's allowed at step-3. It would end up with a FATAL error actually.
> > > > One alternative is to make processes call LocalSetXLogInsertAllowed()
> > > > so that they can write WAL even during recovery, but I don't use it as
> > > > I'm concerned that it could lead to other problems. On the other hand,
> > > > we cannot let the backend to disable logical_decoding and logical_wal
> > > > without WAL warite at step-2 because otherwise the cascaded standby
> > > > won't disable logical decoding.
> > > >
> > >
> > > Why can't we postpone disabling logical WAL, decoding to the next
> > > cycle of checkpointer when RecoveryInProgress() is true without
> > > relying on status_change_inprogress? So, this will lead to a window
> > > where there are no logical slots but still the effective_wal_level is
> > > logical. However, this could be true even without considering this
> > > problem because the checkpointer can take some time to disable the
> > > logical WAL and decoding.
> > >
> > > The other problematic case to consider is during promotion, the
> > > startup has marked logical decoding as disabled but not yet marked
> > > recovery-done. Then the backend created a slot and returned without
> > > marking logical decoding as enabled due to relying on
> > > RecoveryInProgress(). Then the start-up marked Recovery-Done. Now we
> > > have a logical slot present, but logical decoding is disabled. I think
> > > we can simply disallow the creation of a logical slot in this window
> > > (where effective_wal_level is 'replica' and RecoveryInProgress() is
> > > true).
> >
> > It sounds reasonable. Backends are already prohibited from creating
> > logical slots when effective_wal_level is 'replica' and
> > RecoveryInProgress() is true, so it should not be a problem.
> >
> > > If the above is feasible and sounds reasonable, then we don't even
> > > need the status_change_inprogress flag, at least not during the
> > > start-up flow.
> >
> > I've updated the patch based on the above suggestion. I believe we
> > still need the status_change_inprogress flag when not in recovery but
> > in the new version I don't use the flag during end-of-recovery action.
> >
>
> Thanks for the patch.
>
> I had a look at the new changes. It appears that the startup process
> now marks the state as pending_disable rather than directly disabling
> logical decoding itself. In this setup, I think the situation Amit
> described will no longer occur— the case where the startup process
> disables logical decoding, and during the brief window before
> recovery-done is set, a slot-creation attempt is issued and gets
> blocked.
>
> Currently, in the scenario where the primary has one slot and the
> standby has zero slots, if I promote the standby and then try to
> create another slot in parallel (after
> UpdateLogicalDecodingStatusEndOfRecovery but before marking
> RECOVERY_STATE_DONE), the slot creation proceeds (instead of being
> blocked) and eventually hangs in DecodingContextFindStartpoint()—it
> sleeps in read_local_xlog_page_guts() until the promotion completes.
>
> IIUC, Amit’s original idea was to disable logical decoding within the
> startup process itself rather than doing it lazily and block the
> slot-creation in parallel.

Indeed. I misunderstood the idea. The attached new version patch
should implement the idea correctly.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com