v17-0004-Using-non-throwing-clog-interface-from-amcheck.patch

application/octet-stream

Filename: v17-0004-Using-non-throwing-clog-interface-from-amcheck.patch
Type: application/octet-stream
Part: 3
Message: Re: new heapcheck contrib module

Patch

Format: format-patch
Series: patch v17-0004
Subject: Using non-throwing clog interface from amcheck
File+
contrib/amcheck/verify_heapam.c 54 30
src/tools/pgindent/typedefs.list 0 1
From 574d7cb2d8a169fcd50653ec36a7ad919540d808 Mon Sep 17 00:00:00 2001
From: Mark Dilger <mark.dilger@enterprisedb.com>
Date: Tue, 6 Oct 2020 13:33:23 -0700
Subject: [PATCH v17 4/4] Using non-throwing clog interface from amcheck

Converting the heap checking functions to use the recently introduced
non-throwing interface to clog when checking transaction commit status, and
adding corruption reports about missing clog rather than aborting.
---
 contrib/amcheck/verify_heapam.c  | 84 ++++++++++++++++++++------------
 src/tools/pgindent/typedefs.list |  1 -
 2 files changed, 54 insertions(+), 31 deletions(-)

diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c
index b46cce798e..9885a9d55c 100644
--- a/contrib/amcheck/verify_heapam.c
+++ b/contrib/amcheck/verify_heapam.c
@@ -10,6 +10,7 @@
  */
 #include "postgres.h"
 
+#include "access/clogdefs.h"
 #include "access/detoast.h"
 #include "access/genam.h"
 #include "access/heapam.h"
@@ -40,13 +41,6 @@ typedef enum XidBoundsViolation
 	XID_BOUNDS_OK
 } XidBoundsViolation;
 
-typedef enum XidCommitStatus
-{
-	XID_COMMITTED,
-	XID_IN_PROGRESS,
-	XID_ABORTED
-} XidCommitStatus;
-
 typedef enum SkipPages
 {
 	SKIP_PAGES_ALL_FROZEN,
@@ -80,7 +74,7 @@ typedef struct HeapCheckContext
 	 * Cached copies of the most recently checked xid and its status.
 	 */
 	TransactionId cached_xid;
-	XidCommitStatus cached_status;
+	XidStatus	cached_status;
 
 	/* Values concerning the heap relation being checked */
 	Relation	rel;
@@ -138,7 +132,7 @@ static void update_cached_xid_range(HeapCheckContext *ctx);
 static void update_cached_mxid_range(HeapCheckContext *ctx);
 static XidBoundsViolation check_mxid_in_range(MultiXactId mxid, HeapCheckContext *ctx);
 static XidBoundsViolation check_mxid_valid_in_rel(MultiXactId mxid, HeapCheckContext *ctx);
-static XidBoundsViolation get_xid_status(TransactionId xid, HeapCheckContext *ctx, XidCommitStatus *status);
+static XidBoundsViolation get_xid_status(TransactionId xid, HeapCheckContext *ctx, XidStatus *status);
 
 /*
  * Scan and report corruption in heap pages, optionally reconciling toasted
@@ -642,7 +636,7 @@ check_tuple_header_and_visibilty(HeapTupleHeader tuphdr, HeapCheckContext *ctx)
 		else if (infomask & HEAP_MOVED_OFF ||
 				 infomask & HEAP_MOVED_IN)
 		{
-			XidCommitStatus status;
+			XidStatus	status;
 			TransactionId xvac = HeapTupleHeaderGetXvac(tuphdr);
 
 			switch (get_xid_status(xvac, ctx, &status))
@@ -686,17 +680,27 @@ check_tuple_header_and_visibilty(HeapTupleHeader tuphdr, HeapCheckContext *ctx)
 				case XID_BOUNDS_OK:
 					switch (status)
 					{
-						case XID_IN_PROGRESS:
+						case TRANSACTION_STATUS_IN_PROGRESS:
 							return true;	/* HEAPTUPLE_DELETE_IN_PROGRESS */
-						case XID_COMMITTED:
-						case XID_ABORTED:
+						case TRANSACTION_STATUS_COMMITTED:
+						case TRANSACTION_STATUS_ABORTED:
 							return false;	/* HEAPTUPLE_DEAD */
+						case TRANSACTION_STATUS_UNKNOWN:
+							report_corruption(ctx,
+							/*------
+							translator: %u is a transaction identifier */
+											  psprintf(_("old-style VACUUM FULL transaction ID %u commit status is lost"),
+													   xvac));
+							return false;	/* corruption */
+						case TRANSACTION_STATUS_SUB_COMMITTED:
+							elog(ERROR, "get_xid_status failed to resolve parent transaction status");
+							return false;	/* not reached */
 					}
 			}
 		}
 		else
 		{
-			XidCommitStatus status;
+			XidStatus	status;
 
 			switch (get_xid_status(raw_xmin, ctx, &status))
 			{
@@ -737,12 +741,22 @@ check_tuple_header_and_visibilty(HeapTupleHeader tuphdr, HeapCheckContext *ctx)
 				case XID_BOUNDS_OK:
 					switch (status)
 					{
-						case XID_COMMITTED:
+						case TRANSACTION_STATUS_COMMITTED:
 							break;
-						case XID_IN_PROGRESS:
+						case TRANSACTION_STATUS_IN_PROGRESS:
 							return true;	/* insert or delete in progress */
-						case XID_ABORTED:
+						case TRANSACTION_STATUS_ABORTED:
 							return false;	/* HEAPTUPLE_DEAD */
+						case TRANSACTION_STATUS_UNKNOWN:
+							report_corruption(ctx,
+							/*------
+							translator: %u is a transaction identifier */
+											  psprintf(_("raw xmin %u commit status is lost"),
+													   raw_xmin));
+							return false;	/* corruption */
+						case TRANSACTION_STATUS_SUB_COMMITTED:
+							elog(ERROR, "get_xid_status failed to resolve parent transaction status");
+							return false;	/* not reached */
 					}
 			}
 		}
@@ -752,7 +766,7 @@ check_tuple_header_and_visibilty(HeapTupleHeader tuphdr, HeapCheckContext *ctx)
 	{
 		if (infomask & HEAP_XMAX_IS_MULTI)
 		{
-			XidCommitStatus status;
+			XidStatus	status;
 			TransactionId xmax = HeapTupleGetUpdateXid(tuphdr);
 
 			switch (get_xid_status(xmax, ctx, &status))
@@ -795,12 +809,22 @@ check_tuple_header_and_visibilty(HeapTupleHeader tuphdr, HeapCheckContext *ctx)
 				case XID_BOUNDS_OK:
 					switch (status)
 					{
-						case XID_IN_PROGRESS:
+						case TRANSACTION_STATUS_IN_PROGRESS:
 							return true;	/* HEAPTUPLE_DELETE_IN_PROGRESS */
-						case XID_COMMITTED:
-						case XID_ABORTED:
+						case TRANSACTION_STATUS_COMMITTED:
+						case TRANSACTION_STATUS_ABORTED:
 							return false;	/* HEAPTUPLE_RECENTLY_DEAD or
 											 * HEAPTUPLE_DEAD */
+						case TRANSACTION_STATUS_UNKNOWN:
+							report_corruption(ctx,
+							/*------
+							translator: %u is a transaction identifier */
+											  psprintf(_("xmax %u commit status is lost"),
+													   xmax));
+							return false;	/* corruption */
+						case TRANSACTION_STATUS_SUB_COMMITTED:
+							elog(ERROR, "get_xid_status failed to resolve parent transaction status");
+							return false;	/* not reached */
 					}
 			}
 
@@ -1260,7 +1284,11 @@ check_tuple(HeapCheckContext *ctx)
 				break;
 		}
 	}
-	/* If xmax is not a multixact and is normal, it should be within valid range */
+
+	/*
+	 * If xmax is not a multixact and is normal, it should be within valid
+	 * range
+	 */
 	else
 	{
 		switch (get_xid_status(xmax, ctx, NULL))
@@ -1465,7 +1493,7 @@ check_mxid_valid_in_rel(MultiXactId mxid, HeapCheckContext *ctx)
  * status argument will be set with the status of the transaction ID.
  */
 static XidBoundsViolation
-get_xid_status(TransactionId xid, HeapCheckContext *ctx, XidCommitStatus *status)
+get_xid_status(TransactionId xid, HeapCheckContext *ctx, XidStatus *status)
 {
 	XidBoundsViolation result;
 	FullTransactionId fxid;
@@ -1516,19 +1544,15 @@ get_xid_status(TransactionId xid, HeapCheckContext *ctx, XidCommitStatus *status
 		return result;
 	}
 
-	*status = XID_COMMITTED;
+	*status = TRANSACTION_STATUS_COMMITTED;
 	LWLockAcquire(XactTruncationLock, LW_SHARED);
 	clog_horizon = FullTransactionIdFromXidAndCtx(ShmemVariableCache->oldestClogXid, ctx);
 	if (FullTransactionIdPrecedesOrEquals(clog_horizon, fxid))
 	{
 		if (TransactionIdIsCurrentTransactionId(xid))
-			*status = XID_IN_PROGRESS;
-		else if (TransactionIdDidCommit(xid))
-			*status = XID_COMMITTED;
-		else if (TransactionIdDidAbort(xid))
-			*status = XID_ABORTED;
+			*status = TRANSACTION_STATUS_IN_PROGRESS;
 		else
-			*status = XID_IN_PROGRESS;
+			*status = TransactionIdResolveStatus(xid, false);
 	}
 	LWLockRelease(XactTruncationLock);
 	ctx->cached_xid = xid;
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 369b8e7c6f..6ca1bac21a 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2793,7 +2793,6 @@ XactCallbackItem
 XactEvent
 XactLockTableWaitInfo
 XidBoundsViolation
-XidCommitStatus
 XidHorizonPrefetchState
 XidStatus
 XmlExpr
-- 
2.21.1 (Apple Git-122.3)