Re: Add Information during standby recovery conflicts

From: "Drouvot, Bertrand" <bdrouvot(at)amazon(dot)com>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>, Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Add Information during standby recovery conflicts
Date: 2020-11-28 11:08:24
Message-ID: 28e04235-27e4-0697-68df-001d2b75bee6@amazon.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
v9-0005-Log-the-standby-recovery-conflict-waits.patch text/plain 18.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Chapman Flack 2020-11-28 16:10:59 gcc -Wimplicit-fallthrough and pg_unreachable
Previous Message Alexander Korotkov 2020-11-28 10:31:28 Re: Improving spin-lock implementation on ARM.