Date: Sat, 28 Nov 2020 14:36:26 -0300
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: "Drouvot, Bertrand" <bdrouvot@amazon.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>,
	Fujii Masao <masao.fujii@oss.nttdata.com>,
	Masahiko Sawada <masahiko.sawada@2ndquadrant.com>,
	Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: Add Information during standby recovery conflicts
Message-ID: <20201128173626.GA17184@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <28e04235-27e4-0697-68df-001d2b75bee6@amazon.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

Hi Bertrand,

On 2020-Nov-28, Drouvot, Bertrand wrote:

> +		if (nprocs > 0)
> +		{
> +			ereport(LOG,
> +					(errmsg("recovery still waiting after %ld.%03d ms: %s",
> +							msecs, usecs, _(get_recovery_conflict_desc(reason))),
> +					 (errdetail_log_plural("Conflicting process: %s.",
> +										   "Conflicting processes: %s.",
> +										   nprocs, buf.data))));
> +		}
> +		else
> +		{
> +			ereport(LOG,
> +					(errmsg("recovery still waiting after %ld.%03d ms: %s",
> +							msecs, usecs, _(get_recovery_conflict_desc(reason)))));
> +		}
> +
> +		pfree(buf.data);
> +	}
> +	else
> +		ereport(LOG,
> +				(errmsg("recovery still waiting after %ld.%03d ms: %s",
> +						msecs, usecs, _(get_recovery_conflict_desc(reason)))));
> +}

Another trivial stylistic point is that you can collapse all these
ereport calls into one, with something like

  ereport(LOG,
          errmsg("recovery still waiting after ...", opts),
          waitlist != NULL ? errdetail_log_plural("foo bar baz", opts) : 0);

where the "waitlist" has been constructed beforehand, or is set to NULL
if there's no process list.

> +	switch (reason)
> +	{
> +		case PROCSIG_RECOVERY_CONFLICT_BUFFERPIN:
> +			reasonDesc = gettext_noop("for recovery conflict on buffer pin");
> +			break;

Pure bikeshedding after discussing this with my pillow: I think I'd get
rid of the initial "for" in these messages.