Andres Freund escribió:
> On 2013-11-25 17:10:39 -0300, Alvaro Herrera wrote:
> > Let me point out that this is exactly the same code that would be
> > affected by my proposed fix for #8434, which would have this check the
> > updateXid in all cases, not only in KEYS_UPDATED as currently.
>
> Hm. I don't see a patch in any of the mails about #8434 although I seem
> to remember talking with you about one. Maybe that was on IM?
Ah, yeah, that's possible. The patch I proposed back then is attached
here. I haven't merged this to latest master; this applies cleanly on
top of 16a906f5350
> > I don't understand your comment about "can't check for committed". I
> > mean, if the updating transaction committed, then we need to return
> > failure to caller, HeapTupleUpdated.
>
> I mean that in the !KEYS_UPDATED case we don't need to abort if we're
> only acquiring a key share...
Hm, I think that's correct -- we don't need to abort. But we still need
to wait until the updater completes. So this proposed patch is not the
full story.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
commit 71aa5c689c153a03bc09e7f9f433ca55a6f00a23
Author: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Mon Oct 7 18:33:59 2013 -0300
Don't skip waiting on tuple updaters if key is unchanged
My commit 0ac5ad5134 contained a bug that allowed lockers to avoid
sleeping waiting for a future version of the tuple being locked to be
released. This happens while following the update chain to lock the
updated versions of the row. Prior to that commit, there was no such
behavior, so backpatch no further than 9.3.
The isolation test changes illustrate the difference in behavior. Some
transactions which were previously allowed to continue must now sleep
waiting for the other transaction; but there is still more concurrency
than there was prior to the mentioned commit.
Per bug #8434 reported by Tomonari Katsumata
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index ead3d69..530a001 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -4832,15 +4832,12 @@ l4:
xmax = HeapTupleHeaderGetRawXmax(mytup.t_data);
/*
- * If this tuple is updated and the key has been modified (or
- * deleted), what we do depends on the status of the updating
- * transaction: if it's live, we sleep until it finishes; if it has
- * committed, we have to fail (i.e. return HeapTupleUpdated); if it
- * aborted, we ignore it. For updates that didn't touch the key, we
- * can just plough ahead.
+ * If this tuple is updated, what we do depends on the status of the
+ * updating transaction: if it's live, we sleep until it finishes; if
+ * it has committed, we have to fail (i.e. return HeapTupleUpdated); if
+ * it aborted, we ignore it.
*/
- if (!(old_infomask & HEAP_XMAX_INVALID) &&
- (mytup.t_data->t_infomask2 & HEAP_KEYS_UPDATED))
+ if (!(old_infomask & HEAP_XMAX_INVALID))
{
TransactionId update_xid;
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 791f336..efe7d5a 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1970,7 +1970,7 @@ EvalPlanQualFetch(EState *estate, Relation relation, int lockmode,
* heap_lock_tuple() will throw an error, and so would any later
* attempt to update or delete the tuple. (We need not check cmax
* because HeapTupleSatisfiesDirty will consider a tuple deleted
- * by our transaction dead, regardless of cmax.) Wee just checked
+ * by our transaction dead, regardless of cmax.) We just checked
* that priorXmax == xmin, so we can test that variable instead of
* doing HeapTupleHeaderGetXmin again.
*/
diff --git a/src/test/isolation/expected/fk-deadlock.out b/src/test/isolation/expected/fk-deadlock.out
index 69eac88..bbd1867 100644
--- a/src/test/isolation/expected/fk-deadlock.out
+++ b/src/test/isolation/expected/fk-deadlock.out
@@ -11,25 +11,22 @@ step s2c: COMMIT;
starting permutation: s1i s1u s2i s1c s2u s2c
step s1i: INSERT INTO child VALUES (1, 1);
step s1u: UPDATE parent SET aux = 'bar';
-step s2i: INSERT INTO child VALUES (2, 1);
+step s2i: INSERT INTO child VALUES (2, 1); <waiting ...>
step s1c: COMMIT;
+step s2i: <... completed>
step s2u: UPDATE parent SET aux = 'baz';
step s2c: COMMIT;
starting permutation: s1i s1u s2i s2u s1c s2c
step s1i: INSERT INTO child VALUES (1, 1);
step s1u: UPDATE parent SET aux = 'bar';
-step s2i: INSERT INTO child VALUES (2, 1);
-step s2u: UPDATE parent SET aux = 'baz'; <waiting ...>
-step s1c: COMMIT;
-step s2u: <... completed>
-step s2c: COMMIT;
+step s2i: INSERT INTO child VALUES (2, 1); <waiting ...>
+invalid permutation detected
starting permutation: s1i s1u s2i s2u s2c s1c
step s1i: INSERT INTO child VALUES (1, 1);
step s1u: UPDATE parent SET aux = 'bar';
-step s2i: INSERT INTO child VALUES (2, 1);
-step s2u: UPDATE parent SET aux = 'baz'; <waiting ...>
+step s2i: INSERT INTO child VALUES (2, 1); <waiting ...>
invalid permutation detected
starting permutation: s1i s2i s1u s1c s2u s2c
@@ -131,24 +128,21 @@ step s1c: COMMIT;
starting permutation: s2i s2u s1i s1u s1c s2c
step s2i: INSERT INTO child VALUES (2, 1);
step s2u: UPDATE parent SET aux = 'baz';
-step s1i: INSERT INTO child VALUES (1, 1);
-step s1u: UPDATE parent SET aux = 'bar'; <waiting ...>
+step s1i: INSERT INTO child VALUES (1, 1); <waiting ...>
invalid permutation detected
starting permutation: s2i s2u s1i s1u s2c s1c
step s2i: INSERT INTO child VALUES (2, 1);
step s2u: UPDATE parent SET aux = 'baz';
-step s1i: INSERT INTO child VALUES (1, 1);
-step s1u: UPDATE parent SET aux = 'bar'; <waiting ...>
-step s2c: COMMIT;
-step s1u: <... completed>
-step s1c: COMMIT;
+step s1i: INSERT INTO child VALUES (1, 1); <waiting ...>
+invalid permutation detected
starting permutation: s2i s2u s1i s2c s1u s1c
step s2i: INSERT INTO child VALUES (2, 1);
step s2u: UPDATE parent SET aux = 'baz';
-step s1i: INSERT INTO child VALUES (1, 1);
+step s1i: INSERT INTO child VALUES (1, 1); <waiting ...>
step s2c: COMMIT;
+step s1i: <... completed>
step s1u: UPDATE parent SET aux = 'bar';
step s1c: COMMIT;
diff --git a/src/test/isolation/expected/fk-deadlock_1.out b/src/test/isolation/expected/fk-deadlock_1.out
index 6687505..d83c34f 100644
--- a/src/test/isolation/expected/fk-deadlock_1.out
+++ b/src/test/isolation/expected/fk-deadlock_1.out
@@ -11,27 +11,24 @@ step s2c: COMMIT;
starting permutation: s1i s1u s2i s1c s2u s2c
step s1i: INSERT INTO child VALUES (1, 1);
step s1u: UPDATE parent SET aux = 'bar';
-step s2i: INSERT INTO child VALUES (2, 1);
+step s2i: INSERT INTO child VALUES (2, 1); <waiting ...>
step s1c: COMMIT;
+step s2i: <... completed>
+error in steps s1c s2i: ERROR: could not serialize access due to concurrent update
step s2u: UPDATE parent SET aux = 'baz';
-ERROR: could not serialize access due to concurrent update
+ERROR: current transaction is aborted, commands ignored until end of transaction block
step s2c: COMMIT;
starting permutation: s1i s1u s2i s2u s1c s2c
step s1i: INSERT INTO child VALUES (1, 1);
step s1u: UPDATE parent SET aux = 'bar';
-step s2i: INSERT INTO child VALUES (2, 1);
-step s2u: UPDATE parent SET aux = 'baz'; <waiting ...>
-step s1c: COMMIT;
-step s2u: <... completed>
-error in steps s1c s2u: ERROR: could not serialize access due to concurrent update
-step s2c: COMMIT;
+step s2i: INSERT INTO child VALUES (2, 1); <waiting ...>
+invalid permutation detected
starting permutation: s1i s1u s2i s2u s2c s1c
step s1i: INSERT INTO child VALUES (1, 1);
step s1u: UPDATE parent SET aux = 'bar';
-step s2i: INSERT INTO child VALUES (2, 1);
-step s2u: UPDATE parent SET aux = 'baz'; <waiting ...>
+step s2i: INSERT INTO child VALUES (2, 1); <waiting ...>
invalid permutation detected
starting permutation: s1i s2i s1u s1c s2u s2c
@@ -141,27 +138,24 @@ step s1c: COMMIT;
starting permutation: s2i s2u s1i s1u s1c s2c
step s2i: INSERT INTO child VALUES (2, 1);
step s2u: UPDATE parent SET aux = 'baz';
-step s1i: INSERT INTO child VALUES (1, 1);
-step s1u: UPDATE parent SET aux = 'bar'; <waiting ...>
+step s1i: INSERT INTO child VALUES (1, 1); <waiting ...>
invalid permutation detected
starting permutation: s2i s2u s1i s1u s2c s1c
step s2i: INSERT INTO child VALUES (2, 1);
step s2u: UPDATE parent SET aux = 'baz';
-step s1i: INSERT INTO child VALUES (1, 1);
-step s1u: UPDATE parent SET aux = 'bar'; <waiting ...>
-step s2c: COMMIT;
-step s1u: <... completed>
-error in steps s2c s1u: ERROR: could not serialize access due to concurrent update
-step s1c: COMMIT;
+step s1i: INSERT INTO child VALUES (1, 1); <waiting ...>
+invalid permutation detected
starting permutation: s2i s2u s1i s2c s1u s1c
step s2i: INSERT INTO child VALUES (2, 1);
step s2u: UPDATE parent SET aux = 'baz';
-step s1i: INSERT INTO child VALUES (1, 1);
+step s1i: INSERT INTO child VALUES (1, 1); <waiting ...>
step s2c: COMMIT;
+step s1i: <... completed>
+error in steps s2c s1i: ERROR: could not serialize access due to concurrent update
step s1u: UPDATE parent SET aux = 'bar';
-ERROR: could not serialize access due to concurrent update
+ERROR: current transaction is aborted, commands ignored until end of transaction block
step s1c: COMMIT;
starting permutation: s2i s2u s2c s1i s1u s1c
diff --git a/src/test/isolation/expected/lock-update-delete.out b/src/test/isolation/expected/lock-update-delete.out
index 344a6ec..66b1463 100644
--- a/src/test/isolation/expected/lock-update-delete.out
+++ b/src/test/isolation/expected/lock-update-delete.out
@@ -46,11 +46,11 @@ step s2_unlock: SELECT pg_advisory_unlock(0);
pg_advisory_unlock
t
+step s2c: COMMIT;
step s1l: <... completed>
key value
-1 1
-step s2c: COMMIT;
+1 2
starting permutation: s2b s1l s2u s2_blocker1 s2_unlock s2r
pg_advisory_lock
@@ -100,11 +100,11 @@ step s2_unlock: SELECT pg_advisory_unlock(0);
pg_advisory_unlock
t
+step s2r: ROLLBACK;
step s1l: <... completed>
key value
1 1
-step s2r: ROLLBACK;
starting permutation: s2b s1l s2u s2_blocker1 s2c s2_unlock
pg_advisory_lock
diff --git a/src/test/isolation/expected/lock-update-delete_1.out b/src/test/isolation/expected/lock-update-delete_1.out
index 4203573..ff5d2da 100644
--- a/src/test/isolation/expected/lock-update-delete_1.out
+++ b/src/test/isolation/expected/lock-update-delete_1.out
@@ -44,11 +44,9 @@ step s2_unlock: SELECT pg_advisory_unlock(0);
pg_advisory_unlock
t
-step s1l: <... completed>
-key value
-
-1 1
step s2c: COMMIT;
+step s1l: <... completed>
+error in steps s2c s1l: ERROR: could not serialize access due to concurrent update
starting permutation: s2b s1l s2u s2_blocker1 s2_unlock s2r
pg_advisory_lock
@@ -98,11 +96,11 @@ step s2_unlock: SELECT pg_advisory_unlock(0);
pg_advisory_unlock
t
+step s2r: ROLLBACK;
step s1l: <... completed>
key value
1 1
-step s2r: ROLLBACK;
starting permutation: s2b s1l s2u s2_blocker1 s2c s2_unlock
pg_advisory_lock
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
