($INBOX_DIR/description missing)
help / color / mirror / Atom feed[PATCH v4] fix HOT tuples while scanning for index builds
2+ messages / 2 participants
[nested] [flat]
* [PATCH v4] fix HOT tuples while scanning for index builds
@ 2020-08-12 18:02 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Alvaro Herrera @ 2020-08-12 18:02 UTC (permalink / raw)
---
src/backend/access/heap/heapam_handler.c | 20 ++++++++++++++++++++
src/backend/access/heap/pruneheap.c | 5 +++--
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 267a6ee25a..ba44e30035 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -1324,6 +1324,12 @@ heapam_index_build_range_scan(Relation heapRelation,
* buffer continuously while visiting the page, so no pruning
* operation can occur either.
*
+ * In cases with only ShareUpdateExclusiveLock on the table, it's
+ * possible for some HOT tuples to appear that we didn't know about
+ * when we first read the page. To handle that case, we re-obtain the
+ * list of root offsets when a HOT tuple points to a root item that we
+ * don't know about.
+ *
* Also, although our opinions about tuple liveness could change while
* we scan the page (due to concurrent transaction commits/aborts),
* the chain root locations won't, so this info doesn't need to be
@@ -1625,6 +1631,20 @@ heapam_index_build_range_scan(Relation heapRelation,
offnum = ItemPointerGetOffsetNumber(&heapTuple->t_self);
+ /*
+ * If a HOT tuple points to a root that we don't know
+ * about, obtain root items afresh. If that still fails,
+ * report it as corruption.
+ */
+ if (root_offsets[offnum - 1] == InvalidOffsetNumber)
+ {
+ Page page = BufferGetPage(hscan->rs_cbuf);
+
+ LockBuffer(hscan->rs_cbuf, BUFFER_LOCK_SHARE);
+ heap_get_root_tuples(page, root_offsets);
+ LockBuffer(hscan->rs_cbuf, BUFFER_LOCK_UNLOCK);
+ }
+
if (!OffsetNumberIsValid(root_offsets[offnum - 1]))
ereport(ERROR,
(errcode(ERRCODE_DATA_CORRUPTED),
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 256df4de10..7e3d44dfd6 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -732,7 +732,7 @@ heap_page_prune_execute(Buffer buffer,
* root_offsets[k - 1] = j.
*
* The passed-in root_offsets array must have MaxHeapTuplesPerPage entries.
- * We zero out all unused entries.
+ * Unused entries are filled with InvalidOffsetNumber (zero).
*
* The function must be called with at least share lock on the buffer, to
* prevent concurrent prune operations.
@@ -747,7 +747,8 @@ heap_get_root_tuples(Page page, OffsetNumber *root_offsets)
OffsetNumber offnum,
maxoff;
- MemSet(root_offsets, 0, MaxHeapTuplesPerPage * sizeof(OffsetNumber));
+ MemSet(root_offsets, InvalidOffsetNumber,
+ MaxHeapTuplesPerPage * sizeof(OffsetNumber));
maxoff = PageGetMaxOffsetNumber(page);
for (offnum = FirstOffsetNumber; offnum <= maxoff; offnum = OffsetNumberNext(offnum))
--
2.20.1
--opJtzjQTFsWo+cga--
^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: Removing log_cnt from pg_sequence_read_tuple()
@ 2024-08-26 14:19 Nathan Bossart <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Nathan Bossart @ 2024-08-26 14:19 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Postgres hackers <[email protected]>
On Mon, Aug 26, 2024 at 11:11:55AM +0900, Michael Paquier wrote:
> It seems to me that we'd live better without it, at least it matters
> for the sequence AM patch, because some of its callbacks are also
> shaped around the fact that WAL may not be required for sequence value
> computations. Providing a function that should be rather generic does
> not fit well in this context, still I agree that it comes down to how
> the callbacks are defined, of course. My point is that the use of WAL
> is something that should be optional, but log_cnt in this function
> makes it a mandatory concept that all sequence AMs would need to deal
> with.
I am fine with changes to this function that would allow it to be
generically useful for all sequence AMs, provided that it can still be used
for dumpSequenceData(). I only included log_cnt because
pg_sequence_read_tuple() is intended to be a substitute for SELECT from the
sequence, but I'm not aware of any real use-case for that column besides
debugging, which you've already addressed. Even if we remove log_cnt, you
can still find it with SELECT, too.
The patch looks reasonable to me. Do you think the name of the function
still makes sense now that 1) we might have different sequence AMs in the
near future and 2) it no longer returns everything in the sequence tuple?
--
nathan
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2024-08-26 14:19 UTC | newest]
Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-08-12 18:02 [PATCH v4] fix HOT tuples while scanning for index builds Alvaro Herrera <[email protected]>
2024-08-26 14:19 Re: Removing log_cnt from pg_sequence_read_tuple() Nathan Bossart <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox