Re: Add Information during standby recovery conflicts

At Wed, 16 Dec 2020 12:08:31 +0900, Masahiko Sawada
<sawada(dot)mshk(at)gmail(dot)com> wrote in
> On Wed, Dec 16, 2020 at 11:22 AM Kyotaro Horiguchi
> <horikyota(dot)ntt(at)gmail(dot)com> wrote:
> >
> > At Tue, 15 Dec 2020 15:40:03 +0900, Fujii Masao
> <masao(dot)fujii(at)oss(dot)nttdata(dot)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