SELECT FOR UPDATE regression in 9.5
Hello list
While testing an application with PostgreSQL 9.5, we experienced an issue
involving aborted subtransactions and SELECT FOR UPDATE. In certain
situations, a locking query doesn't return rows that should be visible and
already locked by the current transaction.
I bisected this down to commit 27846f02c176eebe7e08ce51ed4d52140454e196
"Optimize locking a tuple already locked by another subxact"
This issue is also reproducible on the current master branch. In an
assertions-enabled build, it traps an assertion in HeapTupleHeaderGetCmax
called by heap_lock_tuple. The following test case demonstrates the issue...
CREATE TABLE IF NOT EXISTS testcase(
id int PRIMARY KEY,
balance numeric
);
INSERT INTO testcase VALUES (1, 0);
BEGIN;
SELECT * FROM testcase WHERE testcase.id = 1 FOR UPDATE;
UPDATE testcase SET balance = balance + 400 WHERE id=1;
SAVEPOINT subxact;
UPDATE testcase SET balance = balance - 100 WHERE id=1;
ROLLBACK TO SAVEPOINT subxact;
-- "division by zero" shouldn't occur because I never deleted any rows
SELECT 1/count(*) from (
SELECT * FROM testcase WHERE id=1 FOR UPDATE
)x;
ROLLBACK;
Regards,
Marti Raudsepp
On 2016-09-06 6:02 PM, Marti Raudsepp wrote:
This issue is also reproducible on the current master branch. In an
assertions-enabled build, it traps an assertion in HeapTupleHeaderGetCmax
called by heap_lock_tuple. The following test case demonstrates the issue...
I think you found a reproducible test case for my bug in
48d3eade-98d3-8b9a-477e-1a8dc32a724d@joh.to. Thanks.
.m
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Marko Tiikkaja wrote:
On 2016-09-06 6:02 PM, Marti Raudsepp wrote:
This issue is also reproducible on the current master branch. In an
assertions-enabled build, it traps an assertion in HeapTupleHeaderGetCmax
called by heap_lock_tuple. The following test case demonstrates the issue...I think you found a reproducible test case for my bug in
48d3eade-98d3-8b9a-477e-1a8dc32a724d@joh.to. Thanks.
Ah, many thanks. I'll have a look.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Marti Raudsepp wrote:
Hello list
While testing an application with PostgreSQL 9.5, we experienced an issue
involving aborted subtransactions and SELECT FOR UPDATE. In certain
situations, a locking query doesn't return rows that should be visible and
already locked by the current transaction.
Okay, so the assertion failure is fixed by the attached patch. Also,
the division-by-zero that your test case says shouldn't occur doesn't
occur. But does it solve the larger problem of not returning rows that
should be visible?
Marko, does this fix your reported problem too? Both the assertion and
the overall test case that causes it to fire?
(The problem fixed by the patch is that we were trying to lock tuples
down the update chain, but one of the tuples in the chain had been
updated by an aborted subtransaction. Obviously, there is no point in
locking such a tuple because it effectively "doesn't exist" in the first
place.)
Thanks!
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
aborted-subxact.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index b8f4fe5..9846b9f 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -5403,6 +5403,16 @@ l4:
return HeapTupleMayBeUpdated;
}
+ /*
+ * Also check Xmin: if this tuple was created by an aborted
+ * transaction, we're done.
+ */
+ if (TransactionIdDidAbort(HeapTupleHeaderGetXmin(mytup.t_data)))
+ {
+ UnlockReleaseBuffer(buf);
+ return HeapTupleMayBeUpdated;
+ }
+
old_infomask = mytup.t_data->t_infomask;
old_infomask2 = mytup.t_data->t_infomask2;
xmax = HeapTupleHeaderGetRawXmax(mytup.t_data);
On 07/09/16 7:29 PM, Alvaro Herrera wrote:
Marko, does this fix your reported problem too? Both the assertion and
the overall test case that causes it to fire?
The test case never realized anything was wrong, but the assertion is
gone. So yup, problem solved on this end, at least.
.m
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
I ran the web application mentioned in Marti's original mail on a
patched Postgres server. It looks like it is working correctly now, no
more test failures.
Thanks
-Magnus
On 07.09.2016 21:49, Marko Tiikkaja wrote:
On 07/09/16 7:29 PM, Alvaro Herrera wrote:
Marko, does this fix your reported problem too? Both the assertion and
the overall test case that causes it to fire?The test case never realized anything was wrong, but the assertion is
gone. So yup, problem solved on this end, at least..m
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Magnus wrote:
I ran the web application mentioned in Marti's original mail on a patched
Postgres server. It looks like it is working correctly now, no more test
failures.
Thanks for the confirmation. I pushed the fix, along with the presented
test case.
I chose to backpatch all the way back to 9.3. While I couldn't find a
way to reproduce the misbehavior in 9.3/9.4, even with my alternative
proposed fix for bug #8470, it seems safer to get the fix everywhere
just in case there is a chance that this can be reproduced with multiple
sessions somehow.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers