Wood, Dan wrote:
> Yes, I’ve been testing 9.6. I’ll try Alvaro’s patch today.
>
> I would prefer to focus on either latest 9X or 11dev.
I tested my patch on 9.4 and 9.5 today and it seems to close the problem
(with the patch, I waited 10x as many iterations as it took for the
problem to occur ~10 times without the patch), but I can reproduce a
problem in 9.6 with my patch installed. There must be something new in
9.6 that is causing the problem to reappear.
> Does Alvaro’s patch presume any of the other patch to set COMMITTED in
> the freeze code?
I don't know what you mean. Here is the 9.6 version of my patch. Note
that HEAP_XMIN_FROZEN (which uses the XMIN_COMMITTED bit as I recall)
was introduced in 9.4.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 46ca12d56402d33a78ea0e13367d1e0e25a474dd Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Fri, 6 Oct 2017 14:11:34 +0200
Subject: [PATCH] Fix bugs
---
src/backend/access/heap/heapam.c | 52 +++++++++++++++++++++++++++++++++----
src/backend/access/heap/pruneheap.c | 4 +--
src/backend/executor/execMain.c | 6 ++---
src/include/access/heapam.h | 3 +++
4 files changed, 54 insertions(+), 11 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index b41f2a2fdd..10916b140e 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -2060,8 +2060,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;
/*
@@ -2244,7 +2243,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;
@@ -2276,6 +2275,50 @@ heap_get_latest_tid(Relation relation,
} /* end of loop
*/
}
+/*
+ * HeapTupleUpdateXmaxMatchesXmin - verify update chain xmax/xmin lineage
+ *
+ * Given the new version of a tuple after some update, verify whether the
+ * given Xmax (corresponding to the previous version) matches the tuple's
+ * Xmin, taking into account that the Xmin might have been frozen after the
+ * update.
+ */
+bool
+HeapTupleUpdateXmaxMatchesXmin(TransactionId xmax, HeapTupleHeader htup)
+{
+ TransactionId xmin = HeapTupleHeaderGetXmin(htup);
+
+ /*
+ * If the xmax of the old tuple is identical to the xmin of the new one,
+ * it's a match.
+ */
+ if (TransactionIdEquals(xmax, xmin))
+ return true;
+
+ /*
+ * If the Xmin that was in effect prior to a freeze matches the Xmax,
+ * it's good too.
+ */
+ if (HeapTupleHeaderXminFrozen(htup) &&
+ TransactionIdEquals(HeapTupleHeaderGetRawXmin(htup), xmax))
+ return true;
+
+ /*
+ * When a tuple is frozen, the original Xmin is lost, but we know it's a
+ * committed transaction. So unless the Xmax is InvalidXid, we don't
know
+ * for certain that there is a match, but there may be one; and we must
+ * return true so that a HOT chain that is half-frozen can be walked
+ * correctly.
+ *
+ * We no longer freeze tuples this way, but we must keep this in order
to
+ * interpret pre-pg_upgrade pages correctly.
+ */
+ if (TransactionIdEquals(xmin, FrozenTransactionId) &&
+ TransactionIdIsValid(xmax))
+ return true;
+
+ return false;
+}
/*
* UpdateXmaxHintBits - update tuple hint bits after xmax transaction ends
@@ -5693,8 +5736,7 @@ l4:
* end of the chain, we're done, so return success.
*/
if (TransactionIdIsValid(priorXmax) &&
-
!TransactionIdEquals(HeapTupleHeaderGetXmin(mytup.t_data),
- priorXmax))
+ !HeapTupleUpdateXmaxMatchesXmin(priorXmax,
mytup.t_data))
{
result = HeapTupleMayBeUpdated;
goto out_locked;
diff --git a/src/backend/access/heap/pruneheap.c
b/src/backend/access/heap/pruneheap.c
index 52231ac417..7753ee7b12 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -473,7 +473,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;
/*
@@ -813,7 +813,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 396b7a1e83..ac7e0fb48b 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -2594,8 +2594,7 @@ EvalPlanQualFetch(EState *estate, Relation relation, int
lockmode,
* atomic, and Xmin never changes in an existing tuple,
except to
* invalid or frozen, and neither of those can match
priorXmax.)
*/
- if
(!TransactionIdEquals(HeapTupleHeaderGetXmin(tuple.t_data),
-
priorXmax))
+ if (!HeapTupleUpdateXmaxMatchesXmin(priorXmax,
tuple.t_data))
{
ReleaseBuffer(buffer);
return NULL;
@@ -2742,8 +2741,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 4e41024e92..9f4367d704 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -146,6 +146,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);
extern void ReleaseBulkInsertStatePin(BulkInsertState bistate);
--
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
