]> granicus.if.org Git - postgresql/commitdiff
Fix FOR UPDATE NOWAIT on updated tuple chains
authorAlvaro Herrera <alvherre@alvh.no-ip.org>
Wed, 27 Aug 2014 23:15:18 +0000 (19:15 -0400)
committerAlvaro Herrera <alvherre@alvh.no-ip.org>
Wed, 27 Aug 2014 23:15:18 +0000 (19:15 -0400)
If SELECT FOR UPDATE NOWAIT tries to lock a tuple that is concurrently
being updated, it might fail to honor its NOWAIT specification and block
instead of raising an error.

Fix by adding a no-wait flag to EvalPlanQualFetch which it can pass down
to heap_lock_tuple; also use it in EvalPlanQualFetch itself to avoid
blocking while waiting for a concurrent transaction.

Authors: Craig Ringer and Thomas Munro, tweaked by Álvaro
http://www.postgresql.org/message-id/51FB6703.9090801@2ndquadrant.com

Per Thomas Munro in the course of his SKIP LOCKED feature submission,
who also provided one of the isolation test specs.

Backpatch to 9.4, because that's as far back as it applies without
conflicts (although the bug goes all the way back).  To that branch also
backpatch Thomas Munro's new NOWAIT test cases, committed in master by
Heikki as commit 9ee16b49f0aac819bd4823d9b94485ef608b34e8 .

15 files changed:
src/backend/executor/execMain.c
src/backend/executor/nodeLockRows.c
src/include/executor/executor.h
src/test/isolation/expected/nowait-2.out [new file with mode: 0644]
src/test/isolation/expected/nowait-3.out [new file with mode: 0644]
src/test/isolation/expected/nowait-4.out [new file with mode: 0644]
src/test/isolation/expected/nowait-4_1.out [new file with mode: 0644]
src/test/isolation/expected/nowait-5.out [new file with mode: 0644]
src/test/isolation/expected/nowait.out [new file with mode: 0644]
src/test/isolation/isolation_schedule
src/test/isolation/specs/nowait-2.spec [new file with mode: 0644]
src/test/isolation/specs/nowait-3.spec [new file with mode: 0644]
src/test/isolation/specs/nowait-4.spec [new file with mode: 0644]
src/test/isolation/specs/nowait-5.spec [new file with mode: 0644]
src/test/isolation/specs/nowait.spec [new file with mode: 0644]

index 072c7df0ada831616f58f81b1607c5b76e7331d7..01eda70f0544afdc8350c2fa3b9cbe394fb03125 100644 (file)
@@ -1863,7 +1863,7 @@ EvalPlanQual(EState *estate, EPQState *epqstate,
        /*
         * Get and lock the updated version of the row; if fail, return NULL.
         */
-       copyTuple = EvalPlanQualFetch(estate, relation, lockmode,
+       copyTuple = EvalPlanQualFetch(estate, relation, lockmode, false /* wait */,
                                                                  tid, priorXmax);
 
        if (copyTuple == NULL)
@@ -1922,6 +1922,7 @@ EvalPlanQual(EState *estate, EPQState *epqstate,
  *     estate - executor state data
  *     relation - table containing tuple
  *     lockmode - requested tuple lock mode
+ *     noWait - wait mode to pass to heap_lock_tuple
  *     *tid - t_ctid from the outdated tuple (ie, next updated version)
  *     priorXmax - t_xmax from the outdated tuple
  *
@@ -1934,7 +1935,7 @@ EvalPlanQual(EState *estate, EPQState *epqstate,
  * but we use "int" to avoid having to include heapam.h in executor.h.
  */
 HeapTuple
-EvalPlanQualFetch(EState *estate, Relation relation, int lockmode,
+EvalPlanQualFetch(EState *estate, Relation relation, int lockmode, bool noWait,
                                  ItemPointer tid, TransactionId priorXmax)
 {
        HeapTuple       copyTuple = NULL;
@@ -1978,14 +1979,23 @@ EvalPlanQualFetch(EState *estate, Relation relation, int lockmode,
 
                        /*
                         * If tuple is being updated by other transaction then we have to
-                        * wait for its commit/abort.
+                        * wait for its commit/abort, or die trying.
                         */
                        if (TransactionIdIsValid(SnapshotDirty.xmax))
                        {
                                ReleaseBuffer(buffer);
-                               XactLockTableWait(SnapshotDirty.xmax,
-                                                                 relation, &tuple.t_data->t_ctid,
-                                                                 XLTW_FetchUpdated);
+                               if (noWait)
+                               {
+                                       if (!ConditionalXactLockTableWait(SnapshotDirty.xmax))
+                                               ereport(ERROR,
+                                                               (errcode(ERRCODE_LOCK_NOT_AVAILABLE),
+                                                                errmsg("could not obtain lock on row in relation \"%s\"",
+                                                                               RelationGetRelationName(relation))));
+                               }
+                               else
+                                       XactLockTableWait(SnapshotDirty.xmax,
+                                                                         relation, &tuple.t_data->t_ctid,
+                                                                         XLTW_FetchUpdated);
                                continue;               /* loop back to repeat heap_fetch */
                        }
 
@@ -2012,7 +2022,7 @@ EvalPlanQualFetch(EState *estate, Relation relation, int lockmode,
                         */
                        test = heap_lock_tuple(relation, &tuple,
                                                                   estate->es_output_cid,
-                                                                  lockmode, false /* wait */ ,
+                                                                  lockmode, noWait,
                                                                   false, &buffer, &hufd);
                        /* We now have two pins on the buffer, get rid of one */
                        ReleaseBuffer(buffer);
index 298d4b4d0173b71a9fbfffdb2845cb0b0123b8e4..814b61efcbaea189cbccf0eed46f3f18a2d89f7b 100644 (file)
@@ -170,7 +170,7 @@ lnext:
                                }
 
                                /* updated, so fetch and lock the updated version */
-                               copyTuple = EvalPlanQualFetch(estate, erm->relation, lockmode,
+                               copyTuple = EvalPlanQualFetch(estate, erm->relation, lockmode, erm->noWait,
                                                                                          &hufd.ctid, hufd.xmax);
 
                                if (copyTuple == NULL)
index 239aff3208827fc5ae01b44c3c5e90ef5e129cb4..02661350d94b85a9069b74377f6e9a7c40de16b7 100644 (file)
@@ -199,7 +199,7 @@ extern TupleTableSlot *EvalPlanQual(EState *estate, EPQState *epqstate,
                         Relation relation, Index rti, int lockmode,
                         ItemPointer tid, TransactionId priorXmax);
 extern HeapTuple EvalPlanQualFetch(EState *estate, Relation relation,
-                                 int lockmode, ItemPointer tid, TransactionId priorXmax);
+                                 int lockmode, bool noWait, ItemPointer tid, TransactionId priorXmax);
 extern void EvalPlanQualInit(EPQState *epqstate, EState *estate,
                                 Plan *subplan, List *auxrowmarks, int epqParam);
 extern void EvalPlanQualSetPlan(EPQState *epqstate,
diff --git a/src/test/isolation/expected/nowait-2.out b/src/test/isolation/expected/nowait-2.out
new file mode 100644 (file)
index 0000000..6e24bbb
--- /dev/null
@@ -0,0 +1,43 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s1a s2a s2b s1b s2c
+step s1a: SELECT * FROM foo FOR SHARE NOWAIT;
+id             data           
+
+1              x              
+step s2a: SELECT * FROM foo FOR SHARE NOWAIT;
+id             data           
+
+1              x              
+step s2b: SELECT * FROM foo FOR UPDATE NOWAIT;
+ERROR:  could not obtain lock on row in relation "foo"
+step s1b: COMMIT;
+step s2c: COMMIT;
+
+starting permutation: s2a s1a s2b s1b s2c
+step s2a: SELECT * FROM foo FOR SHARE NOWAIT;
+id             data           
+
+1              x              
+step s1a: SELECT * FROM foo FOR SHARE NOWAIT;
+id             data           
+
+1              x              
+step s2b: SELECT * FROM foo FOR UPDATE NOWAIT;
+ERROR:  could not obtain lock on row in relation "foo"
+step s1b: COMMIT;
+step s2c: COMMIT;
+
+starting permutation: s2a s2b s1a s1b s2c
+step s2a: SELECT * FROM foo FOR SHARE NOWAIT;
+id             data           
+
+1              x              
+step s2b: SELECT * FROM foo FOR UPDATE NOWAIT;
+id             data           
+
+1              x              
+step s1a: SELECT * FROM foo FOR SHARE NOWAIT;
+ERROR:  could not obtain lock on row in relation "foo"
+step s1b: COMMIT;
+step s2c: COMMIT;
diff --git a/src/test/isolation/expected/nowait-3.out b/src/test/isolation/expected/nowait-3.out
new file mode 100644 (file)
index 0000000..8444646
--- /dev/null
@@ -0,0 +1,17 @@
+Parsed test spec with 3 sessions
+
+starting permutation: s1a s2a s3a s1b s2b s3b
+step s1a: SELECT * FROM foo FOR UPDATE;
+id             data           
+
+1              x              
+step s2a: SELECT * FROM foo FOR UPDATE; <waiting ...>
+step s3a: SELECT * FROM foo FOR UPDATE NOWAIT;
+ERROR:  could not obtain lock on row in relation "foo"
+step s1b: COMMIT;
+step s2a: <... completed>
+id             data           
+
+1              x              
+step s2b: COMMIT;
+step s3b: COMMIT;
diff --git a/src/test/isolation/expected/nowait-4.out b/src/test/isolation/expected/nowait-4.out
new file mode 100644 (file)
index 0000000..26f59be
--- /dev/null
@@ -0,0 +1,19 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s2a s1a s2b s2c s2d s2e s1b s2f
+step s2a: SELECT pg_advisory_lock(0);
+pg_advisory_lock
+
+               
+step s1a: SELECT * FROM foo WHERE pg_advisory_lock(0) IS NOT NULL FOR UPDATE NOWAIT; <waiting ...>
+step s2b: UPDATE foo SET data = data;
+step s2c: BEGIN;
+step s2d: UPDATE foo SET data = data;
+step s2e: SELECT pg_advisory_unlock(0);
+pg_advisory_unlock
+
+t              
+step s1a: <... completed>
+error in steps s2e s1a: ERROR:  could not obtain lock on row in relation "foo"
+step s1b: COMMIT;
+step s2f: COMMIT;
diff --git a/src/test/isolation/expected/nowait-4_1.out b/src/test/isolation/expected/nowait-4_1.out
new file mode 100644 (file)
index 0000000..959d51b
--- /dev/null
@@ -0,0 +1,19 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s2a s1a s2b s2c s2d s2e s1b s2f
+step s2a: SELECT pg_advisory_lock(0);
+pg_advisory_lock
+
+               
+step s1a: SELECT * FROM foo WHERE pg_advisory_lock(0) IS NOT NULL FOR UPDATE NOWAIT; <waiting ...>
+step s2b: UPDATE foo SET data = data;
+step s2c: BEGIN;
+step s2d: UPDATE foo SET data = data;
+step s2e: SELECT pg_advisory_unlock(0);
+pg_advisory_unlock
+
+t              
+step s1a: <... completed>
+error in steps s2e s1a: ERROR:  could not serialize access due to concurrent update
+step s1b: COMMIT;
+step s2f: COMMIT;
diff --git a/src/test/isolation/expected/nowait-5.out b/src/test/isolation/expected/nowait-5.out
new file mode 100644 (file)
index 0000000..c88aae5
--- /dev/null
@@ -0,0 +1,37 @@
+Parsed test spec with 3 sessions
+
+starting permutation: sl1_prep upd_getlock sl1_exec upd_doupdate lk1_doforshare upd_releaselock
+step sl1_prep: 
+       PREPARE sl1_run AS SELECT id FROM test_nowait WHERE pg_advisory_lock(0) is not null FOR UPDATE NOWAIT;
+
+step upd_getlock: 
+       SELECT pg_advisory_lock(0);
+
+pg_advisory_lock
+
+               
+step sl1_exec: 
+       BEGIN ISOLATION LEVEL READ COMMITTED;
+       EXECUTE sl1_run;
+       SELECT xmin, xmax, ctid, * FROM test_nowait;
+ <waiting ...>
+step upd_doupdate: 
+       BEGIN ISOLATION LEVEL READ COMMITTED;
+       UPDATE test_nowait SET value = value WHERE id % 2 = 0;
+       COMMIT;
+
+step lk1_doforshare: 
+       BEGIN ISOLATION LEVEL READ COMMITTED;
+       SELECT id FROM test_nowait WHERE id % 2 = 0 FOR SHARE;
+
+id             
+
+2              
+step upd_releaselock: 
+       SELECT pg_advisory_unlock(0);
+
+pg_advisory_unlock
+
+t              
+step sl1_exec: <... completed>
+error in steps upd_releaselock sl1_exec: ERROR:  could not obtain lock on row in relation "test_nowait"
diff --git a/src/test/isolation/expected/nowait.out b/src/test/isolation/expected/nowait.out
new file mode 100644 (file)
index 0000000..a6343b4
--- /dev/null
@@ -0,0 +1,65 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s1a s1b s2a s2b
+step s1a: SELECT * FROM foo FOR UPDATE NOWAIT;
+id             data           
+
+1              x              
+step s1b: COMMIT;
+step s2a: SELECT * FROM foo FOR UPDATE NOWAIT;
+id             data           
+
+1              x              
+step s2b: COMMIT;
+
+starting permutation: s1a s2a s1b s2b
+step s1a: SELECT * FROM foo FOR UPDATE NOWAIT;
+id             data           
+
+1              x              
+step s2a: SELECT * FROM foo FOR UPDATE NOWAIT;
+ERROR:  could not obtain lock on row in relation "foo"
+step s1b: COMMIT;
+step s2b: COMMIT;
+
+starting permutation: s1a s2a s2b s1b
+step s1a: SELECT * FROM foo FOR UPDATE NOWAIT;
+id             data           
+
+1              x              
+step s2a: SELECT * FROM foo FOR UPDATE NOWAIT;
+ERROR:  could not obtain lock on row in relation "foo"
+step s2b: COMMIT;
+step s1b: COMMIT;
+
+starting permutation: s2a s1a s1b s2b
+step s2a: SELECT * FROM foo FOR UPDATE NOWAIT;
+id             data           
+
+1              x              
+step s1a: SELECT * FROM foo FOR UPDATE NOWAIT;
+ERROR:  could not obtain lock on row in relation "foo"
+step s1b: COMMIT;
+step s2b: COMMIT;
+
+starting permutation: s2a s1a s2b s1b
+step s2a: SELECT * FROM foo FOR UPDATE NOWAIT;
+id             data           
+
+1              x              
+step s1a: SELECT * FROM foo FOR UPDATE NOWAIT;
+ERROR:  could not obtain lock on row in relation "foo"
+step s2b: COMMIT;
+step s1b: COMMIT;
+
+starting permutation: s2a s2b s1a s1b
+step s2a: SELECT * FROM foo FOR UPDATE NOWAIT;
+id             data           
+
+1              x              
+step s2b: COMMIT;
+step s1a: SELECT * FROM foo FOR UPDATE NOWAIT;
+id             data           
+
+1              x              
+step s1b: COMMIT;
index 36acec1ca8e2a7a7f1045db29673696775b8f8c6..3241a91505cf05f6db280673c29b79a50f6513c5 100644 (file)
@@ -22,6 +22,11 @@ test: aborted-keyrevoke
 test: multixact-no-deadlock
 test: multixact-no-forget
 test: propagate-lock-delete
+test: nowait
+test: nowait-2
+test: nowait-3
+test: nowait-4
+test: nowait-5
 test: drop-index-concurrently-1
 test: alter-table-1
 test: timeouts
diff --git a/src/test/isolation/specs/nowait-2.spec b/src/test/isolation/specs/nowait-2.spec
new file mode 100644 (file)
index 0000000..007d023
--- /dev/null
@@ -0,0 +1,37 @@
+# Test NOWAIT with multixact locks.
+
+setup
+{
+  CREATE TABLE foo (
+       id int PRIMARY KEY,
+       data text NOT NULL
+  );
+  INSERT INTO foo VALUES (1, 'x');
+}
+
+teardown
+{
+  DROP TABLE foo;
+}
+
+session "s1"
+setup          { BEGIN; }
+step "s1a"     { SELECT * FROM foo FOR SHARE NOWAIT; }
+step "s1b"     { COMMIT; }
+
+session "s2"
+setup          { BEGIN; }
+step "s2a"     { SELECT * FROM foo FOR SHARE NOWAIT; }
+step "s2b"     { SELECT * FROM foo FOR UPDATE NOWAIT; }
+step "s2c"     { COMMIT; }
+
+# s1 and s2 both get SHARE lock, creating a multixact lock, then s2
+# tries to upgrade to UPDATE but aborts because it cannot acquire a
+# multi-xact lock
+permutation "s1a" "s2a" "s2b" "s1b" "s2c"
+# the same but with the SHARE locks acquired in a different order, so
+# s2 again aborts because it can't acquired a multi-xact lock
+permutation "s2a" "s1a" "s2b" "s1b" "s2c"
+# s2 acquires SHARE then UPDATE, then s1 tries to acquire SHARE but
+# can't so aborts because it can't acquire a regular lock
+permutation "s2a" "s2b" "s1a" "s1b" "s2c"
\ No newline at end of file
diff --git a/src/test/isolation/specs/nowait-3.spec b/src/test/isolation/specs/nowait-3.spec
new file mode 100644 (file)
index 0000000..58dec7f
--- /dev/null
@@ -0,0 +1,33 @@
+# Test NOWAIT with tuple locks.
+
+setup
+{
+  CREATE TABLE foo (
+       id int PRIMARY KEY,
+       data text NOT NULL
+  );
+  INSERT INTO foo VALUES (1, 'x');
+}
+
+teardown
+{
+  DROP TABLE foo;
+}
+
+session "s1"
+setup          { BEGIN; }
+step "s1a"     { SELECT * FROM foo FOR UPDATE; }
+step "s1b"     { COMMIT; }
+
+session "s2"
+setup          { BEGIN; }
+step "s2a"     { SELECT * FROM foo FOR UPDATE; }
+step "s2b"     { COMMIT; }
+
+session "s3"
+setup          { BEGIN; }
+step "s3a"     { SELECT * FROM foo FOR UPDATE NOWAIT; }
+step "s3b"     { COMMIT; }
+
+# s3 skips to second record due to tuple lock held by s2
+permutation "s1a" "s2a" "s3a" "s1b" "s2b" "s3b"
\ No newline at end of file
diff --git a/src/test/isolation/specs/nowait-4.spec b/src/test/isolation/specs/nowait-4.spec
new file mode 100644 (file)
index 0000000..5dbebca
--- /dev/null
@@ -0,0 +1,30 @@
+# Test NOWAIT with an updated tuple chain.
+
+setup
+{
+  CREATE TABLE foo (
+       id int PRIMARY KEY,
+       data text NOT NULL
+  );
+  INSERT INTO foo VALUES (1, 'x');
+}
+
+teardown
+{
+  DROP TABLE foo;
+}
+
+session "s1"
+setup          { BEGIN; }
+step "s1a"     { SELECT * FROM foo WHERE pg_advisory_lock(0) IS NOT NULL FOR UPDATE NOWAIT; }
+step "s1b"     { COMMIT; }
+
+session "s2"
+step "s2a"     { SELECT pg_advisory_lock(0); }
+step "s2b"     { UPDATE foo SET data = data; }
+step "s2c"     { BEGIN; }
+step "s2d"     { UPDATE foo SET data = data; }
+step "s2e"     { SELECT pg_advisory_unlock(0); }
+step "s2f"     { COMMIT; }
+
+permutation "s2a" "s1a" "s2b" "s2c" "s2d" "s2e" "s1b" "s2f"
diff --git a/src/test/isolation/specs/nowait-5.spec b/src/test/isolation/specs/nowait-5.spec
new file mode 100644 (file)
index 0000000..75e9462
--- /dev/null
@@ -0,0 +1,57 @@
+# Test NOWAIT on an updated tuple chain
+
+setup
+{
+
+  DROP TABLE IF EXISTS test_nowait;
+  CREATE TABLE test_nowait (
+       id integer PRIMARY KEY,
+       value integer not null
+  );
+
+  INSERT INTO test_nowait
+  SELECT x,x FROM generate_series(1,2) x;
+}
+
+teardown
+{
+  DROP TABLE test_nowait;
+}
+
+session "sl1"
+step "sl1_prep" {
+       PREPARE sl1_run AS SELECT id FROM test_nowait WHERE pg_advisory_lock(0) is not null FOR UPDATE NOWAIT;
+}
+step "sl1_exec" {
+       BEGIN ISOLATION LEVEL READ COMMITTED;
+       EXECUTE sl1_run;
+       SELECT xmin, xmax, ctid, * FROM test_nowait;
+}
+teardown { COMMIT; }
+
+# A session that's used for an UPDATE of the rows to be locked, for when we're testing ctid
+# chain following.
+session "upd"
+step "upd_getlock" {
+       SELECT pg_advisory_lock(0);
+}
+step "upd_doupdate" {
+       BEGIN ISOLATION LEVEL READ COMMITTED;
+       UPDATE test_nowait SET value = value WHERE id % 2 = 0;
+       COMMIT;
+}
+step "upd_releaselock" {
+       SELECT pg_advisory_unlock(0);
+}
+
+# A session that acquires locks that sl1 is supposed to avoid blocking on
+session "lk1"
+step "lk1_doforshare" {
+       BEGIN ISOLATION LEVEL READ COMMITTED;
+       SELECT id FROM test_nowait WHERE id % 2 = 0 FOR SHARE;
+}
+teardown {
+       COMMIT;
+}
+
+permutation "sl1_prep" "upd_getlock" "sl1_exec" "upd_doupdate" "lk1_doforshare" "upd_releaselock"
diff --git a/src/test/isolation/specs/nowait.spec b/src/test/isolation/specs/nowait.spec
new file mode 100644 (file)
index 0000000..73580cd
--- /dev/null
@@ -0,0 +1,25 @@
+# Test NOWAIT when regular row locks can't be acquired.
+
+setup
+{
+  CREATE TABLE foo (
+       id int PRIMARY KEY,
+       data text NOT NULL
+  );
+  INSERT INTO foo VALUES (1, 'x');
+}
+
+teardown
+{
+  DROP TABLE foo;
+}
+
+session "s1"
+setup          { BEGIN; }
+step "s1a"     { SELECT * FROM foo FOR UPDATE NOWAIT; }
+step "s1b"     { COMMIT; }
+
+session "s2"
+setup          { BEGIN; }
+step "s2a"     { SELECT * FROM foo FOR UPDATE NOWAIT; }
+step "s2b"     { COMMIT; }