From 59f3f80f82ed7a63d86c991d0cb025e4cde2caec Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Wed, 20 Mar 2024 13:36:41 +0200 Subject: [PATCH v5 06/26] Fix logging snapshot conflict horizon. - it was accessed without proper alignment, which won't work on architectures that are strict about alignment. Use memcpy. - in heap_xlog_prune_freeze, the code tried to access the xid with "(xlhp_conflict_horizon *) (xlrec + SizeOfHeapPrune);" But 'xlrec' was "xl_heap_prune *" rather than "char *". That happened to work, because sizeof(xl_heap_prune) == 1, but make it more robust by adding a cast to char *. - remove xlhp_conflict_horizon and store a TransactionId directly. A separate struct would make sense if we needed to store anything else there, but for now it just seems like more code. --- src/backend/access/heap/heapam.c | 6 ++++-- src/backend/access/heap/pruneheap.c | 3 +-- src/backend/access/rmgrdesc/heapdesc.c | 6 ++++-- src/include/access/heapam_xlog.h | 12 +++++------- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 69a9aaa501d..b8d21ddd4dd 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -8628,9 +8628,11 @@ heap_xlog_prune_freeze(XLogReaderState *record) */ if (xlrec->flags & XLHP_HAS_CONFLICT_HORIZON && InHotStandby) { - xlhp_conflict_horizon *horizon = (xlhp_conflict_horizon *) (xlrec + SizeOfHeapPrune); + TransactionId conflict_xid; - ResolveRecoveryConflictWithSnapshot(horizon->xid, + memcpy(&conflict_xid, ((char *) xlrec) + SizeOfHeapPrune, sizeof(TransactionId)); + + ResolveRecoveryConflictWithSnapshot(conflict_xid, xlrec->flags & XLHP_IS_CATALOG_REL, rlocator); } diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c index 704604d206a..6482d9d05c1 100644 --- a/src/backend/access/heap/pruneheap.c +++ b/src/backend/access/heap/pruneheap.c @@ -1284,7 +1284,6 @@ log_heap_prune_and_freeze(Relation relation, Buffer buffer, OffsetNumber *unused, int nunused) { xl_heap_prune xlrec; - xlhp_conflict_horizon horizon; XLogRecPtr recptr; xlhp_freeze freeze; xlhp_prune_items redirect_items, @@ -1320,7 +1319,7 @@ log_heap_prune_and_freeze(Relation relation, Buffer buffer, XLogRegisterData((char *) &xlrec, SizeOfHeapPrune); if (TransactionIdIsValid(conflict_xid)) - XLogRegisterData((char *) &horizon, SizeOfSnapshotConflictHorizon); + XLogRegisterData((char *) &conflict_xid, sizeof(TransactionId)); XLogRegisterBuffer(0, buffer, REGBUF_STANDARD); diff --git a/src/backend/access/rmgrdesc/heapdesc.c b/src/backend/access/rmgrdesc/heapdesc.c index 9ef8a745982..ff238d58279 100644 --- a/src/backend/access/rmgrdesc/heapdesc.c +++ b/src/backend/access/rmgrdesc/heapdesc.c @@ -248,10 +248,12 @@ heap2_desc(StringInfo buf, XLogReaderState *record) if (xlrec->flags & XLHP_HAS_CONFLICT_HORIZON) { - xlhp_conflict_horizon *horizon = (xlhp_conflict_horizon *) (xlrec + SizeOfHeapPrune); + TransactionId conflict_xid; + + memcpy(&conflict_xid, rec + SizeOfHeapPrune, sizeof(TransactionId)); appendStringInfo(buf, "snapshotConflictHorizon: %u", - horizon->xid); + conflict_xid); } appendStringInfo(buf, ", isCatalogRel: %c", diff --git a/src/include/access/heapam_xlog.h b/src/include/access/heapam_xlog.h index 3d41aeb6d47..f0cbd31189e 100644 --- a/src/include/access/heapam_xlog.h +++ b/src/include/access/heapam_xlog.h @@ -247,6 +247,11 @@ typedef struct xl_heap_update typedef struct xl_heap_prune { uint8 flags; + + /* + * If XLHP_HAS_CONFLICT_HORIZON is set, the conflict XID follows, + * unaligned + */ } xl_heap_prune; /* to handle recovery conflict during logical decoding on standby */ @@ -273,13 +278,6 @@ typedef struct xl_heap_prune #define SizeOfHeapPrune (offsetof(xl_heap_prune, flags) + sizeof(uint8)) -typedef struct xlhp_conflict_horizon -{ - TransactionId xid; -} xlhp_conflict_horizon; - -#define SizeOfSnapshotConflictHorizon (offsetof(xlhp_conflict_horizon, xid) + sizeof(uint32)) - /* * This struct represents a 'freeze plan', which describes how to freeze a * group of one or more heap tuples (appears in xl_heap_prune's xlhp_freeze -- 2.39.2