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

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: POC: enable logical decoding when wal_level = 'replica' without a server restart
Date: 2025-09-11 17:46:20
Message-ID: CAD21AoDGJ6st53Y4ez0jwVEKJ8h+1yuo6=DgfL2Sds9_2KQNFA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Sep 10, 2025 at 11:32 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Sat, Sep 6, 2025 at 3:46 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Borisov 2025-09-11 17:51:45 Re: Improve the performance of Unicode Normalization Forms.
Previous Message Paul A Jungwirth 2025-09-11 16:54:54 Re: Foreign key isolation tests