>From 76ff647f8997221de2a6981a51859d12a6b70276 Mon Sep 17 00:00:00 2001 From: Craig Ringer Date: Fri, 2 Aug 2013 14:18:34 +0800 Subject: [PATCH] Add noWait param to EvalPlanQualFetch FOR SHARE|UPDATE NOWAIT would still wait if they had to follow a ctid chain because EvalPlanQualFetch couldn't tell if a statement was NOWAIT and would just assume it should block. --- src/backend/executor/execMain.c | 7 +-- src/backend/executor/nodeLockRows.c | 2 +- src/include/executor/executor.h | 2 +- .../expected/for-updateshare-nowait-nonblock.out | 41 +++++++++++++++ src/test/isolation/isolation_schedule | 1 + .../specs/for-updateshare-nowait-nonblock.spec | 58 ++++++++++++++++++++++ 6 files changed, 106 insertions(+), 5 deletions(-) create mode 100644 src/test/isolation/expected/for-updateshare-nowait-nonblock.out create mode 100644 src/test/isolation/specs/for-updateshare-nowait-nonblock.spec diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 038f064..412dc9a 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -1839,7 +1839,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) @@ -1898,6 +1898,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 * @@ -1910,7 +1911,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; @@ -1986,7 +1987,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); diff --git a/src/backend/executor/nodeLockRows.c b/src/backend/executor/nodeLockRows.c index 5b5c705..efee8a3 100644 --- a/src/backend/executor/nodeLockRows.c +++ b/src/backend/executor/nodeLockRows.c @@ -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) diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 75841c8..9c593a6 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -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/for-updateshare-nowait-nonblock.out b/src/test/isolation/expected/for-updateshare-nowait-nonblock.out new file mode 100644 index 0000000..d86b2e6 --- /dev/null +++ b/src/test/isolation/expected/for-updateshare-nowait-nonblock.out @@ -0,0 +1,41 @@ +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; + +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 +4 +6 +8 +10 +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/isolation_schedule b/src/test/isolation/isolation_schedule index 081e11f..15053ca 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -21,3 +21,4 @@ test: delete-abort-savept-2 test: aborted-keyrevoke test: drop-index-concurrently-1 test: timeouts +test: for-updateshare-nowait-nonblock diff --git a/src/test/isolation/specs/for-updateshare-nowait-nonblock.spec b/src/test/isolation/specs/for-updateshare-nowait-nonblock.spec new file mode 100644 index 0000000..bb1ef98 --- /dev/null +++ b/src/test/isolation/specs/for-updateshare-nowait-nonblock.spec @@ -0,0 +1,58 @@ +# Test excersising a case where NOWAIT still blocked in EvalPlanQualFetch. +# Verifies that NOWAIT is now respected in this case. + +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,10) 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" -- 1.8.3.1