Alvaro Herrera escribió: > Andres Freund escribió: > This seems simple to handle by adding the check you propose to the loop. > Basically if the xmax doesn't match the xmin, we reached the end, > there's nothing more to lock and we can return success without any > further work:
As mentioned in the thread for bug #8434, the complete working patch for this is attached. > > b) Check whether a chain element actually aborted - currently we're > > only doing that in the HEAP_KEYS_UPDATED updated case, but that seems > > wrong (we can't check for committed tho!). > > 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. I posted a patch for this problem in the thread about #8434. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
commit 1a4cadc729e724fa1aa6f260a4988b3615fccd1b Author: Alvaro Herrera <alvhe...@alvh.no-ip.org> Date: Wed Nov 27 12:12:42 2013 -0300 Compare new tuple's Xmin to previous Xmax while following an update chain Not doing so causes us to traverse an update chain that has been broken by concurrent page pruning. All other code that traverses update chains is careful to do this check, so replicate it here too. Failure to do so leads to erroneous CLOG, subtrans or multixact lookups. Per bug report by J Smith in cadfupgc5bmtv-yg9znxv-vcfkb+jprqs7m2oesqxam_4z1j...@mail.gmail.com diagnosed by Andres Freund. diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index c3b2108..5eb45ff 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -4819,6 +4819,7 @@ heap_lock_updated_tuple_rec(Relation rel, ItemPointer tid, TransactionId xid, old_infomask; TransactionId xmax, new_xmax; + TransactionId priorXmax = InvalidTransactionId; ItemPointerCopy(tid, &tupid); @@ -4844,6 +4845,18 @@ l4: CHECK_FOR_INTERRUPTS(); LockBuffer(buf, BUFFER_LOCK_EXCLUSIVE); + /* + * Check the tuple XMIN against prior XMAX, if any. If we reached + * the end of the chain, we're done, so return success. + */ + if (TransactionIdIsValid(priorXmax) && + !TransactionIdEquals(HeapTupleHeaderGetXmin(mytup.t_data), + priorXmax)) + { + UnlockReleaseBuffer(buf); + return HeapTupleMayBeUpdated; + } + old_infomask = mytup.t_data->t_infomask; xmax = HeapTupleHeaderGetRawXmax(mytup.t_data); @@ -4944,6 +4957,7 @@ l4: } /* tail recursion */ + priorXmax = HeapTupleHeaderGetUpdateXid(mytup.t_data); ItemPointerCopy(&(mytup.t_data->t_ctid), &tupid); UnlockReleaseBuffer(buf); }
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers