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

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, Bertrand Drouvot <bertranddrouvot(dot)pg(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-11-10 04:50:52
Message-ID: CAHut+Pvu-=y6kpM0PWRry2VSW+P68X+QR7NXuq4uu3K2n5HWmA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Alena Vinter 2025-11-10 05:02:14 Re: Resetting recovery target parameters in pg_createsubscriber
Previous Message Fujii Masao 2025-11-10 04:45:39 Re: Suggestion to add --continue-client-on-abort option to pgbench