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

  1. Log long wait time on recovery conflict when it's resolved.

  2. Add GUC to log long wait times on recovery conflicts.

  3. Detect the deadlocks between backends and the startup process.

  4. Get rid of the dedicated latch for signaling the startup process.

  5. Add block information in error context of WAL REDO apply loop