I think this is the patch for 9.3. I ran the test a few hundred times
(with some additional changes such as randomly having an update inside a
savepoint that's randomly aborted, randomly aborting the transaction,
randomly skipping the for key share lock, randomly sleeping at a few
points; and also adding filler columns, reducing fillfactor and using
5, 50, 180, 250, 500 sessions after verifying that it causes the tuples
to stay in the same page or migrate to later pages). The final REINDEX
has never complained again about failing to find the root tuple. I hope
it's good now.
The attached patch needs a few small tweaks, such as improving
commentary in the new function, maybe turn it into a macro (otherwise I
think it could be bad for performance; I'd like a static func but not
sure those are readily available in 9.3), change the XID comparison to
use the appropriate macro rather than ==, and such.
Regarding changes of xmin/xmax comparison, I also checked manually the
spots I thought should be modified and later double-checked against the
list that Michael posted. It's a match, except for rewriteheap.c which
I cannot make heads or tails about. (I think it's rather unfortunate
that it sticks a tuple's Xmax into a field that's called Xmin, but let's
put that aside). Maybe there's a problem here, maybe there isn't.
I'm now going to forward-port this to 9.4.
--
Á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..87bce0e7ea 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -1718,8 +1718,7 @@ heap_hot_search_buffer(ItemPointer tid, Relation
relation, Buffer buffer,
* broken.
*/
if (TransactionIdIsValid(prev_xmax) &&
- !TransactionIdEquals(prev_xmax,
-
HeapTupleHeaderGetXmin(heapTuple->t_data)))
+ !HeapTupleUpdateXmaxMatchesXmin(prev_xmax,
heapTuple->t_data))
break;
/*
@@ -1888,7 +1887,7 @@ heap_get_latest_tid(Relation relation,
* tuple. Check for XMIN match.
*/
if (TransactionIdIsValid(priorXmax) &&
- !TransactionIdEquals(priorXmax,
HeapTupleHeaderGetXmin(tp.t_data)))
+ !HeapTupleUpdateXmaxMatchesXmin(priorXmax, tp.t_data))
{
UnlockReleaseBuffer(buffer);
break;
@@ -1920,6 +1919,36 @@ heap_get_latest_tid(Relation relation,
} /* end of loop
*/
}
+/*
+ * Given a tuple, verify whether the given Xmax matches the tuple's Xmin,
+ * taking into account that the Xmin might have been frozen.
+ */
+bool
+HeapTupleUpdateXmaxMatchesXmin(TransactionId xmax, HeapTupleHeader htup)
+{
+ TransactionId xmin = HeapTupleHeaderGetXmin(htup);
+
+ /* xmax must not be invalid or frozen */
+ Assert(TransactionIdIsNormal(xmax));
+
+ /*
+ * If the xmax of the old tuple is identical to the xmin of the new one,
+ * it's a match.
+ */
+ if (xmax == xmin)
+ return true;
+
+ /* FIXME Here we need to check the HEAP_XMIN_FROZEN in 9.4 and up */
+
+ /*
+ * We actually don't know if there's a match, but if the previous tuple
+ * was frozen, we cannot really rely on a perfect match.
+ */
+ if (xmin == FrozenTransactionId)
+ return true;
+
+ return false;
+}
/*
* UpdateXmaxHintBits - update tuple hint bits after xmax transaction ends
@@ -5045,8 +5074,7 @@ l4:
* the end of the chain, we're done, so return success.
*/
if (TransactionIdIsValid(priorXmax) &&
-
!TransactionIdEquals(HeapTupleHeaderGetXmin(mytup.t_data),
- priorXmax))
+ !HeapTupleUpdateXmaxMatchesXmin(priorXmax,
mytup.t_data))
{
UnlockReleaseBuffer(buf);
return HeapTupleMayBeUpdated;
@@ -5500,7 +5528,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..a342f57f42 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -435,7 +435,7 @@ 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))
+ !HeapTupleUpdateXmaxMatchesXmin(priorXmax, htup))
break;
/*
@@ -774,7 +774,7 @@ heap_get_root_tuples(Page page, OffsetNumber *root_offsets)
htup = (HeapTupleHeader) PageGetItem(page, lp);
if (TransactionIdIsValid(priorXmax) &&
- !TransactionIdEquals(priorXmax,
HeapTupleHeaderGetXmin(htup)))
+ !HeapTupleUpdateXmaxMatchesXmin(priorXmax,
htup))
break;
/* Remember the root line pointer for this item */
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 315a555dcf..79e4701bb8 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -2019,8 +2019,7 @@ EvalPlanQualFetch(EState *estate, Relation relation, int
lockmode,
* buffer's content lock, since xmin never changes in
an existing
* tuple.)
*/
- if
(!TransactionIdEquals(HeapTupleHeaderGetXmin(tuple.t_data),
-
priorXmax))
+ if (!HeapTupleUpdateXmaxMatchesXmin(priorXmax,
tuple.t_data))
{
ReleaseBuffer(buffer);
return NULL;
@@ -2138,8 +2137,7 @@ EvalPlanQualFetch(EState *estate, Relation relation, int
lockmode,
/*
* As above, if xmin isn't what we're expecting, do nothing.
*/
- if (!TransactionIdEquals(HeapTupleHeaderGetXmin(tuple.t_data),
- priorXmax))
+ if (!HeapTupleUpdateXmaxMatchesXmin(priorXmax, tuple.t_data))
{
ReleaseBuffer(buffer);
return NULL;
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 4a187f899c..93a481f483 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -127,6 +127,9 @@ extern void heap_get_latest_tid(Relation relation, Snapshot
snapshot,
ItemPointer tid);
extern void setLastTid(const ItemPointer tid);
+extern bool HeapTupleUpdateXmaxMatchesXmin(TransactionId xmax,
+ HeapTupleHeader
htup);
+
extern BulkInsertState GetBulkInsertState(void);
extern void FreeBulkInsertState(BulkInsertState);
--
2.11.0
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
