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.
