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

On Sat, Aug 2, 2025 at 4:53 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Thu, Jul 31, 2025 at 5:00 AM Hayato Kuroda (Fujitsu)
> <kuroda.hayato@fujitsu.com> wrote:
> >
> > Dear Sawada-san,
> >
> > > I thought we could fix this issue by checking the number of in-use
> > > logical slots while holding ReplicationSlotControlLock and
> > > LogicalDecodingControlLock, but it seems we need to deal with another
> > > race condition too between backends and startup processes at the end
> > > of recovery.
> > >
> > > Currently the backend skips controlling logical decoding status if the
> > > server is in recovery (by checking RecoveryInProgress()), but it's
> > > possible that a backend process tries to drop a logical slot after the
> > > startup process calling UpdateLogicalDecodingStatusEndOfRecovery() and
> > > before accepting writes.
> >
> > Right. I also verified on local and found that
> > ReplicationSlotDropAcquired()->DisableLogicalDecodingIfNecessary() sometimes
> > skips to modify the status because RecoveryInProgress is still false.
> >
> > > In this case, the backend ends up not
> > > disabling logical decoding and it remains enabled. I think we would
> > > somehow need to delay the logical decoding status change in this
> > > period until the recovery completes.
> >
> > My primitive idea was to 1) keep startup acquiring the lock till end of recovery
> > and 2) DisableLogicalDecodingIfNecessary() acquires lock before checking the
> > recovery status, but it could not work well. Not sure but WaitForProcSignalBarrier()
> > stucked if the process acquired LogicalDecodingControlLock lock....
>
> I think that it's not realistic to keep holding a lwlock until the
> recovery actually completes because we perform a checkpoint after
> that.
>
> In the latest version patch I attached, I introduce a flag on shared
> memory to delay any logical decoding status change until the recovery
> completes. The implementation got more complex than I expected but I
> don't have a better idea. I'm open to other approaches. Also, I
> incorporated all comments I got so far[1][2][3] and updated the
> documentation.
>

Yes, it is slightly complex, I will put more thoughts into it. That
said, I do have a related scenario in mind concerning the recent fix,
where we might still end up with an incorrect effective_wal_level
after promotion.

Say primary has 'wal_level'=replica and standby has
'wal_level'=logical. Since there are no slots on standby
'effective_wal_level' will still be replica. Now I created a slot both
on primary and standby making 'effective_wal_level'=logical. Now, when
the standby is promoted and the slot is dropped immediately after
UpdateLogicalDecodingStatusEndOfRecovery() releases the lock, we still
expect the effective_wal_level on the promoted standby (now the
primary) to remain logical, since its configured 'wal_level' is
logical and it has become the primary. But I think that may not be the
case because 'DisableLogicalDecodingIfNecessary-->start_logical_decoding_status_change()'
does not consider original wal_level on promoted standby in
retrial-attempt. I feel 'retry' should be above ' wal_level ==
WAL_LEVEL_LOGICAL' check in below code snippet:

+static bool
+start_logical_decoding_status_change(bool new_status)
+{
+ /*
+ * On the primary with 'logical' WAL level, we can skip logical decoding
+ * status change as it's always enabled. On standbys, we need to check the
+ * status on shared memory propagated from the primary and might handle
+ * status change delay.
+ */
+ if (!RecoveryInProgress() && wal_level == WAL_LEVEL_LOGICAL)
+ return false;
+
+retry:
+

Please note that I could not reproduce this scenario because as soon
as I put sleep or injection-point in
UpdateLogicalDecodingStatusEndOfRecovery(), I hit some ProcSignal
Barriers issue i.e. it never completes even when sleep is over. I get
this: 'LOG: still waiting for backend with PID 162838 to accept
ProcSignalBarrier'.

Please let me know if my understanding is not correct above.

thanks
Shveta