From: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
---|---|
To: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> |
Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Fix assert failure when decoding XLOG_PARAMETER_CHANGE on primary |
Date: | 2025-02-05 18:59:22 |
Message-ID: | CAD21AoD_nGu3aaXKiEHMJrFvePcM2KkMjzO4r-aRkot8jinRBg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Feb 4, 2025 at 11:48 PM Bertrand Drouvot
<bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
>
> 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(dot)mshk(at)gmail(dot)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.
+1
>
> + 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"
> "
Looks good, but I'd like to mention that this is required only on
standbys. So how about the following?
FATAL: logical replication slot "s" exists on the standby, but
"hot_standby" = "off"
HINT: Change "hot_standby" to be "on".
I've updated the patch accordingly.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
Attachment | Content-Type | Size |
---|---|---|
v2-0001-Fix-assertion-when-decoding-XLOG_PARAMETER_CHANGE.patch | application/octet-stream | 5.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Sami Imseih | 2025-02-05 19:05:32 | Re: Prevent COPY FREEZE on Foreign tables |
Previous Message | Tom Lane | 2025-02-05 18:56:26 | Re: Avoid possible deference NULL pointer (src/backend/optimizer/path/allpaths.c) |