Re: A new function to wait for the backend exit after termination

On Mon, Mar 15, 2021 at 10:38 AM Fujii Masao
<masao.fujii@oss.nttdata.com> wrote:
> On 2021/03/15 12:27, Bharath Rupireddy wrote:
> > On Sun, Mar 7, 2021 at 2:39 PM Bharath Rupireddy
> > <bharath.rupireddyforpostgres@gmail.com> wrote:
> >> Attaching v7 patch for further review.
> >
> > Attaching v8 patch after rebasing on to the latest master.
>
> Thanks for rebasing the patch!

Thanks for reviewing.

> -       WAIT_EVENT_XACT_GROUP_UPDATE
> +       WAIT_EVENT_XACT_GROUP_UPDATE,
> +       WAIT_EVENT_BACKEND_TERMINATION
>
> These should be listed in alphabetical order.

Done.

> In pg_wait_until_termination's do-while loop, ResetLatch() should be called. Otherwise, it would enter busy-loop after any signal arrives. Because the latch is kept set and WaitLatch() always exits immediately in that case.

Done.

> +       /*
> +        * Wait in steps of waittime milliseconds until this function exits or
> +        * timeout.
> +        */
> +       int64   waittime = 10;
>
> 10 ms per cycle seems too frequent?

Increased it to 100msec.

> +                       ereport(WARNING,
> +                                       (errmsg("timeout cannot be negative or zero: %lld",
> +                                                       (long long int) timeout)));
> +
> +                       result = false;
>
> IMO the parameter should be verified before doing the actual thing.

Done.

> Why is WARNING thrown in this case? Isn't it better to throw ERROR like pg_promote() does?

Done.

Attaching v9 patch for further review.

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com