Re: Add Information during standby recovery conflicts
Kyotaro Horiguchi <horikyota.ntt@gmail.com>
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
To: sawada.mshk@gmail.com
Cc: masao.fujii@oss.nttdata.com, bdrouvot@amazon.com,
alvherre@alvh.no-ip.org, masahiko.sawada@2ndquadrant.com,
pgsql-hackers@postgresql.org
Date: 2020-12-16T05:49:25Z
Lists: pgsql-hackers
At Wed, 16 Dec 2020 12:08:31 +0900, Masahiko Sawada
<sawada.mshk@gmail.com> wrote in
> On Wed, Dec 16, 2020 at 11:22 AM Kyotaro Horiguchi
> <horikyota.ntt@gmail.com> wrote:
> >
> > At Tue, 15 Dec 2020 15:40:03 +0900, Fujii Masao
> <masao.fujii@oss.nttdata.com> wrote in
> > >
> > >
> > > On 2020/12/15 12:04, Kyotaro Horiguchi wrote:
> > > > [40509:startup] DETAIL: Conflicting processes: 41171, 41194.
> > ...
> > > > I'm not sure, but it seems to me at least the period is
> unnecessary
> > > > here.
> > >
> > > Since Error Message Style Guide in the docs says "Detail and
> hint
> > > messages:
> > > Use complete sentences, and end each with a period.", I think
> that a
> > > period
> > > is necessary here. No?
> >
> > In the first place it is not a complete sentence. Might be better
> be
> > something like this if we strictly follow the style guide?
>
> FWIW I borrowed the message style in errdetail from log messages in
> ProcSleep():
> (errmsg("process %d still waiting for %s on %s after %ld.%03d ms",
> MyProcPid, modename, buf.data, msecs, usecs),
> (errdetail_log_plural("Process holding the lock: %s. Wait queue:
> %s.",
> "Processes holding the lock: %s. Wait queue:
> %s.",
> lockHoldersNum, lock_holders_sbuf.data,
> lock_waiters_sbuf.data))));
I was guessing that was the case.
> > > Conflicting processes are 41171, 41194.
> > > Conflicting processes are: 41171, 41194.
Or I came up with the following after scanning throught the tree.
| Some processes are conflicting: 41171, 41194.
> If we use the above message we might want to change other similar
> messages I exemplified as well.
I'm not sure what should we do for other anomalies. Other errdetails
of this category (incomplete sentences or the absence of a period) I
found are:
-- period is absent
pgarch.c596: errdetail("The failed archive command was: %s",
postmaster.c3723: errdetail("Failed process was running: %s",
matview.c654: errdetail("Row: %s",
tablecmds.c2371: errdetail("%s versus %s",
tablecmds.c11512: errdetail("%s depends on column \"%s\"",
subscriptioncmds.c1081: errdetail("The error was: %s", err),
tablesync.c918: errdetail("The error was: %s", res->err)));
be-secure-openssl.c235: errdetail("\"%s\" cannot be higher than
\"%s\"",
auth.c1314: errdetail_internal("SSPI error %x", (unsigned
int) r)));
auth.c2854: errdetail("LDAP diagnostics: %s", message);
pl_exec.c4386: errdetail_internal("parameters: %s",
errdetail) : 0));
postgres.c2401: errdetail("prepare: %s",
pstmt->plansource->query_string);
-- having a period.
proc.c1479: errdetail_log_plural("Process holding the lock:
%s. Wait queue: %s.",
pl_handler.c106: GUC_check_errdetail("Unrecognized key word:
\"%s\".", tok);
Although it dpends on the precise criteria of how they are extracted,
it seems that the absense of a period is more major.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Commits
-
Log long wait time on recovery conflict when it's resolved.
- 39b03690b529 14.0 landed
-
Add GUC to log long wait times on recovery conflicts.
- 0650ff23038b 14.0 landed
-
Detect the deadlocks between backends and the startup process.
- 8900b5a9d59a 14.0 cited
-
Get rid of the dedicated latch for signaling the startup process.
- ac22929a2613 14.0 cited
-
Add block information in error context of WAL REDO apply loop
- 9d0bd95fa90a 14.0 cited