On Thu, Dec 4, 2025 at 1:54 PM Masahiko Sawada <[email protected]> wrote:
>
>
> I've attached the updated patch that incorporated the review comments
> and is rebased to the current HEAD.
>
Thanks, a few things:
1)
+ The system automatically increases the
+ effective WAL level to <literal>logical</literal> when
creating the first
+ logical replication slot, and decreases it back to
<literal>replica</literal>
+ when dropping the last logical replication slot. The current
effective WAL
+ level can be monitored through <xref
linkend="guc-effective-wal-level"/>
+ parameter.
Shouldn't we mention about invalidation of slot along with dropping of
slot? I have suggested this earlier but I think it got missed to be
addressed.
2)
+ else if (sync_replication_slots)
+ {
+ /*
+ * Signal the postmaster to launch the slotsync worker.
+ *
+ * XXX: For simplicity, we keep the slotsync worker running
+ * even after logical decoding is disabled. A future
+ * improvement can consider starting and stopping the worker
+ * based on logical decoding status change.
+ */
+ kill(PostmasterPid, SIGUSR1);
+ }
+ }
+
+ /* Update the status on shared memory */
+ UpdateLogicalDecodingStatus(logical_decoding, true);
I see that we have moved 'Update' post slotsync's start attempt. This
leaves a possibility that that slot-sync is started sooner than last
'Update' call and thus slotsync may exit with:
replication slot synchronization requires "effective_wal_level" >=
"logical" on the primary
I see that update was prior to the slotsync step in earlier patches.
Why have we moved it to a later stage?
3)
+ /* Return if someone already started to enable logical decoding */
Shall we update:
Return if someone already started to enable logical decoding, or if it
is already enabled.
thanks
Shveta