Re: Add Information during standby recovery conflicts

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.

Regards,

-- 
Masahiko Sawada
EnterpriseDB:  https://www.enterprisedb.com/