On Mon, Feb 19, 2024 at 11:54:20AM +0300, Andrey M. Borodin wrote:
> 1. injection_points_wake() will wake all of waiters. But it's not
> suitable for complex tests. I think there must be a way to wake only
> specific waiter by injection point name.
I don't disagree with that, but I don't have a strong argument for
implementing that until there is an explicit need for it in core. It
is also possible to plug in your own module, outside of core, if you
are looking for something more specific. The backend APIs allow that.
> 2. Alexander Korotkov's stopevents could be used in isolation
> tests. This kind of tests is perfect for describing complex race
> conditions. (as a side note, I'd be happy if we could have
> primary\standby in isolation tests too)
This requires plugging is more into src/test/isolation/, with multiple
connection strings. This has been suggested in the past.
> 3. Can we have some Perl function for this?
> +# Wait until the checkpointer is in the middle of the restart point
> +# processing, relying on the custom wait event generated in the
> +# wait callback used in the injection point previously attached.
> +ok( $node_standby->poll_query_until(
> + 'postgres',
> + qq[SELECT count(*) FROM pg_stat_activity
> + WHERE backend_type = 'checkpointer' AND wait_event = 'injection_wait' ;],
> + '1'),
> + 'checkpointer is waiting in restart point'
> +) or die "Timed out while waiting for checkpointer to run restart point";
>
> Perhaps something like
> $node->do_a_query_and_wait_for_injection_point_observed(sql,injection_point_name);
I don't see why not. But I'm not sure how much I need to plug in into
the main modules yet.
> 4. Maybe I missed it, but I'd like to see a guideline on how to name
> injection points.
I don't think we have any of that, or even that we need one. In
short, I'm OK to be more consistent with the choice of ginbtree.c and
give priority to it and make it more official in the docs.
> 5. In many cases we need to have injection point under critical
> section. I propose to have a "prepared injection point". See [0] for
> example in v2-0003-Test-multixact-CV-sleep.patch
> + INJECTION_POINT_PREPARE("GetNewMultiXactId-done");
> +
> START_CRIT_SECTION();
>
> + INJECTION_POINT_RUN_PREPARED();
I don't see how that's different from a wait/wake logic? The only
thing you've changed is to stop a wait when a point is detached and
you want to make the stop conditional. Plugging in a condition
variable is more flexible than a hardcoded sleep in terms of wait,
while being more responsive.
> 7. This is extremely distant, but some DBMSs allow to enable
> injection points by placing files on the filesystem. That would
> allow to test something during recovery when no SQL interface is
> present.
Yeah, I could see why you'd want to do something like that. If a use
case pops up, that can surely be discussed.
--
Michael