Re: Add Information during standby recovery conflicts - Mailing list pgsql-hackers

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.