On Tue, Apr 5, 2022 at 5:25 PM Stephen Frost <sfrost@snowman.net> wrote:
Greetings,
* David Steele (david@pgmasters.net) wrote: > On 4/4/22 11:42 AM, Nathan Bossart wrote: > >I noticed a couple of other things that can be removed. Since we no longer > >wait on exclusive backup mode during smart shutdown, we can change > >connsAllowed (in postmaster.c) to a boolean and remove CAC_SUPERUSER. We > >can also remove a couple of related notes in the documentation. I've done > >all this in the attached patch. > > These changes look good to me. IMV it is a real bonus how much the state > machine has been simplified.
Yeah, agreed.
Definitely.
> I've also run this patch through the pgbackrest regression tests without any > problems.
Fantastic.
Please find attached an updated patch + commit message. Mostly, I just went through and did a bit more in terms of updating the documentation and improving the comments (there were some places that were still worrying about the chance of a 'stray' backup_label file existing, which isn't possible any longer), along with some additional testing and review. This is looking pretty good to me, but other thoughts are certainly welcome. Otherwise, I'm hoping to commit this tomorrow.
+1. LGTM.
I'm not sure I love the renaming of the functions, but I have also yet to come up with a better idea for how to avoid silent breakage, so go with it.