From ebab23b6e341af1d738683af587fb1906ded29e8 Mon Sep 17 00:00:00 2001 From: Mark Dilger 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)