Re: [PATCH] Fix ProcKill lock-group vs procLatch recycle race

On Wed, May 06, 2026 at 02:51:00PM +0500, Andrey Borodin wrote:
> cc to Michael:
>
> prockill_race needs to build the same InjectionPointCondition payload that
> injection_wait consumes to know which PID to block. The struct is currently
> private to injection_points.c, so the patch extracts it into a small header
> that prockill_race.c includes via a relative "../injection_points/" path.
> That works but feels non-idiomatic. Since injection_points grows organically
> to support new bug reproducers anyway, making the condition type part of its
> public header seems like a natural fit - but we are not sure the fix is
> committable as-is, so we wanted to ask before doing any more cleanup: is
> this refactor acceptable at all, and if so, would you prefer a proper
> installed header (as contrib/pg_plan_advice does) over the relative include?

I did not look at the bug fix in details, so this is a comment about
the structure of the test.

+#include "../injection_points/injection_point_condition.h"

Hmm.  I would not see a problem in just moving all that to the module
injection_points instead, and keep it there, including your TAP test.
Noah has done something similar for its removable_cutoff() business,
and we are living well with it.  One issue with the structure you are
proposing is that I suspect that it makes some installcheck scenarios
more iffy to deal with.  More callbacks in the test module is fine.
--
Michael