On Wed, Nov 15, 2023 at 12:21:40PM +0100, Alvaro Herrera wrote:
> On 2023-Nov-15, Michael Paquier wrote:
> Oh, I think you're overthinking what my comment was. I was saying, just
> name it "InjectionPointsHash". Since there appears to be no room for
> another hash table for injection points, then there's no need to specify
> that this one is the ByName hash. I couldn't think of any other way to
> organize the injection points either.
Aha, OK. No problem, this was itching me as well but I didn't see an
argument with changing these names, so I've renamed things a bit more.
>> Yes, still not something that's required in the core APIs or an
>> initial batch.
>
> I agree that we can do the easy thing first and build it up later. I
> just hope we don't get too wedded on the current interface because of
> lack of time in the current release that we get stuck with it.
One thing that I assume we will need with more advanced testing is the
possibility to pass down one (for a structure of data) or more
arguments to the callback a point is attached to. For that, it is
possible to add more macros, like INJECTION_POINT_1ARG,
INJECTION_POINT_ARG(), etc. that use some (void *) pointers. It would
be possible to add that even in stable branches, as need arises,
changing the structure of the shmem hash table if required to control
the way a callback is run.
At the end, I suspect that it is one of these things where we'll need
to adapt depending on what people want to do with this stuff. FWIW, I
can do already quite a bit with this minimalistic design and an
external extension. Custom wait events are also very handy to monitor
a wait.
>> I am not sure that it is a good idea to enforce a specific conditional
>> logic in the backend core code.
>
> Agreed, let's get more experience on what other types of tests people
> want to build, and how are things going to interact with each other.
I've worked more on polishing the core facility today, with 0001
introducing the backend-side facility. One thing that I mentioned
lacking is a local cache for processes so as they don't load an
external callback more than once if run. So I have added this local
cache. When a point is scanned but not found, a previous cache entry
is cleared if any (this leaks but we don't have a way to unload stuff,
and for testing that's not a big deal). I've renamed the routines to
use attach and detach as terms, and adopted the name you've suggested
for the macro. The names around the hash table and its entries have
been changed to what you've suggested. You are right, that's more
intuitive.
0002 is the test module for the basics, split into its own patch, with
a couple of tests for the local cache part. 0003 and 0004 have been
adjusted with the new SQL functions. At the end, I'd like to propose
0004 as it's been a PITA for me and I don't want to break this case
again. It needs more work and can be cheaper. One more argument in
favor of it is the addition of condition variables to wait and wake
points (perhaps with even more variables?) in the test module.
If there is interest for 0003, I'm OK to work more on it as well, but
that's less important IMV.
Thoughts and comments are welcome, with a v4 series attached.
--
Michael