Re: Add Information during standby recovery conflicts - Mailing list 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.c�596:        errdetail("The failed archive command was: %s",
postmaster.c�3723:    errdetail("Failed process was running: %s",
matview.c�654:        errdetail("Row: %s",
tablecmds.c�2371:    errdetail("%s versus %s",
tablecmds.c�11512:    errdetail("%s depends on column \"%s\"",
subscriptioncmds.c�1081:    errdetail("The error was: %s", err),
tablesync.c�918:    errdetail("The error was: %s", res->err)));
be-secure-openssl.c�235:    errdetail("\"%s\" cannot be higher than 
\"%s\"",
auth.c�1314:        errdetail_internal("SSPI error %x", (unsigned 
int) r)));
auth.c�2854:        errdetail("LDAP diagnostics: %s", message);
pl_exec.c�4386:        errdetail_internal("parameters: %s", 
errdetail) : 0));
postgres.c�2401:    errdetail("prepare: %s", 
pstmt->plansource->query_string);

-- having a period.

proc.c�1479:        errdetail_log_plural("Process holding the lock: 
%s. Wait queue: %s.",
pl_handler.c�106:    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