Hello,

Andres Freund wrote:

> On 2018-01-03 02:20:14 +0000, Alvaro Herrera wrote:

> > Include an isolationtester spec that 8 out of 10 times reproduces the
> > deadlock with the unpatched code for me (Álvaro).
> 
> The isolation test doesn't appear to be fully stable:
> 
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=baiji&dt=2018-01-03%2003%3A00%3A01

Yeah, thanks for reporting that.  It's annoying, but obvious in
hindsight -- the second process occasionally gets stuck waiting for the
first one to release its snapshot.  I can reproduce this about half the
time, by adding a 100ms sleep in ProcessUtilitySlow right after the
DefineIndex call.

I can turn the isolation test to produce the output where s2 waits
(without the sleep) if I introduce a third session s3 to lock s2; then
releasing the lock in s3 causes both s1 and s2 to complete.  I think the
completions would be reported always in that order because of the way
isolationtester verifies suspension.

However this means that the test will get removed in 9.4 and 9.5 because
isolationtester is not smart enough there.

I suppose the other option would be to add an alternate expected file
for the test.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From b0eb35672f9e514591afd99b7aec5c14339baf5b Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Wed, 3 Jan 2018 11:16:34 -0300
Subject: [PATCH] Fix isolation test to be less timing-dependent

I did this by adding another locking process, which makes the other two
wait.  This way the output should be stable enough.  However, it means
isolationtester older than 9.6 is unable to run this test, so remove it.

Per buildfarm and Andres Freund
Discussion: 
https://postgr.es/m/20180103034445.t3utrtrnrevfs...@alap3.anarazel.de
---
 src/test/isolation/expected/multiple-cic.out | 17 +++++++++++------
 src/test/isolation/specs/multiple-cic.spec   | 12 ++++++++----
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/src/test/isolation/expected/multiple-cic.out 
b/src/test/isolation/expected/multiple-cic.out
index cc57940392..0b470e7d1d 100644
--- a/src/test/isolation/expected/multiple-cic.out
+++ b/src/test/isolation/expected/multiple-cic.out
@@ -1,6 +1,9 @@
-Parsed test spec with 2 sessions
+Parsed test spec with 3 sessions
 
-starting permutation: s2l s1i s2i
+starting permutation: s2l s1i s2i s3u
+pg_advisory_lock
+
+               
 step s2l: SELECT pg_advisory_lock(281457);
 pg_advisory_lock
 
@@ -11,9 +14,11 @@ step s1i:
  <waiting ...>
 step s2i: 
                CREATE INDEX CONCURRENTLY mcic_two_pkey ON mcic_two (id)
-               WHERE unlck();
+               WHERE unlck() AND lck_shr(572814);
+ <waiting ...>
+step s3u: SELECT unlck();
+unlck          
 
+t              
 step s1i: <... completed>
-s1             
-
-               
+step s2i: <... completed>
diff --git a/src/test/isolation/specs/multiple-cic.spec 
b/src/test/isolation/specs/multiple-cic.spec
index a7ba4eb4fd..fbec67ee25 100644
--- a/src/test/isolation/specs/multiple-cic.spec
+++ b/src/test/isolation/specs/multiple-cic.spec
@@ -26,15 +26,19 @@ session "s1"
 step "s1i"     {
                CREATE INDEX CONCURRENTLY mcic_one_pkey ON mcic_one (id)
                WHERE lck_shr(281457);
-       }
-teardown       { SELECT pg_advisory_unlock_all() AS "s1"; }
+}
+step "s1u"  { SELECT unlck(); }
 
 
 session "s2"
 step "s2l"  { SELECT pg_advisory_lock(281457); }
 step "s2i"     {
                CREATE INDEX CONCURRENTLY mcic_two_pkey ON mcic_two (id)
-               WHERE unlck();
+               WHERE unlck() AND lck_shr(572814);
        }
 
-permutation "s2l" "s1i" "s2i"
+session "s3"
+setup          { SELECT pg_advisory_lock(572814); }
+step "s3u"     { SELECT unlck(); }
+
+permutation "s2l" "s1i" "s2i" "s3u"
-- 
2.11.0

Reply via email to