Re: Improve shutdown during online backup, take 4 - Mailing list pgsql-patches

From Albe Laurenz
Subject Re: Improve shutdown during online backup, take 4
Date
Msg-id D960CB61B694CF459DCFB4B0128514C2020A76C4@exadv11.host.magwien.gv.at
Whole thread Raw
In response to Re: Improve shutdown during online backup, take 4  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Improve shutdown during online backup, take 4  (Tom Lane <tgl@sss.pgh.pa.us>)
Re: Improve shutdown during online backup, take 4  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-patches
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

pgsql-patches by date:

Previous
From: Alvaro Herrera
Date:
Subject: Re: lc_time and localized dates
Next
From: Tom Lane
Date:
Subject: Re: Improve shutdown during online backup, take 4