Re: Improve shutdown during online backup

Simon Riggs wrote:
>> Few comments:
>>
>> * smart shutdown waits for sessions to complete, yet this just ignores
>> smart shutdowns which is something a little different. I think we
>> should wait for the backup to complete and then shutdown.
>
> If we add a function called something like BackupInProgress() to xlog.c,
> exported via miscadmin.h then we can use it within the
> PostmasterStateMachine() function like this
>
> if (pmState == PM_WAIT_BACKENDS)
> {
> if (CountChildren() == 0 &&
> StartupPID == 0 &&
> (BgWriterPID == 0 || !FatalError) &&
> WalWriterPID == 0 &&
> AutoVacPID == 0 &&
> !BackupInProgress()) <---- new line
>
> so that the postmaster doesn't need to know about how we do backups.
>
> That way you don't need any of the special cases in your patch, nor is
> there any need to duplicate the #defines.

I looked at that, and it won't work, for these reasons:

PostmasterStateMachine() is called once after a smart shutdown.
If there are children or a backup is in progress, pmState will remain
PM_WAIT_BACKENDS.

Now whenever a child exits, the reaper() will be called, which in turn
calls PostmasterStateMachine() again and advances pmState if appropriate.
This won't work for backups though, because removal of backup_label will
not send a SIGCHLD to the postmaster.

Moreover, if Shutdown == SmartShutdown, new connections won't be accepted,
and nobody can connect and call pg_stop_backup().
So even if I'd add a check for
(pmState == PM_WAIT_BACKENDS) && !BackupInProgress() somewhere in the
ServerLoop(), it wouldn't do much good, because the only way for somebody
to cancel online backup mode would be to manually remove the file.

So the only reasonable thing to do on smart shutdown during an online
backup is to have the shutdown request fail, right? The only alternative being
that a smart shutdown request should interrupt online backup mode.

So - unless you point out a flaw in my reasoning - I'll implement it
that way, but will put all code that handles backup_label files into
xlog.c.

Yours,
Laurenz Albe