MIME-Version: 1.0
References: <CAMm1aWZ9Tv=Wrx52_2Ppw+6ULf_twRZuQm=ZWLA_a-kXWykHkQ@mail.gmail.com>
 <aUIoTCHzW8sdDpe_@paquier.xyz> <CAMm1aWb47v9Bx40P1_6YpRxxKi9XSwjAV_bLbFxx66Rg8o3+=g@mail.gmail.com>
 <aYPnGiQQAcDiUNrB@paquier.xyz>
In-Reply-To: <aYPnGiQQAcDiUNrB@paquier.xyz>
From: Nitin Jadhav <nitinjadhavpostgres@gmail.com>
Date: Thu, 19 Feb 2026 08:24:02 +0530
Message-ID: <CAMm1aWZQfMm1Wn=cMorqNsqdySERZCgV2JL87jVy4yCmLY8xvg@mail.gmail.com>
Subject: =?UTF-8?Q?Re=3A_Change_checkpoint=E2=80=91record=E2=80=91missing_PANIC_to_FA?=
	=?UTF-8?Q?TAL?=
To: Michael Paquier <michael@paquier.xyz>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAMm1aWZQfMm1Wn%3DcMorqNsqdySERZCgV2JL87jVy4yCmLY8xvg%40mail.gmail.com>
Precedence: bulk

Hi Michael,

Thanks for the detailed feedback.

> For clarity's sake, we are talking about lowering this one in
> xlogrecovery.c, which relates to the code path where these is no
> backup_label file:
> ereport(PANIC,
>         errmsg("could not locate a valid checkpoint record at %X/%08X",
>                LSN_FORMAT_ARGS(CheckPointLoc)));

I agree that case (1) is sufficient for the purpose of this change. I
mentioned the scenarios where a backup_label file exists mainly to
consider additional coverage in this area, but I agree those would
only be bonuses, as you note later.

> For the sake of the change from the PANIC to FATAL mentioned at the
> top of this message, (1) would be enough.
>
> The two cases of (2) I'm mentioning would be nice bonuses.  I would
> recommend to double-check first if we trigger these errors in some
> tests of the existing tests, actually, perhaps we don't need to add
> anything except a check in some node's logs for the error string
> patterns wanted.

I agree with your assessment. Case (1) is enough for this change, and
the cases in (2) would be nice bonuses. I=E2=80=99m fine with dropping case=
s
(3), (4) for now.

I had a quick look at the existing recovery TAP tests and didn=E2=80=99t
immediately find a case where simply adding log checks would cover
these error paths, but I=E2=80=99ll double=E2=80=91check once more before s=
ending the
patch. I=E2=80=99ll work on this and share the patch soon.

Best Regards,
Nitin Jadhav
Azure Database for PostgreSQL
Microsoft