Horton

Re: Weird test mixup

Michael Paquier <michael@paquier.xyz>

View thread View on postgr.es
From: Michael Paquier <michael@paquier.xyz>
To: Noah Misch <noah@leadboat.com>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>, Tom Lane <tgl@sss.pgh.pa.us>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2024-05-10T01:04:17Z
Lists: pgsql-hackers

Attachments

On Thu, May 09, 2024 at 04:39:00PM -0700, Noah Misch wrote:

Thanks for the feedback.

> The return-bool approach sounds fine.  Up to you whether to do in this patch,
> else I'll do it when I add the test.

I see no reason to not change the signature of the routine now if we
know that we're going to do it anyway in the future.  I was shortly
wondering if doing the same for InjectionpointAttach() would make
sense, but it has more error states, so I'm not really tempted without
an actual reason (cannot think of a case where I'd want to put more
control into a module after a failed attach).

>> It could
>> always be possible that a concurrent backend does a detach followed by
>> an attach with the same name, causing the shmem exit callback to drop
>> a point it should not, but that's not really a plausible case IMO :)
> 
> Agreed.  It's reasonable to expect test cases to serialize backend exits,
> attach calls, and detach calls.  If we need to fix that later, we can use
> attachment serial numbers.

Okay by me.

> I'd name this INJ_CONDITION_UNCONDITIONAL or INJ_CONDITION_ALWAYS.  INVALID
> sounds like a can't-happen event or an injection point that never runs.
> Otherwise, the patch looks good and makes src/test/modules/gin safe for
> installcheck.  Thanks.

INJ_CONDITION_ALWAYS sounds like a good compromise here.

Attached is an updated patch for now, indented with a happy CI.  I am
still planning to look at that a second time on Monday with a fresher
mind, in case I'm missing something now.
--
Michael

Commits

  1. injection_points: Store runtime conditions in private area

  2. Introduce private data area for injection points

  3. injection_points: Fix incorrect spinlock acquisition

  4. Make GIN tests using injection points concurrent-safe

  5. injection_points: Fix race condition with local injection point tests

  6. injection_points: Introduce runtime conditions

  7. Make GIN test using injection points repeatable

  8. Fix backstop in gin test if injection point is not reached

  9. Try to unbreak injection-fault tests in the buildfarm

  10. Disable tests using injection points in installcheck