Horton

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

shveta malik <shveta.malik@gmail.com>

View thread
From: shveta malik <shveta.malik@gmail.com>
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Amit Kapila <amit.kapila16@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>, shveta malik <shveta.malik@gmail.com>
Date: 2025-11-27T09:50:57Z
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 Thu, Nov 27, 2025 at 2:32 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
>
> I've squashed all fixup patches and attached the updated patch.
>

Thanks for test results and the patch. Please find a few comments:

1)
If primary has effective_wal_level as logical and standby has zero
slots; then during promotion of standby, if I try to create a slot, it
is restricted as intended, but it gives misleading message:
postgres=# SELECT pg_create_logical_replication_slot('slot',
'pgoutput', false, false, false);
ERROR:  logical decoding on standby requires "effective_wal_level" >=
"logical" on the primary
HINT:  Set "wal_level" >= "logical" or create at least one logical
slot when "wal_level" = "replica".

One possibility is we extend the message as follows:
"Logical decoding on standby requires either effective_wal_level >=
logical on the primary or that recovery has finished." and remove
errhint?

2)
+ * completes. Here's how we handle concurrent toggling logical decoding:

toggling --> toggling of

3)
+ /*
+ * This is the authoritative value used by all processes to determine
+ * whether to write additional information required by logical decoding to
+ * WAL. Since this information could be checked frequently, each process
+ * caches this value in XLogLogicalInfo for better performance.
+ */
+ bool xlog_logical_info;

Shall we mention about cache XLogLogicalInfoXactCache as well here?

4)
EnsureLogicalDecodingEnabled():
+ if (wal_level == WAL_LEVEL_MINIMAL)
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("cannot enable logical decoding when \"wal_level\" = \"minimal\""));

Do we need this error here? CheckSlotRequirements checks this already.
Won't Assert suffice here?

5)
EnsureLogicalDecodingEnabled():
+ if (RecoveryInProgress())
+ {
+ Assert(IsLogicalDecodingEnabled());
+ return;
+ }

I understand that it will be no-op on standby. But it took me some
time to understand why we expect logical-decoding enabled here.  I see
that CheckLogicalDecodingRequirements() won't allow the flow to reach
here if logical-decoding was not enabled in the first place. Shall we
add some comments here?

6)
DisableLogicalDecodingIfNecessary() has this:
+ if (wal_level != WAL_LEVEL_REPLICA)
+ return;

Also it has this:
+ /* Write the WAL to disable logical decoding on standbys too */
+ if (XLogStandbyInfoActive())
+ write_logical_decoding_status_update_record(false);

Do we actually need XLogStandbyInfoActive() here? Given that wal_level
is replica, and we’ve already confirmed that no slot exists while
pending_disable and xlog_logical_info are still true, isn’t that
enough to write a WAL record for the disable operation?

7)
+ /*
+ * With 'minimal' WAL level, there are not logical replication slots
+ * during recovery. Logical decoding is always disabled, so there is no
+ * need to synchronize XLogLogicalInfo..
+ */

not -> no
2 dots at the end.
=====

Reviewing more..

thanks
Shveta