public inbox for [email protected]  

help / color / mirror / Atom feed

From: Nitin Jadhav <[email protected]>
To: Michael Paquier <[email protected]>
Cc: Pg Hackers <[email protected]>
Subject: Re: Change checkpoint‑record‑missing PANIC to FATAL
Date: Thu, 19 Feb 2026 08:24:02 +0530
Message-ID: <CAMm1aWZQfMm1Wn=cMorqNsqdySERZCgV2JL87jVy4yCmLY8xvg@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAMm1aWZ9Tv=Wrx52_2Ppw+6ULf_twRZuQm=ZWLA_a-kXWykHkQ@mail.gmail.com>
	<[email protected]>
	<CAMm1aWb47v9Bx40P1_6YpRxxKi9XSwjAV_bLbFxx66Rg8o3+=g@mail.gmail.com>
	<[email protected]>

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’m fine with dropping cases
(3), (4) for now.

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

Best Regards,
Nitin Jadhav
Azure Database for PostgreSQL
Microsoft

view thread (7+ messages)  latest in thread

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: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: Change checkpoint‑record‑missing PANIC to FATAL
  In-Reply-To: <CAMm1aWZQfMm1Wn=cMorqNsqdySERZCgV2JL87jVy4yCmLY8xvg@mail.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