Horton

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

Peter Smith <smithpb2250@gmail.com>

View thread
From: Peter Smith <smithpb2250@gmail.com>
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, shveta malik <shveta.malik@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-12-16T06:32: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.

Hi Sawada-San.

Some minor review comments for v35-0001.

======
src/backend/replication/logical/logicalctl.c

ProcessBarrierUpdateXLogLogicalInfo:

1.
+/*
+ * This routine is called when we are told to update XLogLogicalInfo
+ * by a ProcSignalBarrier.
+ */
+bool
+ProcessBarrierUpdateXLogLogicalInfo(void)
+{
+ if (GetTopTransactionIdIfAny() != InvalidTransactionId)
+ {
+ /* Delay updating XLogLogicalInfo until the transaction end */
+ XLogLogicalInfoUpdatePending = true;
+ }
+ else
+ update_xlog_logical_info();
+
+ return true;
+}
+

Strange to have a boolean function that unconditionally returns true.
Should the function comment explain the meaning of the (always true)
return value?

The return value is assigned to a variable which defaults true anyway,
so really this function might as well be void.

~~~

EnsureLogicalDecodingEnabled:

2.
+ if (RecoveryInProgress())
+ {
+ /*
+ * CheckLogicalDecodingRequirements() must have already error out if
+ * logical decoding is not enabled since we cannot enable the logical
+ * decoding status during recovery.
+ */
+ Assert(IsLogicalDecodingEnabled());
+ return;
+ }

typo? "already error out"

======
.../recovery/t/050_effective_wal_level.pl

3.
+$primary->safe_psql('postgres',
+ qq[select pg_create_physical_replication_slot('test_phy_slot', false, false)]
+);
+
+# Check that creating a physical slot doesn't affect effective_wal_level.
+test_wal_level($primary, "replica|replica",
+ "effective_wal_level doesn't change with a new physical slot");
+$primary->safe_psql('postgres',
+ qq[select pg_drop_replication_slot('test_phy_slot')]);
+

That pg_create_physical_replication_slot() should be beneath the
comment that says what the test is doing.

~~~

4.
+# Create logical slots on the both nodes.

typo /the both/both/

~~~

5.
+# effective_wal_level should be 'logical' on the both nodes.

typo /the both/both/

~~~

6.
+# Verify that the effective_wal_level remains 'logical' on the both nodes

typo /the both/both/

======
Kind Regards,
Peter Smith.
Fujitsu Australia