On 15/03/2024 09:39, Michael Paquier wrote:
On Thu, Mar 14, 2024 at 07:13:53PM -0400, Tom Lane wrote:
I can see that some tests would want to be able to inject code
cluster-wide, but I bet that's going to be a small minority.
I suggest that we invent a notion of "global" vs "local"
injection points, where a "local" one only fires in the DB it
was defined in. Then only tests that require a global injection
point need to be NO_INSTALLCHECK.
Attached is a POC of what could be done. I have extended the module
injection_points so as it is possible to register what I'm calling a
"condition" in the module that can be defined with a new SQL function.
The condition is stored in shared memory with the point name, then at
runtime the conditions are cross-checked in the callbacks. With the
interface of this patch, the condition should be registered *before* a
point is attached, but this stuff could also be written so as
injection_points_attach() takes an optional argument with a database
name. Or this could use a different, new SQL function, say a
injection_points_attach_local() that registers a condition with
MyDatabaseId on top of attaching the point, making the whole happening
while holding once the spinlock of the shmem state for the module.
For the gin test, a single "SELECT injection_points_attach_local()" at
the top of the test file would be most convenient.
If I have to do "SELECT
injection_points_condition('gin-finish-incomplete-split', :'datname');"
for every injection point in the test, I will surely forget it sometimes.
In the 'gin' test, they could actually be scoped to the same backend.
Wrt. the spinlock and shared memory handling, I think this would be
simpler if you could pass some payload in the InjectionPointAttach()
call, which would be passed back to the callback function:
void
InjectionPointAttach(const char *name,
const char *library,
- const char *function)
+ const char *function,
+ uint64 payload)
In this case, the payload would be the "slot index" in shared memory.
Or perhaps always allocate, say, 1024 bytes of working area for every
attached injection point that the test module can use any way it wants.
Like for storing extra conditions, or for the wakeup counter stuff in
injection_wait(). A fixed size working area is a little crude, but would
be very handy in practice.
By the way, modules/gin/ was missing missing a detach, so the test was
not repeatable with successive installchecks.
Oops.
It would be nice to automatically detach all the injection points on
process exit. You wouldn't always want that, but I think most tests hold
a session open throughout the test, and for those it would be handy.
--
Heikki Linnakangas
Neon (https://neon.tech)
