On Wed, Feb 22, 2023 at 12:07:06PM +0900, Kyotaro Horiguchi wrote: > At Wed, 22 Feb 2023 12:29:59 +1100, Peter Smith <smithpb2...@gmail.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
signature.asc
Description: PGP signature
