Re: non-exclusive backup cleanup is mildly broken - Mailing list pgsql-hackers

From Tom Lane
Subject Re: non-exclusive backup cleanup is mildly broken
Date
Msg-id 15092.1576615720@sss.pgh.pa.us
Whole thread Raw
In response to Re: non-exclusive backup cleanup is mildly broken  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: non-exclusive backup cleanup is mildly broken  (Kyotaro Horiguchi <horikyota.ntt@gmail.com>)
Re: non-exclusive backup cleanup is mildly broken  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
I wrote:
> Took a quick look.  I agree that this seems a lot cleaner than the
> alternative proposals.  I'd suggest however that the header comment
> for do_pg_abort_backup could do with more work, perhaps along the
> lines of "The odd-looking signature allows this to be registered
> directly as a shmem_exit handler".

> Personally I'd have kept the handler as a separate function that's just
> a one-line wrapper around "void do_pg_abort_backup(bool emit_warning)".
> We don't usually treat callbacks as functions to be also called in
> their own right.  But if you don't want to do that, I'll settle for an
> acknowledgement of the hack in the comment.

Oh, scratch that --- looking closer, I see that the only two use-cases in
the patched code are via before_shmem_exit and PG_ENSURE_ERROR_CLEANUP,
and both of those require a function with the signature of an on_exit
callback.  So there's no need for a separate wrapper because this isn't
going to be called any other way.  I still recommend amending the
comment to explain why it has this signature, though.

            regards, tom lane



pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: client auth docs seem to have devolved
Next
From: Robert Haas
Date:
Subject: Re: tableam vs. TOAST