On 4/10/18 06:29, Pavan Deolasee wrote:
> One of our 2ndQuadrant support customers recently reported a sudden rush
> of TOAST errors post a crash recovery, nearly causing an outage. Most
> errors read like this:
>
> ERROR: unexpected chunk number 0 (expected 1) for toast value nnnn
While researching this, I found that the terminology in this code is
quite inconsistent. It talks about chunks ids, chunk indexes, chunk
numbers, etc. seemingly interchangeably. The above error is actually
about the chunk_seq, not about the chunk_id, as one might think.
The attached patch is my attempt to clean this up a bit. Thoughts?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From db7f5eacc23114f24230dd087837d94bcefc4c88 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Wed, 11 Apr 2018 20:11:35 -0400
Subject: [PATCH] Clarify TOAST terminology in variables and error messages
---
src/backend/access/heap/tuptoaster.c | 67 ++++++++++++++--------------
1 file changed, 34 insertions(+), 33 deletions(-)
diff --git a/src/backend/access/heap/tuptoaster.c
b/src/backend/access/heap/tuptoaster.c
index cd42c50b09..4113cb29d2 100644
--- a/src/backend/access/heap/tuptoaster.c
+++ b/src/backend/access/heap/tuptoaster.c
@@ -1882,10 +1882,10 @@ toast_fetch_datum(struct varlena *attr)
struct varlena *result;
struct varatt_external toast_pointer;
int32 ressize;
- int32 residx,
- nextidx;
+ int32 res_seq,
+ next_seq;
int32 numchunks;
- Pointer chunk;
+ Pointer chunk_data;
bool isnull;
char *chunkdata;
int32 chunksize;
@@ -1930,13 +1930,13 @@ toast_fetch_datum(struct varlena *attr)
ObjectIdGetDatum(toast_pointer.va_valueid));
/*
- * Read the chunks by index
+ * Read the chunks by sequence number
*
- * Note that because the index is actually on (valueid, chunkidx) we
will
- * see the chunks in chunkidx order, even though we didn't explicitly
ask
+ * Note that because the index is actually on (chunk_id, chunk_seq) we
will
+ * see the chunks in chunk_seq order, even though we didn't explicitly
ask
* for it.
*/
- nextidx = 0;
+ next_seq = 0;
init_toast_snapshot(&SnapshotToast);
toastscan = systable_beginscan_ordered(toastrel, toastidxs[validIndex],
@@ -1946,20 +1946,20 @@ toast_fetch_datum(struct varlena *attr)
/*
* Have a chunk, extract the sequence number and the data
*/
- residx = DatumGetInt32(fastgetattr(ttup, 2, toasttupDesc,
&isnull));
+ res_seq = DatumGetInt32(fastgetattr(ttup, 2, toasttupDesc,
&isnull));
Assert(!isnull);
- chunk = DatumGetPointer(fastgetattr(ttup, 3, toasttupDesc,
&isnull));
+ chunk_data = DatumGetPointer(fastgetattr(ttup, 3, toasttupDesc,
&isnull));
Assert(!isnull);
- if (!VARATT_IS_EXTENDED(chunk))
+ if (!VARATT_IS_EXTENDED(chunk_data))
{
- chunksize = VARSIZE(chunk) - VARHDRSZ;
- chunkdata = VARDATA(chunk);
+ chunksize = VARSIZE(chunk_data) - VARHDRSZ;
+ chunkdata = VARDATA(chunk_data);
}
- else if (VARATT_IS_SHORT(chunk))
+ else if (VARATT_IS_SHORT(chunk_data))
{
/* could happen due to heap_form_tuple doing its thing
*/
- chunksize = VARSIZE_SHORT(chunk) - VARHDRSZ_SHORT;
- chunkdata = VARDATA_SHORT(chunk);
+ chunksize = VARSIZE_SHORT(chunk_data) - VARHDRSZ_SHORT;
+ chunkdata = VARDATA_SHORT(chunk_data);
}
else
{
@@ -1974,33 +1974,34 @@ toast_fetch_datum(struct varlena *attr)
/*
* Some checks on the data we've found
*/
- if (residx != nextidx)
- elog(ERROR, "unexpected chunk number %d (expected %d)
for toast value %u in %s",
- residx, nextidx,
+ if (res_seq != next_seq)
+ elog(ERROR, "unexpected chunk_seq %d (expected %d) for
toast value %u in %s",
+ res_seq, next_seq,
toast_pointer.va_valueid,
RelationGetRelationName(toastrel));
- if (residx < numchunks - 1)
+
+ if (res_seq < numchunks - 1)
{
if (chunksize != TOAST_MAX_CHUNK_SIZE)
- elog(ERROR, "unexpected chunk size %d (expected
%d) in chunk %d of %d for toast value %u in %s",
+ elog(ERROR, "unexpected chunk size %d (expected
%d) in chunk_seq %d of %d for toast value %u in %s",
chunksize, (int) TOAST_MAX_CHUNK_SIZE,
- residx, numchunks,
+ res_seq, numchunks,
toast_pointer.va_valueid,
RelationGetRelationName(toastrel));
}
- else if (residx == numchunks - 1)
+ else if (res_seq == numchunks - 1)
{
- if ((residx * TOAST_MAX_CHUNK_SIZE + chunksize) !=
ressize)
- elog(ERROR, "unexpected chunk size %d (expected
%d) in final chunk %d for toast value %u in %s",
+ if ((res_seq * TOAST_MAX_CHUNK_SIZE + chunksize) !=
ressize)
+ elog(ERROR, "unexpected chunk size %d (expected
%d) in final chunk_seq %d for toast value %u in %s",
chunksize,
- (int) (ressize - residx *
TOAST_MAX_CHUNK_SIZE),
- residx,
+ (int) (ressize - res_seq *
TOAST_MAX_CHUNK_SIZE),
+ res_seq,
toast_pointer.va_valueid,
RelationGetRelationName(toastrel));
}
else
- elog(ERROR, "unexpected chunk number %d (out of range
%d..%d) for toast value %u in %s",
- residx,
+ elog(ERROR, "unexpected chunk_seq number %d (out of
range %d..%d) for toast value %u in %s",
+ res_seq,
0, numchunks - 1,
toast_pointer.va_valueid,
RelationGetRelationName(toastrel));
@@ -2008,19 +2009,19 @@ toast_fetch_datum(struct varlena *attr)
/*
* Copy the data into proper place in our result
*/
- memcpy(VARDATA(result) + residx * TOAST_MAX_CHUNK_SIZE,
+ memcpy(VARDATA(result) + res_seq * TOAST_MAX_CHUNK_SIZE,
chunkdata,
chunksize);
- nextidx++;
+ next_seq++;
}
/*
* Final checks that we successfully fetched the datum
*/
- if (nextidx != numchunks)
- elog(ERROR, "missing chunk number %d for toast value %u in %s",
- nextidx,
+ if (next_seq != numchunks)
+ elog(ERROR, "missing chunk_seq number %d for toast value %u in
%s",
+ next_seq,
toast_pointer.va_valueid,
RelationGetRelationName(toastrel));
--
2.17.0
