Re: Make mesage at end-of-recovery less scary.

From: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
To: michael(at)paquier(dot)xyz
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Make mesage at end-of-recovery less scary.
Date: 2020-03-05 07:06:50
Message-ID: 20200305.160650.1027844943773682801.horikyota.ntt@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello.

I changed the condition from randAccess to fetching_ckpt considering
the discussion in another thread [1]. Then I moved the block that
shows the new messages to more appropriate place.

At Fri, 28 Feb 2020 17:28:06 +0900 (JST), Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> wrote in
> >
> > Have you considered an error context here? Your patch leads to a bit
> > of duplication with the message a bit down of what you are changing
> > where the end of local pg_wal is reached.
>
> It is a DEBUG message and it is for the time moving from crash
> recovery to archive recovery. I could remove that but decided to leave
> it for tracability.

I modified the message so that it has the same look to the new
messages, but I left it being DEBUG1, since it is just a intermediate
state. We should finally see one of the new three messages.

After the messages changed, another message from wal sender came to
look redundant.

| [20866] LOG: replication terminated by primary server
| [20866] DETAIL: End of WAL reached on timeline 1 at 0/30001C8.
| [20866] FATAL: could not send end-of-streaming message to primary: no COPY in progress
| [20851] LOG: reached end of WAL at 0/30001C8 on timeline 1 in archive during standby mode
| [20851] DETAIL: invalid record length at 0/30001C8: wanted 24, got 0

I changed the above to the below, which looks more adequate.

| [24271] LOG: replication terminated by primary server on timeline 1 at 0/3000240.
| [24271] FATAL: could not send end-of-streaming message to primary: no COPY in progress
| [24267] LOG: reached end of WAL at 0/3000240 on timeline 1 in archive during standby mode
| [24267] DETAIL: invalid record length at 0/3000240: wanted 24, got 0

> > > + * reached the end of WAL. Otherwise something's really wrong and
> > > + * we report just only the errormsg if any. If we don't receive
> >
> > This sentence sounds strange to me. Or you meant "Something is wrong,
> > so use errormsg as report if it is set"?

The message no longer exists.

> > > + (errmsg ("rached end of WAL at %X/%X on timeline %u in %s during crash recovery",
> >
> > FWIW, you are introducing three times the same typo, in the same
> > word, in three different messages.
>
> They're copy-pasto. I refrained from constructing an error message
> from multiple nonindipendent parts. Are you suggesting to do so?

The tree times repetition of almost same phrases is very unreadable. I
rewrote it in more simple shape.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachment Content-Type Size
v2-0001-Make-End-Of-Recovery-error-less-scary.patch text/x-patch 5.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2020-03-05 07:15:28 Re: pg_stat_progress_basebackup - progress reporting for pg_basebackup, in the server side
Previous Message Amit Kapila 2020-03-05 06:44:46 Re: [HACKERS] Moving relation extension locks out of heavyweight lock manager