v23-0001-amcheck-adjusting-corruption-report-output.patch

From ebab23b6e341af1d738683af587fb1906ded29e8 Mon Sep 17 00:00:00 2001
From: Mark Dilger <mark.dilger@enterprisedb.com>
Date: Mon, 26 Apr 2021 09:00:10 -0700
Subject: [PATCH v23] amcheck: adjusting corruption report output

Changing how toast corruption reports refer to toast chunks and how
chunk_seq numbering is anticipated.  No longer referring to the Nth
chunk returned from a toast index scan as "chunk N" since this
language could be misinterpreted by a reasonable user to mean the
chunk with chunk_seq=N.  No longer expecting each chunk to have
chunk_seq=N, but rather expecting each chunk to have chunk_seq equal
to one greater than the chunk_seq from the prior chunk from the
toast index scan.

Per complaint from Robert Haas that the old wording could confuse
users, and that missing chunks would precipitate a cascade of
corruption reports for all subsequent chunks rather than just for
the first subsequent chunk.
---
 contrib/amcheck/verify_heapam.c | 57 ++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 29 deletions(-)

diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c
index 9f159eb3db..fe4a32dd0f 100644
--- a/contrib/amcheck/verify_heapam.c
+++ b/contrib/amcheck/verify_heapam.c
@@ -150,8 +150,8 @@ typedef struct HeapCheckContext
 static void sanity_check_relation(Relation rel);
 static void check_tuple(HeapCheckContext *ctx);
 static void check_toast_tuple(HeapTuple toasttup, HeapCheckContext *ctx,
-							  ToastedAttribute *ta, int32 chunkno,
-							  int32 endchunk);
+							  ToastedAttribute *ta, int32 *expected_chunk_seq,
+							  int32 last_chunk_seq);
 
 static bool check_tuple_attribute(HeapCheckContext *ctx);
 static void check_toasted_attribute(HeapCheckContext *ctx,
@@ -1159,23 +1159,27 @@ check_tuple_visibility(HeapCheckContext *ctx)
  * each toast tuple being checked against where we are in the sequence, as well
  * as each toast tuple having its varlena structure sanity checked.
  *
- * Returns whether the toast tuple passed the corruption checks.
+ * expected_chunk_seq: on entry, the chunk_seq expected for this chunk; on
+ * return, the chunk_seq expected for the next chunk
+ * last_chunk_seq: the final chunk_seq expected for this toasted attribute
  */
 static void
 check_toast_tuple(HeapTuple toasttup, HeapCheckContext *ctx,
-				  ToastedAttribute *ta, int32 chunkno, int32 endchunk)
+				  ToastedAttribute *ta, int32 *expected_chunk_seq,
+				  int32 last_chunk_seq)
 {
-	int32		curchunk;
+	int32		chunk_seq;
 	Pointer		chunk;
 	bool		isnull;
 	int32		chunksize;
 	int32		expected_size;
+	int32		expected_seq = *expected_chunk_seq;
 
 	/*
 	 * Have a chunk, extract the sequence number and the data
 	 */
-	curchunk = DatumGetInt32(fastgetattr(toasttup, 2,
-										 ctx->toast_rel->rd_att, &isnull));
+	chunk_seq = DatumGetInt32(fastgetattr(toasttup, 2,
+										  ctx->toast_rel->rd_att, &isnull));
 	if (isnull)
 	{
 		report_toast_corruption(ctx, ta,
@@ -1183,13 +1187,15 @@ check_toast_tuple(HeapTuple toasttup, HeapCheckContext *ctx,
 										 ta->toast_pointer.va_valueid));
 		return;
 	}
+	*expected_chunk_seq = chunk_seq + 1;
+
 	chunk = DatumGetPointer(fastgetattr(toasttup, 3,
 										ctx->toast_rel->rd_att, &isnull));
 	if (isnull)
 	{
 		report_toast_corruption(ctx, ta,
 								psprintf("toast value %u chunk %d has null data",
-										 ta->toast_pointer.va_valueid, chunkno));
+										 ta->toast_pointer.va_valueid, chunk_seq));
 		return;
 	}
 	if (!VARATT_IS_EXTENDED(chunk))
@@ -1209,38 +1215,32 @@ check_toast_tuple(HeapTuple toasttup, HeapCheckContext *ctx,
 		report_toast_corruption(ctx, ta,
 								psprintf("toast value %u chunk %d has invalid varlena header %0x",
 										 ta->toast_pointer.va_valueid,
-										 chunkno, header));
+										 chunk_seq, header));
 		return;
 	}
 
 	/*
 	 * Some checks on the data we've found
 	 */
-	if (curchunk != chunkno)
-	{
+	if (chunk_seq != expected_seq)
 		report_toast_corruption(ctx, ta,
-								psprintf("toast value %u chunk %d has sequence number %d, but expected sequence number %d",
+								psprintf("toast value %u index scan returned chunk %d when expecting chunk %d",
 										 ta->toast_pointer.va_valueid,
-										 chunkno, curchunk, chunkno));
-		return;
-	}
-	if (chunkno > endchunk)
-	{
+										 chunk_seq, expected_seq));
+	if (chunk_seq > last_chunk_seq)
 		report_toast_corruption(ctx, ta,
 								psprintf("toast value %u chunk %d follows last expected chunk %d",
 										 ta->toast_pointer.va_valueid,
-										 chunkno, endchunk));
-		return;
-	}
+										 chunk_seq, last_chunk_seq));
 
-	expected_size = curchunk < endchunk ? TOAST_MAX_CHUNK_SIZE
-		: VARATT_EXTERNAL_GET_EXTSIZE(ta->toast_pointer) - (endchunk * TOAST_MAX_CHUNK_SIZE);
+	expected_size = chunk_seq < last_chunk_seq ? TOAST_MAX_CHUNK_SIZE
+		: VARATT_EXTERNAL_GET_EXTSIZE(ta->toast_pointer) - (last_chunk_seq * TOAST_MAX_CHUNK_SIZE);
 
 	if (chunksize != expected_size)
 		report_toast_corruption(ctx, ta,
 								psprintf("toast value %u chunk %d has size %u, but expected size %u",
 										 ta->toast_pointer.va_valueid,
-										 chunkno, chunksize, expected_size));
+										 chunk_seq, chunksize, expected_size));
 }
 
 /*
@@ -1437,9 +1437,9 @@ check_toasted_attribute(HeapCheckContext *ctx, ToastedAttribute *ta)
 	bool		found_toasttup;
 	HeapTuple	toasttup;
 	int32		chunkno;
-	int32		endchunk;
+	int32		last_chunk_seq;
 
-	endchunk = (VARATT_EXTERNAL_GET_EXTSIZE(ta->toast_pointer) - 1) / TOAST_MAX_CHUNK_SIZE;
+	last_chunk_seq = (VARATT_EXTERNAL_GET_EXTSIZE(ta->toast_pointer) - 1) / TOAST_MAX_CHUNK_SIZE;
 
 	/*
 	 * Setup a scan key to find chunks in toast table with matching va_valueid
@@ -1465,8 +1465,7 @@ check_toasted_attribute(HeapCheckContext *ctx, ToastedAttribute *ta)
 									 ForwardScanDirection)) != NULL)
 	{
 		found_toasttup = true;
-		check_toast_tuple(toasttup, ctx, ta, chunkno, endchunk);
-		chunkno++;
+		check_toast_tuple(toasttup, ctx, ta, &chunkno, last_chunk_seq);
 	}
 	systable_endscan_ordered(toastscan);
 
@@ -1474,11 +1473,11 @@ check_toasted_attribute(HeapCheckContext *ctx, ToastedAttribute *ta)
 		report_toast_corruption(ctx, ta,
 								psprintf("toast value %u not found in toast table",
 										 ta->toast_pointer.va_valueid));
-	else if (chunkno != (endchunk + 1))
+	else if (chunkno != (last_chunk_seq + 1))
 		report_toast_corruption(ctx, ta,
 								psprintf("toast value %u was expected to end at chunk %d, but ended at chunk %d",
 										 ta->toast_pointer.va_valueid,
-										 (endchunk + 1), chunkno));
+										 (last_chunk_seq + 1), chunkno));
 }
 
 /*
-- 
2.21.1 (Apple Git-122.3)