Wong, Yi Wen wrote:
> My interpretation of README.HOT is the check is just to ensure the chain is 
> continuous; in which case the condition should be:
> 
> >                 if (TransactionIdIsValid(priorXmax) &&
> >                         !TransactionIdEquals(priorXmax, 
> > HeapTupleHeaderGetRawXmin(htup)))
> >                         break;
> 
> So the difference is GetRawXmin vs GetXmin, because otherwise we get the 
> FreezeId instead of the Xmin when the transaction happened

I independently arrived at the same conclusion.  Since I was trying with
9.3, the patch differs -- in the old version we must explicitely test
for the FrozenTransactionId value, instead of using GetRawXmin.
Attached is the patch I'm using, and my own oneliner test (pretty much
the same I posted earlier) seems to survive dozens of iterations without
showing any problem in REINDEX.

This patch is incomplete, since I think there are other places that need
to be patched in the same way (EvalPlanQualFetch? heap_get_latest_tid?).
Of course, for 9.4 and onwards we need to patch like you described.


This bit in EvalPlanQualFetch caught my attention ... why is it saying
xmin never changes?  It does change with freezing.

                        /*
                         * If xmin isn't what we're expecting, the slot must 
have been
                         * recycled and reused for an unrelated tuple.  This 
implies that
                         * the latest version of the row was deleted, so we 
need do
                         * nothing.  (Should be safe to examine xmin without 
getting
                         * buffer's content lock, since xmin never changes in 
an existing
                         * tuple.)
                         */
                        if 
(!TransactionIdEquals(HeapTupleHeaderGetXmin(tuple.t_data),
                                                                         
priorXmax))


-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index e00dc6c1ca..e68746fc3b 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -5500,7 +5500,10 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
                        if (TransactionIdPrecedes(xid, cutoff_xid))
                        {
                                if (TransactionIdDidCommit(xid))
+                               {
+                                       xid = FrozenTransactionId;
                                        *flags = FRM_MARK_COMMITTED | 
FRM_RETURN_IS_XID;
+                               }
                                else
                                {
                                        *flags |= FRM_INVALIDATE_XMAX;
diff --git a/src/backend/access/heap/pruneheap.c 
b/src/backend/access/heap/pruneheap.c
index 9a8db74cb9..561acd144a 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -435,7 +435,8 @@ heap_prune_chain(Relation relation, Buffer buffer, 
OffsetNumber rootoffnum,
                 * Check the tuple XMIN against prior XMAX, if any
                 */
                if (TransactionIdIsValid(priorXmax) &&
-                       !TransactionIdEquals(HeapTupleHeaderGetXmin(htup), 
priorXmax))
+                       !TransactionIdEquals(HeapTupleHeaderGetXmin(htup), 
priorXmax) &&
+                       HeapTupleHeaderGetXmin(htup) != FrozenTransactionId)
                        break;
 
                /*
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to