Re: Add Information during standby recovery conflicts

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@alvh.no-ip.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