On Wed, May 15, 2019 at 8:36 PM Melanie Plageman <melanieplage...@gmail.com>
wrote:
>
> On Wed, May 15, 2019 at 6:50 PM Andres Freund <and...@anarazel.de> wrote:
>
>>
>> > I noticed that there is not a test case which would cover the
>> speculative
>> > wait
>> > codepath. This seems much more challenging, however, it does seem like a
>> > worthwhile test to have.
>>
>> Shouldn't be that hard to create, I think. I think acquiring another
>> lock in a second, non-unique, expression index, ought to do the trick?
>> It probably has to be created after the unique index (so it's later in
>> the
>>
>> I would think that the sequence would be s1 and s2 probe the index, s1
> and s2
> insert into the table, s1 updates the index but does not complete the
> speculative insert and clear the token (pause before
> table_complete_speculative). s2 is in speculative wait when attempting to
> update
> the index.
>
> Something like
>
> permutation
> "controller_locks"
> "controller_show"
> "s1_upsert" "s2_upsert"
> "controller_show"
> "controller_unlock_1_1" "controller_unlock_2_1"
> "controller_unlock_1_3" "controller_unlock_2_3"
> "controller_unlock_1_2"
> "s1_magically_pause_before_complete_speculative"
> # put s2 in speculative wait
> "controller_unlock_2_2"
> "s1_magically_unpause_before_complete_speculative"
>
> So, how would another lock on another index keep s1 from clearing the
> speculative token after it has updated the index?
>
The second index would help to hold the session after inserting the tuple
in unique index but before completing the speculative insert. Hence, helps
to create the condition easily. I believe order of index insertion is
helping here that unique index is inserted and then non-unique index is
inserted too.
Attaching patch with the test using the idea Andres mentioned and it works
to excercise the speculative wait.
From 24e44f1a971e8957503205376d655e22b0e5fbdc Mon Sep 17 00:00:00 2001
From: Ashwin Agrawal <aagra...@pivotal.io>
Date: Wed, 15 May 2019 22:21:48 -0700
Subject: [PATCH] Add test to validate speculative wait is performed.
---
.../expected/insert-conflict-specconflict.out | 69 +++++++++++++++++++
.../specs/insert-conflict-specconflict.spec | 44 ++++++++++++
2 files changed, 113 insertions(+)
diff --git a/src/test/isolation/expected/insert-conflict-specconflict.out b/src/test/isolation/expected/insert-conflict-specconflict.out
index 5726bdb8e8..ca2b65ca56 100644
--- a/src/test/isolation/expected/insert-conflict-specconflict.out
+++ b/src/test/isolation/expected/insert-conflict-specconflict.out
@@ -177,3 +177,72 @@ step controller_show: SELECT * FROM upserttest;
key data
k1 inserted s1 with conflict update s2
+
+starting permutation: s1_create_non_unique_index controller_locks controller_show s1_upsert s2_upsert controller_show controller_unlock_1_1 controller_unlock_2_1 controller_unlock_1_3 controller_unlock_2_3 controller_show controller_lock_2_4 controller_unlock_2_2 controller_show controller_unlock_1_2 controller_print_speculative_locks controller_unlock_2_4 controller_show
+step s1_create_non_unique_index: CREATE INDEX ON upserttest((blurt_and_lock2(key)));
+step controller_locks: SELECT pg_advisory_lock(sess, lock), sess, lock FROM generate_series(1, 2) a(sess), generate_series(1,3) b(lock);
+pg_advisory_locksess lock
+
+ 1 1
+ 1 2
+ 1 3
+ 2 1
+ 2 2
+ 2 3
+step controller_show: SELECT * FROM upserttest;
+key data
+
+step s1_upsert: INSERT INTO upserttest(key, data) VALUES('k1', 'inserted s1') ON CONFLICT (blurt_and_lock(key)) DO UPDATE SET data = upserttest.data || ' with conflict update s1'; <waiting ...>
+step s2_upsert: INSERT INTO upserttest(key, data) VALUES('k1', 'inserted s2') ON CONFLICT (blurt_and_lock(key)) DO UPDATE SET data = upserttest.data || ' with conflict update s2'; <waiting ...>
+step controller_show: SELECT * FROM upserttest;
+key data
+
+step controller_unlock_1_1: SELECT pg_advisory_unlock(1, 1);
+pg_advisory_unlock
+
+t
+step controller_unlock_2_1: SELECT pg_advisory_unlock(2, 1);
+pg_advisory_unlock
+
+t
+step controller_unlock_1_3: SELECT pg_advisory_unlock(1, 3);
+pg_advisory_unlock
+
+t
+step controller_unlock_2_3: SELECT pg_advisory_unlock(2, 3);
+pg_advisory_unlock
+
+t
+step controller_show: SELECT * FROM upserttest;
+key data
+
+step controller_lock_2_4: SELECT pg_advisory_lock(2, 4);
+pg_advisory_lock
+
+
+step controller_unlock_2_2: SELECT pg_advisory_unlock(2, 2);
+pg_advisory_unlock
+
+t
+step controller_show: SELECT * FROM upserttest;
+key data
+
+step controller_unlock_1_2: SELECT pg_advisory_unlock(1, 2);
+pg_advisory_unlock
+
+t
+step controller_print_speculative_locks: SELECT locktype,classid,objid,mode,granted FROM pg_locks WHERE locktype='speculative token' order by granted;
+locktype classid objid mode granted
+
+speculative token4 0 ShareLock f
+speculative token4 0 ExclusiveLock t
+step controller_unlock_2_4: SELECT pg_advisory_unlock(2, 4);
+pg_advisory_unlock
+
+t
+step s1_upsert: <... completed>
+step s2_upsert: <... completed>
+step controller_show: SELECT * FROM upserttest;
+key data
+
+k1 inserted s2 with conflict update s1
diff --git a/src/test/isolation/specs/insert-conflict-specconflict.spec b/src/test/isolation/specs/insert-conflict-specconflict.spec
index 3a70484fc2..71e46727cf 100644
--- a/src/test/isolation/specs/insert-conflict-specconflict.spec
+++ b/src/test/isolation/specs/insert-conflict-specconflict.spec
@@ -23,6 +23,13 @@ setup
RETURN $1;
END;$$;
+ CREATE OR REPLACE FUNCTION blurt_and_lock2(text) RETURNS text IMMUTABLE LANGUAGE plpgsql AS $$
+ BEGIN
+ RAISE NOTICE 'blurt_and_lock1 called for %', $1;
+ PERFORM pg_advisory_xact_lock(current_setting('spec.session')::int, 4);
+ RETURN $1;
+ END;$$;
+
CREATE TABLE upserttest(key text, data text);
CREATE UNIQUE INDEX ON upserttest((blurt_and_lock(key)));
@@ -45,7 +52,10 @@ step "controller_unlock_1_2" { SELECT pg_advisory_unlock(1, 2); }
step "controller_unlock_2_2" { SELECT pg_advisory_unlock(2, 2); }
step "controller_unlock_1_3" { SELECT pg_advisory_unlock(1, 3); }
step "controller_unlock_2_3" { SELECT pg_advisory_unlock(2, 3); }
+step "controller_lock_2_4" { SELECT pg_advisory_lock(2, 4); }
+step "controller_unlock_2_4" { SELECT pg_advisory_unlock(2, 4); }
step "controller_show" {SELECT * FROM upserttest; }
+step "controller_print_speculative_locks" { SELECT locktype,classid,objid,mode,granted FROM pg_locks WHERE locktype='speculative token' order by granted; }
session "s1"
setup
@@ -54,6 +64,7 @@ setup
SET spec.session = 1;
}
step "s1_begin" { BEGIN; }
+step "s1_create_non_unique_index" { CREATE INDEX ON upserttest((blurt_and_lock2(key))); }
step "s1_upsert" { INSERT INTO upserttest(key, data) VALUES('k1', 'inserted s1') ON CONFLICT (blurt_and_lock(key)) DO UPDATE SET data = upserttest.data || ' with conflict update s1'; }
step "s1_commit" { COMMIT; }
@@ -147,3 +158,36 @@ permutation
"controller_show"
"s2_commit"
"controller_show"
+
+# Test that speculative wait is performed if session sees a speculatively
+# inserted tuple. Speculatively inserted tuple means tuple is inserted into the
+# table and unique index but yet to *complete* the speculative insertion
+permutation
+ # acquire a number of locks, to control execution flow - the
+ # blurt_and_lock function acquires advisory locks that allow us to
+ # continue after a) the optimistic conflict probe b) after the
+ # insertion of the speculative tuple.
+ "s1_create_non_unique_index"
+ "controller_locks"
+ "controller_show"
+ "s1_upsert" "s2_upsert"
+ "controller_show"
+ # Switch both sessions to wait on the other lock next time (the speculative insertion)
+ "controller_unlock_1_1" "controller_unlock_2_1"
+ # Allow both sessions to continue
+ "controller_unlock_1_3" "controller_unlock_2_3"
+ "controller_show"
+ # take lock to block second session after inserting in unique index but
+ # before completing the speculative insert
+ "controller_lock_2_4"
+ # Allow the second session to move forward
+ "controller_unlock_2_2"
+ # This should still not show a successful insertion
+ "controller_show"
+ # Allow the first session, it should perform speculative wait
+ "controller_unlock_1_2"
+ # Should report waiting on speculative lock
+ "controller_print_speculative_locks"
+ "controller_unlock_2_4"
+ # This should now show a successful UPSERT
+ "controller_show"
--
2.19.1
