Greetings,
* Michael Paquier (michael@paquier.xyz) wrote:
> On Fri, Jul 02, 2021 at 12:03:07PM +0530, Bharath Rupireddy wrote:
> > My bad. I was talking about the cases when do_pg_stop_backup is called
> > while the server is in recovery mode i.e. backup_started_in_recovery =
> > RecoveryInProgress(); evaluates to true. I'm not sure in these cases
> > whether we should replace pg_usleep with WaitLatch. If yes, whether we
> > should use procLatch/MyLatch or recoveryWakeupLatch as they are
> > currently serving different purposes.
>
> It seems to me that you should re-read the description of
> recoveryWakeupLatch at the top of xlog.c and check for which purpose
> it exists, which is, in this case, to wake up the startup process to
> accelerate WAL replay. So do_pg_stop_backup() has no business with
> it.
>
> Switching pg_stop_backup() to use a latch rather than pg_usleep() has
> benefits:
> - It simplifies the wait event handling.
> - The process waiting for the last WAL segment to be archived will be
> more responsive on signals like SIGHUP and on postmaster death.
Yes, agreed.
> These don't sound bad to me to apply here, so 0002 could be simplified
> as attached.
Took a quick look and the patch looks good to me.
In general, I agree with Tom's up-thread comment about children hanging
around after postmaster death making things more difficult for debugging
and just in general, so I'm in favor of trying to eliminate as many
cases where that's happening as we reasonably can without impacting
performance by checking too often.
Thanks,
Stephen
