From a53bc02b3bffd910b28ce03b4a36053b6683c436 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Sun, 9 Apr 2023 18:14:27 -0700 Subject: [PATCH v4 1/3] Fix heapdesc infomask array output. --- src/include/access/heapam_xlog.h | 2 +- src/include/access/rmgrdesc_utils.h | 39 ++++++ src/backend/access/heap/heapam.c | 6 +- src/backend/access/rmgrdesc/heapdesc.c | 120 +++++++++++-------- src/backend/access/rmgrdesc/rmgrdesc_utils.c | 20 ---- 5 files changed, 114 insertions(+), 73 deletions(-) diff --git a/src/include/access/heapam_xlog.h b/src/include/access/heapam_xlog.h index e71d84895..bf1d05965 100644 --- a/src/include/access/heapam_xlog.h +++ b/src/include/access/heapam_xlog.h @@ -279,7 +279,7 @@ typedef struct xl_heap_vacuum /* This is what we need to know about lock */ typedef struct xl_heap_lock { - TransactionId locking_xid; /* might be a MultiXactId not xid */ + TransactionId xmax; /* might be a MultiXactId */ OffsetNumber offnum; /* locked tuple's offset on page */ int8 infobits_set; /* infomask and infomask2 bits to set */ uint8 flags; /* XLH_LOCK_* flag bits */ diff --git a/src/include/access/rmgrdesc_utils.h b/src/include/access/rmgrdesc_utils.h index 00e614649..1686147f8 100644 --- a/src/include/access/rmgrdesc_utils.h +++ b/src/include/access/rmgrdesc_utils.h @@ -12,6 +12,45 @@ #ifndef RMGRDESC_UTILS_H_ #define RMGRDESC_UTILS_H_ +/* + * Guidelines for rmgr descriptor routine authors: + * + * The goal of these guidelines is to avoid gratuitous inconsistencies across + * each rmgr, and to allow users to parse desc output strings without too much + * difficulty. This is not an API specification or an interchange format. + * (Only heapam and nbtree desc routines follow these guidelines at present, + * in any case.) + * + * Record descriptions are similar to JSON style key/value objects. However, + * there is no explicit "string" type/string escaping. Top-level { } brackets + * should be omitted. For example: + * + * snapshotConflictHorizon: 0, flags: 0x03 + * + * Record descriptions may contain variable-length arrays. For example: + * + * nunused: 5, unused: [1, 2, 3, 4, 5] + * + * Nested objects are supported via { } brackets. They generally appear + * inside variable-length arrays. For example: + * + * ndeleted: 0, nupdated: 1, deleted: [], updated: [{ off: 45, nptids: 1, ptids: [0] }] + * + * Try to output things in an order that faithfully represents the order of + * things in the physical WAL record struct. It's a good idea if the number + * of items in the array appears before the array. It's also a good idea if + * key names are reliably unique (within the same nesting level). + * + * It's okay for individual WAL record types to invent their own conventions. + * For example, heapam's PRUNE records output the follow representation of + * redirects: + * + * ... redirected: [39->46], ... + * + * Arguably the PRUNE desc routine should be using object notation instead. + * This ad-hoc representation of redirects has the advantage of being terse in + * a context where that might matter a lot. + */ extern void array_desc(StringInfo buf, void *array, size_t elem_size, int count, void (*elem_desc) (StringInfo buf, void *elem, void *data), void *data); diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index f389ceee1..b300a4675 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -3567,7 +3567,7 @@ l2: XLogRegisterBuffer(0, buffer, REGBUF_STANDARD); xlrec.offnum = ItemPointerGetOffsetNumber(&oldtup.t_self); - xlrec.locking_xid = xmax_lock_old_tuple; + xlrec.xmax = xmax_lock_old_tuple; xlrec.infobits_set = compute_infobits(oldtup.t_data->t_infomask, oldtup.t_data->t_infomask2); xlrec.flags = @@ -4777,7 +4777,7 @@ failed: XLogRegisterBuffer(0, *buffer, REGBUF_STANDARD); xlrec.offnum = ItemPointerGetOffsetNumber(&tuple->t_self); - xlrec.locking_xid = xid; + xlrec.xmax = xid; xlrec.infobits_set = compute_infobits(new_infomask, tuple->t_data->t_infomask2); xlrec.flags = cleared_all_frozen ? XLH_LOCK_ALL_FROZEN_CLEARED : 0; @@ -9848,7 +9848,7 @@ heap_xlog_lock(XLogReaderState *record) BufferGetBlockNumber(buffer), offnum); } - HeapTupleHeaderSetXmax(htup, xlrec->locking_xid); + HeapTupleHeaderSetXmax(htup, xlrec->xmax); HeapTupleHeaderSetCmax(htup, FirstCommandId, false); PageSetLSN(page, lsn); MarkBufferDirty(buffer); diff --git a/src/backend/access/rmgrdesc/heapdesc.c b/src/backend/access/rmgrdesc/heapdesc.c index 3bd083875..d182d8048 100644 --- a/src/backend/access/rmgrdesc/heapdesc.c +++ b/src/backend/access/rmgrdesc/heapdesc.c @@ -17,30 +17,58 @@ #include "access/heapam_xlog.h" #include "access/rmgrdesc_utils.h" +/* + * NOTE: "keyname" argument cannot have trailing spaces or punctuation + * characters + */ static void -out_infobits(StringInfo buf, uint8 infobits) +infobits_desc(StringInfo buf, uint8 infobits, const char *keyname) { - if ((infobits & XLHL_XMAX_IS_MULTI) == 0 && - (infobits & XLHL_XMAX_LOCK_ONLY) == 0 && - (infobits & XLHL_XMAX_EXCL_LOCK) == 0 && - (infobits & XLHL_XMAX_KEYSHR_LOCK) == 0 && - (infobits & XLHL_KEYS_UPDATED) == 0) - return; + appendStringInfo(buf, "%s: [", keyname); - appendStringInfoString(buf, ", infobits: ["); + Assert(buf->data[buf->len - 1] != ' '); if (infobits & XLHL_XMAX_IS_MULTI) - appendStringInfoString(buf, " IS_MULTI"); + appendStringInfoString(buf, "IS_MULTI, "); if (infobits & XLHL_XMAX_LOCK_ONLY) - appendStringInfoString(buf, ", LOCK_ONLY"); + appendStringInfoString(buf, "LOCK_ONLY, "); if (infobits & XLHL_XMAX_EXCL_LOCK) - appendStringInfoString(buf, ", EXCL_LOCK"); + appendStringInfoString(buf, "EXCL_LOCK, "); if (infobits & XLHL_XMAX_KEYSHR_LOCK) - appendStringInfoString(buf, ", KEYSHR_LOCK"); + appendStringInfoString(buf, "KEYSHR_LOCK, "); if (infobits & XLHL_KEYS_UPDATED) - appendStringInfoString(buf, ", KEYS_UPDATED"); + appendStringInfoString(buf, "KEYS_UPDATED, "); - appendStringInfoString(buf, " ]"); + if (buf->data[buf->len - 1] == ' ') + { + /* Truncate-away final unneeded ", " */ + Assert(buf->data[buf->len - 2] == ','); + buf->len -= 2; + buf->data[buf->len] = '\0'; + } + + appendStringInfoString(buf, "]"); +} + +static void +truncate_flags_desc(StringInfo buf, uint8 flags) +{ + appendStringInfoString(buf, "flags: ["); + + if (flags & XLH_TRUNCATE_CASCADE) + appendStringInfoString(buf, "CASCADE, "); + if (flags & XLH_TRUNCATE_RESTART_SEQS) + appendStringInfoString(buf, "RESTART_SEQS, "); + + if (buf->data[buf->len - 1] == ' ') + { + /* Truncate-away final unneeded ", " */ + Assert(buf->data[buf->len - 2] == ','); + buf->len -= 2; + buf->data[buf->len] = '\0'; + } + + appendStringInfoString(buf, "]"); } static void @@ -82,48 +110,36 @@ heap_desc(StringInfo buf, XLogReaderState *record) { xl_heap_delete *xlrec = (xl_heap_delete *) rec; - appendStringInfo(buf, "off: %u, flags: 0x%02X", - xlrec->offnum, - xlrec->flags); - out_infobits(buf, xlrec->infobits_set); + appendStringInfo(buf, "xmax: %u, off: %u, ", + xlrec->xmax, xlrec->offnum); + infobits_desc(buf, xlrec->infobits_set, "infobits"); + appendStringInfo(buf, ", flags: 0x%02X", xlrec->flags); } else if (info == XLOG_HEAP_UPDATE) { xl_heap_update *xlrec = (xl_heap_update *) rec; - appendStringInfo(buf, "off: %u, xmax: %u, flags: 0x%02X", - xlrec->old_offnum, - xlrec->old_xmax, - xlrec->flags); - out_infobits(buf, xlrec->old_infobits_set); - appendStringInfo(buf, ", new off: %u, xmax %u", - xlrec->new_offnum, - xlrec->new_xmax); + appendStringInfo(buf, "old_xmax: %u, old_off: %u, ", + xlrec->old_xmax, xlrec->old_offnum); + infobits_desc(buf, xlrec->old_infobits_set, "old_infobits"); + appendStringInfo(buf, ", flags: 0x%02X, new_xmax: %u, new_off: %u", + xlrec->flags, xlrec->new_xmax, xlrec->new_offnum); } else if (info == XLOG_HEAP_HOT_UPDATE) { xl_heap_update *xlrec = (xl_heap_update *) rec; - appendStringInfo(buf, "off: %u, xmax: %u, flags: 0x%02X", - xlrec->old_offnum, - xlrec->old_xmax, - xlrec->flags); - out_infobits(buf, xlrec->old_infobits_set); - appendStringInfo(buf, ", new off: %u, xmax: %u", - xlrec->new_offnum, - xlrec->new_xmax); + appendStringInfo(buf, "old_xmax: %u, old_off: %u, ", + xlrec->old_xmax, xlrec->old_offnum); + infobits_desc(buf, xlrec->old_infobits_set, "old_infobits"); + appendStringInfo(buf, ", flags: 0x%02X, new_xmax: %u, new_off: %u", + xlrec->flags, xlrec->new_xmax, xlrec->new_offnum); } else if (info == XLOG_HEAP_TRUNCATE) { xl_heap_truncate *xlrec = (xl_heap_truncate *) rec; - appendStringInfoString(buf, "flags: ["); - if (xlrec->flags & XLH_TRUNCATE_CASCADE) - appendStringInfoString(buf, " CASCADE"); - if (xlrec->flags & XLH_TRUNCATE_RESTART_SEQS) - appendStringInfoString(buf, ", RESTART_SEQS"); - appendStringInfoString(buf, " ]"); - + truncate_flags_desc(buf, xlrec->flags); appendStringInfo(buf, ", nrelids: %u", xlrec->nrelids); appendStringInfoString(buf, ", relids:"); array_desc(buf, xlrec->relids, sizeof(Oid), xlrec->nrelids, @@ -139,9 +155,10 @@ heap_desc(StringInfo buf, XLogReaderState *record) { xl_heap_lock *xlrec = (xl_heap_lock *) rec; - appendStringInfo(buf, "off: %u, xid: %u, flags: 0x%02X", - xlrec->offnum, xlrec->locking_xid, xlrec->flags); - out_infobits(buf, xlrec->infobits_set); + appendStringInfo(buf, "xmax: %u, off: %u, ", + xlrec->xmax, xlrec->offnum); + infobits_desc(buf, xlrec->infobits_set, "infobits"); + appendStringInfo(buf, ", flags: 0x%02X", xlrec->flags); } else if (info == XLOG_HEAP_INPLACE) { @@ -230,7 +247,9 @@ heap2_desc(StringInfo buf, XLogReaderState *record) OffsetNumber *offsets; plans = (xl_heap_freeze_plan *) XLogRecGetBlockData(record, 0, NULL); - offsets = (OffsetNumber *) &plans[xlrec->nplans]; + offsets = (OffsetNumber *) ((char *) plans + + (xlrec->nplans * + sizeof(xl_heap_freeze_plan))); appendStringInfoString(buf, ", plans:"); array_desc(buf, plans, sizeof(xl_heap_freeze_plan), xlrec->nplans, &plan_elem_desc, &offsets); @@ -251,18 +270,21 @@ heap2_desc(StringInfo buf, XLogReaderState *record) appendStringInfo(buf, "ntuples: %d, flags: 0x%02X", xlrec->ntuples, xlrec->flags); - appendStringInfoString(buf, ", offsets:"); if (!XLogRecHasBlockImage(record, 0) && !isinit) + { + appendStringInfoString(buf, ", offsets:"); array_desc(buf, xlrec->offsets, sizeof(OffsetNumber), xlrec->ntuples, &offset_elem_desc, NULL); + } } else if (info == XLOG_HEAP2_LOCK_UPDATED) { xl_heap_lock_updated *xlrec = (xl_heap_lock_updated *) rec; - appendStringInfo(buf, "off: %u, xmax: %u, flags: 0x%02X", - xlrec->offnum, xlrec->xmax, xlrec->flags); - out_infobits(buf, xlrec->infobits_set); + appendStringInfo(buf, "xmax: %u, off: %u, ", + xlrec->xmax, xlrec->offnum); + infobits_desc(buf, xlrec->infobits_set, "infobits"); + appendStringInfo(buf, ", flags: 0x%02X", xlrec->flags); } else if (info == XLOG_HEAP2_NEW_CID) { diff --git a/src/backend/access/rmgrdesc/rmgrdesc_utils.c b/src/backend/access/rmgrdesc/rmgrdesc_utils.c index 3d16b1fcb..90051f0df 100644 --- a/src/backend/access/rmgrdesc/rmgrdesc_utils.c +++ b/src/backend/access/rmgrdesc/rmgrdesc_utils.c @@ -16,26 +16,6 @@ #include "access/rmgrdesc_utils.h" #include "storage/off.h" -/* - * Guidelines for formatting desc functions: - * - * member1_name: member1_value, member2_name: member2_value - * - * If the value is a list, please use: - * - * member3_name: [ member3_list_value1, member3_list_value2 ] - * - * The first item appended to the string should not be prepended by any spaces - * or comma, however all subsequent appends to the string are responsible for - * prepending themselves with a comma followed by a space. - * - * Flags should be in ALL CAPS. - * - * For lists/arrays of items, the number of those items should be listed at - * the beginning with all of the other numbers. - * - * Composite objects in a list should be surrounded with { }. - */ void array_desc(StringInfo buf, void *array, size_t elem_size, int count, void (*elem_desc) (StringInfo buf, void *elem, void *data), -- 2.40.0