On Wed, Nov 19, 2025 at 1:52 AM Masahiko Sawada <[email protected]> wrote: > > On Mon, Nov 17, 2025 at 10:49 PM Amit Kapila <[email protected]> wrote: > > > > On Tue, Nov 18, 2025 at 1:02 AM Masahiko Sawada <[email protected]> > > wrote: > > > > > > On Sun, Nov 16, 2025 at 9:51 PM Amit Kapila <[email protected]> > > > 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. thanks Shveta
