From: | "Drouvot, Bertrand" <bdrouvot(at)amazon(dot)com> |
---|---|
To: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> |
Cc: | 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-12-01 08:29:31 |
Message-ID: | 594f4419-62aa-c272-ef7d-4287e5c6b654@amazon.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
On 12/1/20 12:35 AM, Masahiko Sawada wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>
>
>
> On Tue, Dec 1, 2020 at 3:25 AM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
>> On 2020-Dec-01, Fujii Masao wrote:
>>
>>> + if (proc)
>>> + {
>>> + if (nprocs == 0)
>>> + appendStringInfo(&buf, "%d", proc->pid);
>>> + else
>>> + appendStringInfo(&buf, ", %d", proc->pid);
>>> +
>>> + nprocs++;
>>>
>>> What happens if all the backends in wait_list have gone? In other words,
>>> how should we handle the case where nprocs == 0 (i.e., nprocs has not been
>>> incrmented at all)? This would very rarely happen, but can happen.
>>> In this case, since buf.data is empty, at least there seems no need to log
>>> the list of conflicting processes in detail message.
>> Yes, I noticed this too; this can be simplified by changing the
>> condition in the ereport() call to be "nprocs > 0" (rather than
>> wait_list being null), otherwise not print the errdetail. (You could
>> test buf.data or buf.len instead, but that seems uglier to me.)
> +1
>
> Maybe we can also improve the comment of this function from:
>
> + * This function also reports the details about the conflicting
> + * process ids if *wait_list is not NULL.
>
> to " This function also reports the details about the conflicting
> process ids if exist" or something.
>
Thank you all for the review/remarks.
They have been addressed in the new attached patch version.
Thanks!
Bertrand
Attachment | Content-Type | Size |
---|---|---|
v9-0008-Log-the-standby-recovery-conflict-waits.patch | text/plain | 17.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Masahiko Sawada | 2020-12-01 08:31:00 | Re: autovac issue with large number of tables |
Previous Message | Dean Rasheed | 2020-12-01 08:15:15 | Re: Additional improvements to extended statistics |