Re: Add Information during standby recovery conflicts

Hi,

On 11/27/20 2:40 PM, Alvaro Herrera wrote:
> On 2020-Nov-27, Drouvot, Bertrand wrote:
>
>> +             if (nprocs > 0)
>> +             {
>> +                     ereport(LOG,
>> +                                     (errmsg("%s after %ld.%03d ms",
>> +                                                     get_recovery_conflict_desc(reason), msecs, usecs),
>> +                                      (errdetail_log_plural("Conflicting process: %s.",
>> +                                                                                "Conflicting processes: %s.",
>> +                                                                                nprocs, buf.data))));
>> +             }
>> +/* Return the description of recovery conflict */
>> +static const char *
>> +get_recovery_conflict_desc(ProcSignalReason reason)
>> +{
>> +     const char *reasonDesc = gettext_noop("unknown reason");
>> +
>> +     switch (reason)
>> +     {
>> +             case PROCSIG_RECOVERY_CONFLICT_BUFFERPIN:
>> +                     reasonDesc = gettext_noop("recovery is still waiting for recovery conflict on buffer pin");
>> +                     break;
> This doesn't work from a translation point of view.  First, you're
> building a sentence from parts, which is against policy.  Second, you're
> not actually invoking gettext to translate the string returned by
> get_recovery_conflict_desc.
>
> I think this needs to be rethought.  To handle the first problem I
> suggest to split the error message in two.  One phrase is the complain
> that recovery is waiting, and the other string is the reason for the
> wait.  Separate both either by splitting with a :, or alternatively put
> the other sentence in DETAIL.  (The latter would require to mix with the
> list of conflicting processes, which might be hard.)
>
> The first idea would look like this:
>
> LOG:  recovery still waiting after %ld.03d ms: for recovery conflict on buffer pin
> DETAIL:  Conflicting processes: 1, 2, 3.
>
> To achieve this, apart from editing the messages returned by
> get_recovery_conflict_desc, you'd need to ereport this way:
>
>      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.",
>
Thanks for your comments, I did not know that.

I've attached a new version that takes your comments into account.

Thanks!

Bertrand