Re: Fix assert failure when decoding XLOG_PARAMETER_CHANGE on primary

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.mshk@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