| From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> | 
|---|---|
| To: | Robert Haas <robertmhaas(at)gmail(dot)com> | 
| Cc: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, David Steele <david(at)pgmasters(dot)net>, Magnus Hagander <magnus(at)hagander(dot)net>, Michael Paquier <michael(at)paquier(dot)xyz>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> | 
| Subject: | Re: non-exclusive backup cleanup is mildly broken | 
| Date: | 2019-12-17 20:48:40 | 
| Message-ID: | 15092.1576615720@sss.pgh.pa.us | 
| Views: | Whole Thread | Raw Message | Download mbox | Resend email | 
| Thread: | |
| Lists: | 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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Robert Haas | 2019-12-17 21:12:49 | Re: tableam vs. TOAST | 
| Previous Message | Tom Lane | 2019-12-17 20:42:34 | Re: client auth docs seem to have devolved |