On Thu, Jan 04, 2024 at 08:53:11AM +0900, Michael Paquier wrote: > On Tue, Jan 02, 2024 at 11:14:56PM -0600, Nathan Bossart wrote: >> I'm wondering how important it is to cache the callbacks locally. >> load_external_function() won't reload an already-loaded library, so AFAICT >> this is ultimately just saving a call to dlsym(). > > This keeps a copy to a callback under the same address space, and I > guess that it would matter if the code where a callback is added gets > very hot because this means less function pointers. At the end I > would keep the cache as the code to handle it is neither complex nor > long, while being isolated in its own paths.
Fair enough. >> 0003 and 0004 add tests to the test_injection_points module. Is the idea >> that we'd add any tests that required injection points here? I think it'd >> be better if we could move the tests closer to the logic they're testing, >> but perhaps that is difficult because you also need to define the callback >> functions somewhere. Hm... > > Yeah. Agreed that the final result should not have these tests in the > module test_injection_points. What I was thinking here is to move > 002_invalid_checkpoint_after_promote.pl to src/test/recovery/ and pull > the module with the callbacks with an EXTRA_INSTALL. +1 -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
