Re: Remove useless arguments in ReadCheckpointRecord().

On Thu, Jul 21, 2022 at 11:24 AM Kyotaro Horiguchi
<horikyota(dot)ntt(at)gmail(dot)com> wrote:
>
> > > May be unrelated, IIRC, for the errors like ereport(PANIC,
> > > (errmsg("could not locate a valid checkpoint record"))); we wanted to
> > > add a hint asking users to consider running pg_resetwal to fix the
> > > issue. The error for ReadCheckpointRecord() in backup_label file
> > > cases, already gives a hint errhint("If you are restoring from a
> > > backup, touch \"%s/recovery.signal\" ...... Adding the hint of running
> > > pg_resetwal (of course with a caution that it can cause inconsistency
> > > in the data and use it as a last resort as described in the docs)
> > > helps users and support engineers a lot to mitigate the server down
> > > cases quickly.
> >
> > +1 to add some hint messages. But I'm not sure if it's good to hint
> > the use of pg_resetwal because, as you're saying, it should be used as
> > a last resort and has some big risks like data loss, corruption,
> > etc. That is, I'm concerned about that some users might quickly run
> > pg_resetwal because hint message says that, without reading the docs
> > nor understanding those risks.
>
> I don't think we want to recommend pg_resetwal to those who don't
> reach it by themselves by other means. I know of a few instances of
> people who had the tool (unrecoverably) break their own clusters.

Agree. We might want to take this topic separately as it needs more
careful study of common issues such as PANICs and then adding hints
with proven ways to repair the server and bring it back online.

Regards,
Bharath Rupireddy.