Hi, On Wed, Feb 05, 2025 at 12:08:26PM +0530, Amit Kapila wrote: > On Wed, Feb 5, 2025 at 2:06 AM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > > > I've attached the updated patch. The fix needs to be back-patched to > > v16 where logical decoding on standby was introduced.
Nice catch and thanks for the patch! I also do prefer the current idea (disallow to start if the hot_standby is off and there is an existing logical slot). > Isn't it better to give the new ERROR near the below code? > RestoreSlotFromDisk() > { > ... > if (cp.slotdata.database != InvalidOid && wal_level < WAL_LEVEL_LOGICAL) > ereport(FATAL, > (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), > errmsg("logical replication slot \"%s\" exists, but \"wal_level\" < > \"logical\"", > NameStr(cp.slotdata.name)), > ... > } > > If feasible, this will avoid an additional loop over the slots and a > new ERROR will be added at the same place as an existing similar > ERROR. Agree. + if (SlotIsLogical(s) && !EnableHotStandby) + ereport(FATAL, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("hot standby must be enabled for pre-existing logical replication slots"))); On a primary lowering wal_level < logical, we'd get something like: " FATAL: logical replication slot "logical_slot" exists, but "wal_level" < "logical" " What about being close to it, so something like? " FATAL: logical replication slot "logical_slot" exists, but "hot_standby" = "off" " Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com