Horton

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

Amit Kapila <amit.kapila16@gmail.com>

View thread
From: Amit Kapila <amit.kapila16@gmail.com>
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: shveta malik <shveta.malik@gmail.com>, Peter Smith <smithpb2250@gmail.com>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>, Shlok Kyal <shlok.kyal.oss@gmail.com>, Bertrand Drouvot <bertranddrouvot.pg@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2025-11-13T09:55:56Z
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, Nov 12, 2025 at 3:36 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> I've attached the updated version patch. I addressed all comments I
> got so far, and made some cosmetic changes.
>

Few comments:
1. BTW, to accomplish what we do in
UpdateLogicalDecodingStatusEndOfRecovery(), can't we follow a logic
similar to what we do in EnsureLogicalDecodingEnabled()?

Basically, enable xlog_logical_info and mark SharedRecoveryState =
RECOVERY_STATE_DONE under one spinlock. After the spinlock is
released, wait for other backends to reflect the xlog_logical_info via
WaitForProcSignalBarrier(EmitProcSignalBarrier(PROCSIGNAL_BARRIER_UPDATE_XLOG_LOGICAL_INFO))
and then enable logical decoding by setting logical_decoding_enabled
and writing a new WAL record.

Can't this avoid the special handling required in the patch for the
status_change_allowed flag? This flag seems to be primarily required
because recovery_done action and the change of wal_level action are
being performed separately during promotion. If it is feasible to
perform them together, then we don't need an additional state that
says logical decoding status_change_allowed.

The reason I am trying to explore an alternative to the current way to
handle promotion is that it can eliminate one of the complex parts of
the patch. But, if that is not feasible then we can live with the
current approach.

2.
We don't need to
+ * wait for running transactions to finish as we don't accept any writes
+ * yet. We need the wait even if we've not updated the status above as the
+ * status have been turned on and off during recovery, having running
+ * processes have different status on their local caches.

In the first sentence, the comment says, we don't need to wait, and in
the second, it says we need the wait... Isn't it confusing? What is
the difference between both waits, if any? Can we improve this
comment?

IIUC, this comment means that we should wait for all other backends to
reflect the new state for xlog_logical_info and
logical_decoding_enabled. But the way it is written makes it
confusing.

-- 
With Regards,
Amit Kapila.