Re: Add Information during standby recovery conflicts

Hi Alvaro,

On 11/28/20 6:36 PM, Alvaro Herrera wrote:
> 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.

Nice!

>
>> +     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.

both comments implemented in the new attached version.

Thanks!

Bertrand