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-02-28 08:28:06 |
Message-ID: | 20200228.172806.1107809554616271723.horikyota.ntt@gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Thank you for the comments.
At Fri, 28 Feb 2020 16:33:18 +0900, Michael Paquier <michael(at)paquier(dot)xyz> wrote in
> On Fri, Feb 28, 2020 at 04:01:00PM +0900, Kyotaro Horiguchi wrote:
> > Hello, this is a followup thread of [1].
> >
> > # I didn't noticed that the thread didn't cover -hackers..
> >
> > When recovery of any type ends, we see several kinds of error messages
> > that says "WAL is broken".
>
> 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.
> > + * 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 whole comment there follows.
| recovery. If we get here during recovery, we can assume that we
| reached the end of WAL. Otherwise something's really wrong and
| we report just only the errormsg if any. If we don't receive
| errormsg here, we already logged something. We don't emit
| "reached end of WAL" in muted messages.
"Othhersise" means "other than the case of recovery". "Just only the
errmsg" means "show the message not as a part the message "reached end
of WAL".
> > + * Note: errormsg is alreay translated.
>
> Typo here.
Thanks. Will fix along with "rached".
> > + if (StandbyMode)
> > + ereport(actual_emode,
> > + (errmsg ("rached end of WAL at %X/%X on timeline %u in %s during streaming replication",
>
> StandbyMode happens also with only WAL archiving, depending on if
> primary_conninfo is set or not.
Right. I'll fix it. Maybe to "during standby mode".
> > + (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?
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
From | Date | Subject | |
---|---|---|---|
Next Message | John Naylor | 2020-02-28 08:42:34 | Re: truncating timestamps on arbitrary intervals |
Previous Message | Michael Paquier | 2020-02-28 08:24:29 | Re: Ought to use heap_multi_insert() for pg_attribute/depend insertions? |