From d3ccc28b07854f6333621f1413d05b8f7c1a8aed Mon Sep 17 00:00:00 2001 From: Lukas Fittl Date: Sat, 31 Oct 2026 11:53:59 -0700 Subject: [PATCH v2 2/5] Fix broken HOT chain handling in parallel BRIN and GIN index builds When a non-concurrent index build encounters a RECENTLY_DEAD tuple that was HOT-updated, heapam_index_build_range_scan() skips it, indexing only the live tuple at the end of the HOT chain, and sets ii_BrokenHotChain. index_build() then marks the index with indcheckxmin, so that transactions with a snapshot older than index creation don't use the index, as it may be missing tuple versions still visible to them. In parallel index builds this requires each worker to report back to the leader whether it encountered a broken HOT chain. btree correctly does this, but BRIN and GIN parallel index builds do not. As a result, indcheckxmin was never set, and a transaction holding an older snapshot could use the new index and get wrong results. Fix by tracking the brokenhotchain flag in the shared state of both AMs, like btree does, and copying it back into the ambuild IndexInfo in the leader after all participants are done. Oversight when parallel index builds were added for BRIN (b4375717147) and GIN (8492feb98f6) and backpatch to 17 and 18 respectively. Author: Lukas Fittl Reviewed-by: Discussion: https://postgr.es/m/CAP53Pkw3WcVbg61G29AmN=6XkuxGryoWKnhTyC1JmkT08i8qVA@mail.gmail.com --- src/backend/access/brin/brin.c | 33 ++++++++++++++++++++++-------- src/backend/access/gin/gininsert.c | 32 +++++++++++++++++++++-------- 2 files changed, 47 insertions(+), 18 deletions(-) diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c index 5059da8dce7..fedc0387ca0 100644 --- a/src/backend/access/brin/brin.c +++ b/src/backend/access/brin/brin.c @@ -99,10 +99,14 @@ typedef struct BrinShared * reltuples is the total number of input heap tuples. * * indtuples is the total number of tuples that made it into the index. + * + * brokenhotchain indicates if any worker detected a broken HOT chain + * during build. */ int nparticipantsdone; double reltuples; double indtuples; + bool brokenhotchain; /* * ParallelTableScanDescData data follows. Can't directly embed here, as @@ -236,8 +240,10 @@ static void _brin_begin_parallel(BrinBuildState *buildstate, Relation heap, Rela bool isconcurrent, int request); static void _brin_end_parallel(BrinLeader *brinleader, BrinBuildState *state); static Size _brin_parallel_estimate_shared(Relation heap, Snapshot snapshot); -static double _brin_parallel_heapscan(BrinBuildState *state); -static double _brin_parallel_merge(BrinBuildState *state); +static double _brin_parallel_heapscan(BrinBuildState *state, + bool *brokenhotchain); +static double _brin_parallel_merge(BrinBuildState *state, + IndexInfo *indexInfo); static void _brin_leader_participate_as_worker(BrinBuildState *buildstate, Relation heap, Relation index); static void _brin_parallel_scan_and_build(BrinBuildState *state, @@ -1223,7 +1229,7 @@ brinbuild(Relation heap, Relation index, IndexInfo *indexInfo) TUPLESORT_NONE); /* scan the relation and merge per-worker results */ - reltuples = _brin_parallel_merge(state); + reltuples = _brin_parallel_merge(state, indexInfo); _brin_end_parallel(state->bs_leader, state); } @@ -2483,6 +2489,7 @@ _brin_begin_parallel(BrinBuildState *buildstate, Relation heap, Relation index, brinshared->nparticipantsdone = 0; brinshared->reltuples = 0.0; brinshared->indtuples = 0.0; + brinshared->brokenhotchain = false; table_parallelscan_initialize(heap, ParallelTableScanFromBrinShared(brinshared), @@ -2589,10 +2596,11 @@ _brin_end_parallel(BrinLeader *brinleader, BrinBuildState *state) * already be underway within worker processes (when leader participates * as a worker, we should end up here just as workers are finishing). * - * Returns the total number of heap tuples scanned. + * Returns the total number of heap tuples scanned. Also sets *brokenhotchain + * to whether any participant detected a broken HOT chain. */ static double -_brin_parallel_heapscan(BrinBuildState *state) +_brin_parallel_heapscan(BrinBuildState *state, bool *brokenhotchain) { BrinShared *brinshared = state->bs_leader->brinshared; int nparticipanttuplesorts; @@ -2606,6 +2614,7 @@ _brin_parallel_heapscan(BrinBuildState *state) /* copy the data into leader state */ state->bs_reltuples = brinshared->reltuples; state->bs_numtuples = brinshared->indtuples; + *brokenhotchain = brinshared->brokenhotchain; SpinLockRelease(&brinshared->mutex); break; @@ -2630,10 +2639,12 @@ _brin_parallel_heapscan(BrinBuildState *state) * summaries for the same page range, and also fill-in empty summaries for * ranges without any tuples. * - * Returns the total number of heap tuples scanned. + * Returns the total number of heap tuples scanned. Also sets + * indexInfo->ii_BrokenHotChain if any participant detected a broken HOT chain, + * so that index.c can mark the index as unsafe for older snapshots. */ static double -_brin_parallel_merge(BrinBuildState *state) +_brin_parallel_merge(BrinBuildState *state, IndexInfo *indexInfo) { BrinTuple *btup; BrinMemTuple *memtuple = NULL; @@ -2644,7 +2655,8 @@ _brin_parallel_merge(BrinBuildState *state) double reltuples; /* wait for workers to scan table and produce partial results */ - reltuples = _brin_parallel_heapscan(state); + reltuples = _brin_parallel_heapscan(state, + &indexInfo->ii_BrokenHotChain); /* do the actual sort in the leader */ tuplesort_performsort(state->bs_sortstate); @@ -2859,12 +2871,15 @@ _brin_parallel_scan_and_build(BrinBuildState *state, state->bs_reltuples += reltuples; /* - * Done. Record ambuild statistics. + * Done. Record ambuild statistics, and whether we encountered a broken + * HOT chain. */ SpinLockAcquire(&brinshared->mutex); brinshared->nparticipantsdone++; brinshared->reltuples += state->bs_reltuples; brinshared->indtuples += state->bs_numtuples; + if (indexInfo->ii_BrokenHotChain) + brinshared->brokenhotchain = true; SpinLockRelease(&brinshared->mutex); /* Notify leader */ diff --git a/src/backend/access/gin/gininsert.c b/src/backend/access/gin/gininsert.c index cbfaa5075b0..f42c5bda4e4 100644 --- a/src/backend/access/gin/gininsert.c +++ b/src/backend/access/gin/gininsert.c @@ -92,10 +92,14 @@ typedef struct GinBuildShared * reltuples is the total number of input heap tuples. * * indtuples is the total number of tuples that made it into the index. + * + * brokenhotchain indicates if any worker detected a broken HOT chain + * during build. */ int nparticipantsdone; double reltuples; double indtuples; + bool brokenhotchain; /* * ParallelTableScanDescData data follows. Can't directly embed here, as @@ -191,8 +195,10 @@ static void _gin_begin_parallel(GinBuildState *buildstate, Relation heap, Relati bool isconcurrent, int request); static void _gin_end_parallel(GinLeader *ginleader, GinBuildState *state); static Size _gin_parallel_estimate_shared(Relation heap, Snapshot snapshot); -static double _gin_parallel_heapscan(GinBuildState *state); -static double _gin_parallel_merge(GinBuildState *state); +static double _gin_parallel_heapscan(GinBuildState *state, + bool *brokenhotchain); +static double _gin_parallel_merge(GinBuildState *state, + IndexInfo *indexInfo); static void _gin_leader_participate_as_worker(GinBuildState *buildstate, Relation heap, Relation index); static void _gin_parallel_scan_and_build(GinBuildState *state, @@ -750,7 +756,7 @@ ginbuild(Relation heap, Relation index, IndexInfo *indexInfo) TUPLESORT_NONE); /* scan the relation in parallel and merge per-worker results */ - reltuples = _gin_parallel_merge(state); + reltuples = _gin_parallel_merge(state, indexInfo); _gin_end_parallel(state->bs_leader, state); } @@ -1043,6 +1049,7 @@ _gin_begin_parallel(GinBuildState *buildstate, Relation heap, Relation index, ginshared->nparticipantsdone = 0; ginshared->reltuples = 0.0; ginshared->indtuples = 0.0; + ginshared->brokenhotchain = false; table_parallelscan_initialize(heap, ParallelTableScanFromGinBuildShared(ginshared), @@ -1145,10 +1152,11 @@ _gin_end_parallel(GinLeader *ginleader, GinBuildState *state) * already be underway within worker processes (when leader participates * as a worker, we should end up here just as workers are finishing). * - * Returns the total number of heap tuples scanned. + * Returns the total number of heap tuples scanned. Also sets *brokenhotchain + * to whether any participant detected a broken HOT chain. */ static double -_gin_parallel_heapscan(GinBuildState *state) +_gin_parallel_heapscan(GinBuildState *state, bool *brokenhotchain) { GinBuildShared *ginshared = state->bs_leader->ginshared; int nparticipanttuplesorts; @@ -1162,6 +1170,7 @@ _gin_parallel_heapscan(GinBuildState *state) /* copy the data into leader state */ state->bs_reltuples = ginshared->reltuples; state->bs_numtuples = ginshared->indtuples; + *brokenhotchain = ginshared->brokenhotchain; SpinLockRelease(&ginshared->mutex); break; @@ -1646,10 +1655,12 @@ GinBufferCanAddKey(GinBuffer *buffer, GinTuple *tup) * summaries for the same page range, and also fill-in empty summaries for * ranges without any tuples. * - * Returns the total number of heap tuples scanned. + * Returns the total number of heap tuples scanned. Also sets + * indexInfo->ii_BrokenHotChain if any participant detected a broken HOT chain, + * so that index.c can mark the index as unsafe for older snapshots. */ static double -_gin_parallel_merge(GinBuildState *state) +_gin_parallel_merge(GinBuildState *state, IndexInfo *indexInfo) { GinTuple *tup; Size tuplen; @@ -1660,7 +1671,7 @@ _gin_parallel_merge(GinBuildState *state) double numtuples = 0; /* wait for workers to scan table and produce partial results */ - reltuples = _gin_parallel_heapscan(state); + reltuples = _gin_parallel_heapscan(state, &indexInfo->ii_BrokenHotChain); /* Execute the sort */ pgstat_progress_update_param(PROGRESS_CREATEIDX_SUBPHASE, @@ -2088,12 +2099,15 @@ _gin_parallel_scan_and_build(GinBuildState *state, state->bs_reltuples += reltuples; /* - * Done. Record ambuild statistics. + * Done. Record ambuild statistics, and whether we encountered a broken + * HOT chain. */ SpinLockAcquire(&ginshared->mutex); ginshared->nparticipantsdone++; ginshared->reltuples += state->bs_reltuples; ginshared->indtuples += state->bs_numtuples; + if (indexInfo->ii_BrokenHotChain) + ginshared->brokenhotchain = true; SpinLockRelease(&ginshared->mutex); /* Notify leader */ -- 2.47.1