On Mon, Jul 5, 2021 at 7:33 AM 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.
Hm. The shared recoveryWakeupLatch is being owned by the startup
process to wait and other backends/processes are using it to wake up
the startup process.
> 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.
>
> These don't sound bad to me to apply here, so 0002 could be simplified
> as attached.
The attached stop-backup-latch-v2.patch looks good to me.
Regards,
Bharath Rupireddy.