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-11-10T04:50:52Z
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 review comments for v24-0001.

======
src/backend/replication/slot.c

ReplicationSlotsDropDBSlots:

1.
  LWLockAcquire(ReplicationSlotControlLock, LW_SHARED);
  for (i = 0; i < max_replication_slots; i++)
  {
  ReplicationSlot *s;
+ bool invalidated;
  char    *slotname;
  int active_pid;

@@ -1462,11 +1494,22 @@ restart:
  if (!SlotIsLogical(s))
  continue;

+ SpinLockAcquire(&s->mutex);
+ invalidated = s->data.invalidated == RS_INVAL_NONE;
+ SpinLockRelease(&s->mutex);
+
+ /*
+ * Count slots on other databases too so we can disable logical
+ * decoding only if no slots in the cluster.
+ */
+ if (invalidated)
+ n_valid_logicalslots++;
+

IMO it's misleading/backwards to have bool invalidated == true when
the invalidated member is RS_INVAL_NONE.

It's fine to rename the bool var as 'invalidated', but don't you also
need to reverse the assignment to (s->data.invalidated !=
RS_INVAL_NONE) instead of (s->data.invalidated == RS_INVAL_NONE)?

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