On Thu, Nov 27, 2025 at 11:23 PM vignesh C <[email protected]> wrote: > > > Few comments: > 1) There is no validation after creation of temporary logical > replication slot, can we see if "logical decoding is enabled upon > creating a new logical replication slot" is logged as a validation for > this: > +# Create a temporary logical slot but exits without releasing it explicitly. > +# This enables logical decoding but skips disabling it and delegates to the > +# checkpointer. > +$primary->safe_psql('postgres', > + qq[select pg_create_logical_replication_slot('test_tmp_slot', > 'test_decoding', true)] > +); > + > +# Wait for the checkpointer to disable logical decoding. > +wait_for_logical_decoding_disabled($primary); > > I had a look at the header file inclusions in the patch and found few > issues with it: > 2) Here logicalctl.h should be included before logicallauncher.h: > #include "postmaster/interrupt.h" > #include "replication/logicallauncher.h" > +#include "replication/logicalctl.h" > #include "replication/slotsync.h" > > 3) I was able to compile without inclusion of logicalctl.h, may be it > is not required because we include origin.h which includes xlog.h > which includes logicalctl.h already: > diff --git a/src/backend/access/transam/xlog.c > b/src/backend/access/transam/xlog.c > index 22d0a2e8c3a..6f3f10af8fd 100644 > --- a/src/backend/access/transam/xlog.c > +++ b/src/backend/access/transam/xlog.c > @@ -79,7 +79,9 @@ > #include "postmaster/walsummarizer.h" > #include "postmaster/walwriter.h" > #include "replication/origin.h" > +#include "replication/logicalctl.h" > > 4) Similarly it is not required in checkpointer.c, logical.c, > logicalctl.c, slotsync.c, slot.c, slotfuncs.c, walsender.c, ipci.c, > procsignal.c, standby.c and postinit.c > > 5) Few comments in the test file exceeds 80 chars, if possible we can > move it to the next line: > 5.a) +# Create and drop another logical slot, then check if > effective_wal_level remains > +# 'logical'. > 5.b) +# Add other settings to test if we disable logical decoding when > invalidating the last > +# logical slot. > 5.c) # Check if logical decoding is disabled after invalidating the > last logical slot. > 5.d) # Check if effective_wal_level is increased to 'logical' on the > cascaded standby. > 5.e) # Check that the logical decoding is not enabled on the standby4. > Note that it still has > # the invalidated logical slot. > 5.f) # Restart the primary with setting wal_level = 'logical' and > create a new logical > # slot. > 5.g) # Drop the logical slot, requesting to disable logical decoding > to the checkpointer. > 5.h) # Test the abort process of logical decoding activation. We drop > the primary's > # slot to decrease its effective_wal_level to 'replica'.
I've fixed the above points and run pgperltidy again. I'll submit the updated patch soon. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
