On Mon, Jul 20, 2026 at 9:47 AM Srinath Reddy Sadipiralla <[email protected]> wrote: > > Hi Masahiko-san, > > On Thu, Jul 16, 2026 at 6:52 AM Masahiko Sawada <[email protected]> wrote: >> >> >> While reviewing concurrency aspects of this code before pushing the >> fix, I found other race conditions in the same area: logical decoding >> can be deactivated while a logical slot is being created on a standby. >> >> On standbys, logical decoding can be deactivated while a logical slot >> is being created: either by replaying an >> XLOG_LOGICAL_DECODING_STATUS_CHANGE record, or by the end-of-recovery >> transition upon promotion, which deactivates logical decoding if no >> valid logical slot exists. When logical decoding is deactivated, all >> logical slots on the standby are invalidated, but this cannot find a >> slot that is not visible yet. Therefore, a slot creation whose status >> check interleaved with the deactivation could continue based on a >> stale status. This affects two paths: >> >> For regular slot creation, EnsureLogicalDecodingEnabled() assumed that >> logical decoding must still be enabled during recovery since the >> caller had already checked it. If a promotion interleaves as described >> above, the backend creating the slot fails with: >> >> TRAP: failed Assert("IsLogicalDecodingEnabled()"), File: "logicalctl.c" >> >> For slot synchronization, the local slot could be created and >> persisted based on the remote slot information fetched before the >> deactivation was replayed, leaving a valid slot whose restart_lsn >> precedes the deactivation. Decoding such a slot after a failover fails >> with: >> >> ERROR: unexpected logical decoding status change 0 >> >> These races are confined to the narrow window between checking the >> logical decoding status and the new slot becoming visible; once the >> slot is visible, the invalidation performed by the deactivation >> already covers it. So the fix is simple: re-check the logical decoding >> status after the new slot becomes visible. Regular slot creation >> raises an error and slot synchronization skips persisting the slot. If >> the deactivation happens after the recheck instead, it is guaranteed >> to invalidate the now-visible slot as usual. The attached 0002 >> implements this. > > > i have looked into these conditions and they make sense and reviewed the > v3-0002 patch, LGTM.
Thank you for reviewing the patch! > > while reviewing this, I had a thought (it's not related to these race > issues), but > if we disable logical decoding in primary by removing all the slots when > wal_level = replica; it directly invalidates the private slots of the standby > which > seems unfair cause there might be some consumers using it, but then suddenly > they get an error to either change the wal_level = logical on primary or add > a slot > on the primary, but instead i think we can make primary aware of the private > slots of standby and keep logical decoding on, during the > XLOG_LOGICAL_DECODING_STATUS_CHANGE record redo, thoughts? > IIUC it's too late to inform the primary during XLOG_LOGICAL_DECODING_STATUS_CHANGE redo; the standby replays that record only after the primary has disabled logical decoding, so WAL lacking the information required for logical decoding has already been generated. Once such a gap exists, the standby's slots cannot decode past it even if the primary re-enabled logical decoding in response, so we would have to invalidate them anyway. Alternatively, standbys could proactively tell the primary about their slots (like hot_standby_feedback), but I don't think this can be made reliable. With cascaded standbys the information has to be propagated up through each level, and the propagation lag leaves an unavoidable race: by the time the primary learns that a downstream server still needs logical WAL, its slots may already be gone, or a new slot could be created right after the primary decided to disable. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
