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