On Fri, Aug 7, 2020 at 11:09 PM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Fri, Aug 7, 2020 at 1:12 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > That's a meaningless statement for any one caller. So it needs to be more
> > like "we expect callers to add and remove temporary before_shmem_exit
> > callbacks in strict LIFO order".
>
> Sure, that seems fine.
>
v2 patch has the comments modified.
>
> > I wonder whether we ought to change the function to complain if the
> > last list entry doesn't match. We'd have caught this bug sooner
> > if it did, and it's not very clear why silently doing nothing is
> > a good idea when there's no match.
>
> +1.
>
This is a good idea. v3 patch has both the modified comments(from v2)
as well as a DEBUG3 (DEBUG3 level, because the other
non-error/non-fatal logs in ipc.c are using the same level) log to
report when the latest entry for removal is not matched with the one
the caller cancel_before_shmem_exit() is looking for and a hint on how
to safely use temporary before_shmem_exit() callbacks. In v3 patch,
function pointer is being printed, I'm not sure how much it is helpful
to have function pointers in the logs though there are some other
places printing pointers into the logs, I wish I could print function
names. (Is there a way we could get function names from function
pointers?).
Thoughts?
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com