On Tue, Jan 02, 2024 at 11:14:56PM -0600, Nathan Bossart wrote:
> Should we specify HASH_FIXED_SIZE, too? This hash table will be in the
> main shared memory segment and therefore won't be able to expand too far
> beyond the declared maximum size.
Good point.
> 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.
> I think </library> is supposed to be </literal> here.
Okay.
> 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.
--
Michael