Can can I make an injection point wait occur no more than once?

Started by Peter Geoghegan6 months ago12 messages

pg@bowt.ie

6 months ago

I'm working on adding test coverage to _bt_lock_and_validate_left,
which was enhanced by Postgres 18 commit 1bd4bc85ca. In particular,
coverage of its unhappy path: the path where multiple concurrent page
splits necessitate that the scan (which generally moves to the left)
moves to the right multiple times, until finally it gives up. When it
gives up it returns to the original lastcurrblkno to see what's up
with it -- it'll need to get that page's now-current left sibling
link, beginning the whole process anew (by looping back to the start
of _bt_lock_and_validate_left).

An isolation test that uses injection points seems like a natural
approach (actually, it's likely the *only* approach that can produce a
maintainable test). One session should perform a backwards scan that
is forced to wait at the top of _bt_lock_and_validate_left. Another
session then inserts enough index tuples to cause several leaf page
splits that'll make life harder for the backwards scan. Finally, we
wake the backwards scan session, and get the desired test coverage;
it'll reliably have to do things the hard way.

I have all this working already. However, there are certain aspects of
the isolation test (and the injection points themselves) that seem
unsatisfactory. I could really use a way to make the wait within
_bt_lock_and_validate_left happen no more than once, in a way that's
directly under the control of my isolation test.

Any test like this needs to account for various implementation
details. For example, if the test needs to work with non-standard
BLCKSZ (which seems like a good idea), then the number of page splits
required might be greater or fewer than with standard BLCKSZ. This
shouldn't really be a problem; it necessitates inserting more data
than is strictly necessary most of the time: there needs to be some
margin or error to account for these effects. But that shouldn't be
much of a problem.

However, as things stand, this does create a problem: accounting for
these implementation details in this manner makes the number of times
that the injection point is reached unpredictable/hard to control. I
only want the wait within _bt_lock_and_validate_left to happen once,
before the concurrent inserts take place from within the other
isolation test session. I don't want any possible future calls to
_bt_lock_and_validate_left (that come after the other session is done)
to wait at all -- that'll make the backwards scan test session wait
forever (since no other session will be around to wake it up a second
or a third time).

I have successfully simulated "wait no more than once" by adding C
code to nbtree that looks like this:

if (likely(!P_ISDELETED(opaque) &&
opaque->btpo_next == lastcurrblkno))
{
/* Found desired page, return it */
#ifdef USE_INJECTION_POINTS
if (IS_INJECTION_POINT_ATTACHED("lock-and-validate-left"))
{
InjectionPointDetach("lock-and-validate-left");
}
#endif

But that's pretty ugly and non-modular. There are multiple return
paths within _bt_lock_and_validate_left, and I'd probably need to
cover them all with similar code. That seems borderline unacceptable.

It would be far preferable if I could just use some built-in way of
waiting exactly once, that can be used directly from SQL, through the
injection_points extension. That would allow me to write the isolation
test without having to add any code to nbtsearch.c that knows all
about the requirements of one particular isolation test.

Thanks
--
Peter Geoghegan