Re: non-exclusive backup cleanup is mildly broken

On Tue, Dec 17, 2019 at 12:52:05PM -0500, Robert Haas wrote:
> On Tue, Dec 17, 2019 at 8:38 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> Since there doesn't seem to be any opposition to my original fix,
>> except for the fact that I included a bug in it, I'm going to go see
>> about getting that committed.
>
> Perhaps I spoke too soon: I'm not sure whether Michael's comments
> amount to an objection. While I give him a chance to respond, here's
> an updated patch.

stoppoint = do_pg_stop_backup(label_file->data, waitforarchive, NULL);
- cancel_before_shmem_exit(nonexclusive_base_backup_cleanup, (Datum) 0);
[...]
+void
+register_persistent_abort_backup_handler(void)
+{
+ static bool already_done = false;
+
+ if (already_done)
+ return;
So that's how you prevent piling up multiple registrations of this
callback compared to v1. FWIW, I think that it is a cleaner approach
to remove the callback once a non-exclusive backup is done, because a
session has no need for it once it is done with its non-exclusive
backup, and this session may remain around for some time.

+ if (emit_warning)
+ ereport(WARNING,
+ (errmsg("aborting backup due to backend exiting before
pg_stop_back up was called")));
This warning is incorrect => "pg_stop_back up". (Mentioned upthread.)
--
Michael