On Mon, Jul 27, 2026 at 9:36 PM Masahiko Sawada <[email protected]> wrote: > > On Fri, Jul 24, 2026 at 12:20 AM Amit Kapila <[email protected]> wrote: > > > > The other observation I had while looking at this patch was: On a > > standby, InvalidateObsoleteReplicationSlots() can disable logical > > decoding whenever it invalidates the last valid logical slot via > > following check: > > > > if (invalidated_logical && !found_valid_logicalslot) > > RequestDisableLogicalDecoding(); > > RequestDisableLogicalDecoding() isn't guarded against recovery, so a > > purely local invalidation (RS_INVAL_HORIZON/WAL_REMOVED/IDLE_TIMEOUT) > > makes the standby disable decoding even though the primary still has > > it enabled. The standby's status is supposed to follow the primary via > > XLOG_LOGICAL_DECODING_STATUS_CHANGE replay, and there's no > > self-healing since the primary never sends an "enable". So, won't that > > be a problem because after that no new slots will be allowed to be > > created on standby and slotsync also won't be able perform sync. Am, I > > missing something? If not then probable the above check needs > > additional check: "!RecoveryInProgress()". > > In this case, the checkpointer will try to disable logical decoding > but does nothing as it's still in recovery. See the > RecoveryInProgress() check in DisableLogicalDecodingIfNecessary(). > > If we skip calling RequestDisableLogicalDecoding() in this case, we > would end up missing the disable request as it can be interleaved with > the promotion process. Imagine: > > 1. the startup process begins the promotion, decides the > new_status=true as there is a logical slot (in > UpdateLogicalDecodingStatusEndOfRecovery()). > 2. a backend process drops the last logical slot and skips calling > RequestDisableLogicalDecoding() as the recovery status is still > in-progress. > 3. the startup completes the > UpdateLogicalDecodingStatusEndOfRecovery(). The logical decoding is > enabled even though there is no logical slot. >
Thanks, I missed the check in DisableLogicalDecodingIfNecessary(). But will it be better to add a comment atop that check and also atop RequestDisableLogicalDecoding()? If so, how about something like attached? -- With Regards, Amit Kapila.
v1_fix_comments_1.patch
Description: Binary data
