Thanks for the comment. At Wed, 22 Feb 2023 12:29:59 +1100, Peter Smith <smithpb2...@gmail.com> wrote in > On Wed, Feb 22, 2023 at 12:03 PM Kyotaro Horiguchi > <horikyota....@gmail.com> wrote: > > > > At Tue, 21 Feb 2023 10:31:29 +0000, "shiy.f...@fujitsu.com" > > <shiy.f...@fujitsu.com> wrote in > > > Thanks for your reply. I agree that's expensive. Attach a new patch which > > > adds a > > > static boolean to avoid duplicate registration. > > > > Thank you for the patch. It is exactly what I had in my mind. But now > > that I've had a chance to mull it over, I came to think it might be > > better to register the callbacks at one place. I'm thinking we could > > create a new function called register_callbacks() or something and > > move all the calls to CacheRegisterSyscacheCallback() into that. What > > do you think about that refactoring? > > > > I guess you could say that that refactoring somewhat weakens the > > connection or dependency between init_rel_sync_cache and > > rel_sync_cache_relation_cb, but anyway the callback works even if > > RelationSyncCache is not around. > > > > 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. > Then almost nothing else to do...e.g. no need for a new extra static > boolean if static RelationSyncCache is acting as the one-time guard > anyway. Unfortunately, RelationSyncCache doesn't work - it is set to NULL at plugin shutdown. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
