On Wed, Feb 22, 2023 2:20 PM Michael Paquier <mich...@paquier.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 <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. >
Thanks for your reply. Using two flags makes sense to me. Attach the updated patch. Regards, Shi Yu
v3-0001-Avoid-duplicate-registration-of-callbacks-in-pgou.patch
Description: v3-0001-Avoid-duplicate-registration-of-callbacks-in-pgou.patch
