agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Mats Kindahl <mats.kindahl@gmail.com>
To: Tatsuya Kawata <kawatatatsuya0913@gmail.com>
Cc: Zsolt Parragi <zsolt.parragi@percona.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: pg_rewind does not rewind diverging timelines
Date: Sun, 16 Aug 2026 13:37:32 +0200
Message-ID: <0ed702a2-5cb2-4d5b-a9e2-c9ae66fe7c83@gmail.com> (raw)
In-Reply-To: <CAHza6qczQXOwEyqNX2rq5UNo2XQb_bWY33F=9sNXzVPr6DztMA@mail.gmail.com>
References: <CAN305gBeJr8m7ZRW9mH0zakEFR4hDUPDo8fJRKJOHWMORG5_Bg@mail.gmail.com>
<CAOVWO5oZZtniLR4Pyd=e_cS-FNxh837Gbz9TDUnSwWqmbap=bw@mail.gmail.com>
<CAN305gCcj4Mhr3uBQAnQCYsx6F-syp1rGtazoy=h+_EHO0xOXA@mail.gmail.com>
<SY7PR01MB1092190B1E748F1438CAB5D7AB60A2@SY7PR01MB10921.ausprd01.prod.outlook.com>
<CAN305gBPFE8KPgT5cdsbK8Xwxxii_+Hp4WVhCWsjFOYJ9j4xaw@mail.gmail.com>
<SY7PR01MB109216DF36CF987CF6EF10B29B60B2@SY7PR01MB10921.ausprd01.prod.outlook.com>
<CAN305gCaErXmG3fg48n50dWUC7=ETBBopuFL_cgyzutXUdp-5g@mail.gmail.com>
<SY7PR01MB10921E0F8383139EB27B33C11B6162@SY7PR01MB10921.ausprd01.prod.outlook.com>
<9ce0d2b9-7a41-4a8a-b299-da295bb4514f@gmail.com>
<CAN4CZFO+HVWRWELsu4CuQ+Fr=J4e+jdHM5siRQ3G3vyJ5-Y6Sg@mail.gmail.com>
<8683af69-28af-4a2d-a1db-aa1447b02446@gmail.com>
<CAHza6qczQXOwEyqNX2rq5UNo2XQb_bWY33F=9sNXzVPr6DztMA@mail.gmail.com>
On 7/26/26 11:57, Tatsuya Kawata wrote:
> Hi Mats-san, Zsolt-san,
>
> Thanks -- I went through both v7 and the new version.
>
> > + PG_CATCH();
> > + {
> > + ErrorData *edata = CopyErrorData();
> > +
> > + FlushErrorState();
> > + ereport(FATAL,
> > + errmsg("invalid UUID in history file \"%s\"", path),
> > + errdetail("%s", edata->message));
> > + }
> >
> > This is missing a MemoryContextSwitchTo before CopyErrorData, and
> > results in an assertion with debug builds.
>
> > Thank you for reviewing this and sorry for the delay. I have attached a
> > new version with the issues you pointed to handled. See comments inline
> > below.
>
> The context-switch
> fix in readTimeLineHistory() (restoring the caller's context before
> CopyErrorData()) looks correct to me.
>
> One note: the original problem was not only a debug-build assertion. On
> non-assert builds CopyErrorData() allocates the ErrorData in ErrorContext,
> FlushErrorState() then frees it, and the following
> errdetail("%s", edata->message) reads freed memory -- a use-after-free
> that
> can crash a production server, not just trip an Assert(). Your fix already
> covers this; I'm just sharing it since it bears on the severity.
Got that. Assertions are just a way to trigger a potential problem
early. I did not assume this change was needed just to avoid the assertion.
> One minor point: on an invalid UUID the backend FATALs while the frontend
> (pg_rewind) silently treats it as "unknown" (all-zero) -- probably
> intentional, just flagging it.And should you ever want to drop the
> PG_TRY/PG_CATCH here, uuid_in supports soft errors, so a
> DirectInputFunctionCallSafe() call with an ErrorSaveContext would avoid
> CopyErrorData()/FlushErrorState() and the context switch entirely -- i.e.
> it removes the very handling that had to be fixed here, so this class of
> mistake can't recur. The current fix is correct and minimal, so this is
> purely optional.
Yes, I wanted to keep the UUID just as a final discriminator, after the
TLI, and keep the changes minimal.
Best wishes,
Mats Kindahl
>
> Regards,
> Tatsuya Kawata
>
view thread (37+ messages) latest in thread
Message-ID: <0ed702a2-5cb2-4d5b-a9e2-c9ae66fe7c83@gmail.com>
Permalink: ../0ed702a2-5cb2-4d5b-a9e2-c9ae66fe7c83@gmail.com/
Also on: postgresql.org/message-id/0ed702a2-5cb2-4d5b-a9e2-c9ae66fe7c83@gmail.com
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: mats.kindahl@gmail.com, kawatatatsuya0913@gmail.com, zsolt.parragi@percona.com, pgsql-hackers@lists.postgresql.org
Subject: Re: pg_rewind does not rewind diverging timelines
In-Reply-To: <0ed702a2-5cb2-4d5b-a9e2-c9ae66fe7c83@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox