On Thu, Oct 31, 2024 at 08:47:04AM +0900, Michael Paquier wrote:
> Thanks for the review.  Applied that, then.

I was looking at src/test/isolation/README, and based on what is
described for parenthesized markers (which I didn't know about so we
learn a new thing every day), it is possible to force a strict
ordering of the test.  Quote that matters:
"A marker consisting solely of a step name indicates that this step
may not be reported as completing until that other step has
completed."

In our case, the problem that caused 9f00edc22888 is that the "wait"
step could be so slow that the "detach" step could report before the
"wait" step.

So I'd like to propose that we revisit 9f00edc22888, and re-enable the
permutation with a marker so as the "detach" step waits for the "wait"
step to complete before printing its own report, as per the attached.
That should make the permutation safe to use.

The output of the attached patch is the same as what has been removed
in 9f00edc22888 except that the permutation is changed from that:
permutation wait1 wakeup2 detach2
To that, making sure that detach2 report happens only after wait1
has returned its own result:
permutation wait1 wakeup2 detach2(wait1)

Thoughts?
--
Michael
diff --git a/src/test/modules/injection_points/expected/basic.out b/src/test/modules/injection_points/expected/basic.out
index 9499ef4bd9d..840ce2dac90 100644
--- a/src/test/modules/injection_points/expected/basic.out
+++ b/src/test/modules/injection_points/expected/basic.out
@@ -1,5 +1,31 @@
 Parsed test spec with 2 sessions
 
+starting permutation: wait1 wakeup2 detach2
+injection_points_attach
+-----------------------
+                       
+(1 row)
+
+step wait1: SELECT injection_points_run('injection-points-wait'); <waiting ...>
+step wakeup2: SELECT injection_points_wakeup('injection-points-wait');
+injection_points_wakeup
+-----------------------
+                       
+(1 row)
+
+step wait1: <... completed>
+injection_points_run
+--------------------
+                    
+(1 row)
+
+step detach2: SELECT injection_points_detach('injection-points-wait');
+injection_points_detach
+-----------------------
+                       
+(1 row)
+
+
 starting permutation: wait1 detach2 wakeup2
 injection_points_attach
 -----------------------
diff --git a/src/test/modules/injection_points/specs/basic.spec b/src/test/modules/injection_points/specs/basic.spec
index 47db676c0c1..753128e7f36 100644
--- a/src/test/modules/injection_points/specs/basic.spec
+++ b/src/test/modules/injection_points/specs/basic.spec
@@ -25,12 +25,11 @@ session s2
 step wakeup2	{ SELECT injection_points_wakeup('injection-points-wait'); }
 step detach2	{ SELECT injection_points_detach('injection-points-wait'); }
 
-# Detach after wait and wakeup.
-# Permutations like the following one commented out should be avoided, as
-# the detach may finish before the SQL function doing the wait returns
-# its result.  It is recommended to use wakeups as the last permutation
-# should a wait be done within an SQL function.
-#permutation wait1 wakeup2 detach2
+# Detach after wait and wakeup.  Note that the detach may finish before
+# the SQL function doing the wait returns its result.  In order to avoid
+# any ordering issues, the detach step reports its result only once the
+# wait has completed.  This is enforced with a parenthesized marker.
+permutation wait1 wakeup2 detach2(wait1)
 
 # Detach before wakeup.  s1 waits until wakeup, ignores the detach.
 permutation wait1 detach2 wakeup2

Attachment: signature.asc
Description: PGP signature

Reply via email to