RE: "out of relcache_callback_list slots" after multiple calls to pg_logical_slot_get_binary_changes

On Wed, Feb 22, 2023 2:20 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Wed, Feb 22, 2023 at 12:07:06PM +0900, Kyotaro Horiguchi wrote:
> > At Wed, 22 Feb 2023 12:29:59 +1100, Peter Smith <smithpb2250(at)gmail(dot)com>
> wrote in
> >> If you are going to do that, then won't just copying the
> >> CacheRegisterSyscacheCallback(PUBLICATIONOID... into function
> >> init_rel_sync_cache() be effectively the same as doing that?
> >
> > I'm not sure if it has anything to do with the relation sync cache.
> > On the other hand, moving all the content of init_rel_sync_cache() up
> > to pgoutput_startup() doesn't seem like a good idea.. Another option,
> > as you see, was to separate callback registration code.
>
> Both are kept separate in the code, so keeping this separation makes
> sense to me.
>
> + /* Register callbacks if we didn't do that. */
> + if (!callback_registered)
> + CacheRegisterSyscacheCallback(PUBLICATIONOID,
> + publication_invalidation_cb,
> + (Datum) 0);
>
> /* Initialize relation schema cache. */
> init_rel_sync_cache(CacheMemoryContext);
> + callback_registered = true;
> [...]
> + /* Register callbacks if we didn't do that. */
> + if (!callback_registered)
>
> I am a bit confused by the use of one single flag called
> callback_registered to track both the publication callback and the
> relation callbacks. Wouldn't it be cleaner to use two flags? I don't
> think that we'll have soon a second code path calling
> init_rel_sync_cache(), but if we do then the callback load could again
> be messed up.
>

Thanks for your reply. Using two flags makes sense to me.
Attach the updated patch.

Regards,
Shi Yu