Re: Improve shutdown during online backup, take 4

["Albe Laurenz" <laurenz(dot)albe(at)wien(dot)gv(dot)at>]

Tom Lane wrote:
>>> Lastly, the changes to pmdie's SIGINT handling seem quite bogus.
>>> Don't you need to transition into WAIT_BACKUP rather than WAIT_BACKENDS
>>> state in that case too?  Shouldn't you do CancelBackup *before*
>>> PostmasterStateMachine?  The thing screams of race conditions.
> 
>> I suspect there must be a misunderstanding.
>> You cannot really mean that the postmaster should enter WAIT_BACKUP
>> state on a fast shutdown request.
> 
> Why not?  It'll fall out of the state again immediately in
> PostmasterStateMachine, no, if we do a CancelBackup here?  I don't think
> these two paths of control should be any more different than really
> necessary.

We cannot call CancelBackup there because that's exactly the state
in which a smart shutdown waits for a superuser to issue pg_stop_backup().

> Well, if there were anything conditional about calling it, then maybe
> that argument would hold some water, but the way you've got it here it
> *will* get called anyway, just after the PostmasterStateMachine call
[...]

I see.

> The other reason for the remark about race conditions is that the
> PostmasterStateMachine call should absolutely be the last thing that
> pmdie() does --- putting anything after it is wrong, especially things
> that might alter the PM state, as indeed CancelBackup could.  

I see that too. Thanks for explaining.

> If you actually want the behavior you propose, then the only correct way
> to implement it is to embed it into the state machine logic, ie, do the
> CancelBackup inside PostmasterStateMachine in some state transition
> taken after the last child is gone.

I've attached a patch that works for me. I hope I got it right.

Yours,
Laurenz Albe

Attachment: cancel_backup.patch
Description: cancel_backup.patch