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

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.

(FYI, we use this method of callback registration for everything
that's not a one-time code path, like hash tables for RI triggers,
base backup callbacks, etc.)
--
Michael