Re: POC: enable logical decoding when wal_level = 'replica' without a server restart

Masahiko Sawada <sawada.mshk@gmail.com>

From: Masahiko Sawada <sawada.mshk@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, Shlok Kyal <shlok.kyal.oss@gmail.com>, Bertrand Drouvot <bertranddrouvot.pg@gmail.com>, shveta malik <shveta.malik@gmail.com>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2025-09-11T17:46:20Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Fix regression test failure when wal_level is set to minimal.

  2. Toggle logical decoding dynamically based on logical slot presence.

  3. Disallow server start with sync_replication_slots = on and wal_level < logical.

On Wed, Sep 10, 2025 at 11:32 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Sat, Sep 6, 2025 at 3:46 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > I've attached the updated patch that incorporated all comments I got so far.
> >
>
> *
> + /*
> + * While all processes are using the new status, there could be some
> + * transactions that might have started with the old status. So wait
> + * for the running transactions to complete so that logical decoding
> + * doesn't include transactions that wrote WAL with insufficient
> + * information.
> + */
> + running = GetRunningTransactionData();
> + LWLockRelease(ProcArrayLock);
> + LWLockRelease(XidGenLock);
> +
> + elog(DEBUG1, "waiting for %d transactions to complete", running->xcnt);
> +
> + for (int i = 0; i < running->xcnt; i++)
> + {
> + TransactionId xid = running->xids[i];
> +
> + if (TransactionIdIsCurrentTransactionId(xid))
> + continue;
> +
> + XactLockTableWait(xid, NULL, NULL, XLTW_None);
> + }
>
> When building a snapshot during the start of logical decoding, we
> anyway wait for running transactions to finish via the snapbuild
> machinery. So, why do we need it here? And if it is needed, can we
> update the comments to explain why it is required in spite of
> snapbuild machinery doing similar thing?

Fair point. I don't see any reason we need to wait here. Will remove this step.

> * Is it a good idea to enable/disable decoding for temporary logical
> slots? The temporary slots are released during ERROR or at session
> end, is that a good time to do the disable processing that even
> requires WAL writing.

I think the same is true for slots with RS_EPEMERAL state. Since it
could confuse users if automatic effective_wal_level change is
supported only for non-temporary slots, I personally would like not to
push aside temporary slots. I agree that it might not be a good time
to disable processing during process shutdown time; in addition to
requiring WAL record, it also requires waits for concurrent state
change processings while it holds all interrupts, which could easily
involve dead-locks. It might be worth considering doing the disable
process in a lazy way. For example, other processes (like
checkpointer) periodically checks the logical decoding status and
disables it if necessary.

Regards,

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