From 196248fa7e8ac17772974cc1fba436b4e4c379fd Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Wed, 19 Aug 2026 18:09:23 -0400 Subject: [PATCH v1] Fix GIN VACUUM unfinished split page deletion bug. Author: Peter Geoghegan Discussion: https://postgr.es/m/CAH2-Wz=sKJcn+OtfVN9rdg+Ps9e4cuQWNP-9t12UE2d8nEG90Q@mail.gmail.com Backpatch-through: 14 --- src/backend/access/gin/ginvacuum.c | 18 ++- src/test/modules/gin/Makefile | 2 +- .../gin/expected/gin_incomplete_splits.out | 112 +++++++++++++++++- .../modules/gin/sql/gin_incomplete_splits.sql | 81 ++++++++++++- 4 files changed, 207 insertions(+), 6 deletions(-) diff --git a/src/backend/access/gin/ginvacuum.c b/src/backend/access/gin/ginvacuum.c index d69d59748..7a1a1da7f 100644 --- a/src/backend/access/gin/ginvacuum.c +++ b/src/backend/access/gin/ginvacuum.c @@ -167,6 +167,8 @@ ginDeletePostingPage(GinVacuumState *gvs, Buffer dBuffer, Buffer lBuffer, page = BufferGetPage(dBuffer); rightlink = GinPageGetOpaque(page)->rightlink; + Assert(GinPageGetOpaque(BufferGetPage(lBuffer))->rightlink == deleteBlkno); + /* * Any insert which would have gone on the leaf block will now go to its * right sibling. @@ -334,10 +336,20 @@ ginScanPostingTreeToDelete(GinVacuumState *gvs, DataPageDeleteStack *myStackItem if (isempty) { /* - * Proceed to the ginDeletePostingPage() if that's not the leftmost or - * the rightmost page. + * Proceed to the ginDeletePostingPage() if target page is not the + * leftmost or the rightmost page. + * + * We cannot delete an empty target page whose left sibling is + * incompletely split; ginDeletePostingPage expects to find and remove + * such a target page's downlink, but there isn't any yet (there won't + * be until an inserter finishes the incomplete split). We also avoid + * deleting an empty target page that is itself incompletely split; + * allowing that case would make it impossible for us to detect that + * it's unsafe to delete the target's right sibling page later on. */ - if (BufferIsValid(myStackItem->leftBuffer) && !GinPageRightMost(page)) + if (BufferIsValid(myStackItem->leftBuffer) && !GinPageRightMost(page) && + !GinPageIsIncompleteSplit(page) && + !GinPageIsIncompleteSplit(BufferGetPage(myStackItem->leftBuffer))) { Assert(!myStackItem->isRoot); ginDeletePostingPage(gvs, buffer, myStackItem->leftBuffer, diff --git a/src/test/modules/gin/Makefile b/src/test/modules/gin/Makefile index e007e38ac..468e46d35 100644 --- a/src/test/modules/gin/Makefile +++ b/src/test/modules/gin/Makefile @@ -1,6 +1,6 @@ # src/test/modules/gin/Makefile -EXTRA_INSTALL = src/test/modules/injection_points +EXTRA_INSTALL = src/test/modules/injection_points contrib/pageinspect REGRESS = gin_incomplete_splits diff --git a/src/test/modules/gin/expected/gin_incomplete_splits.out b/src/test/modules/gin/expected/gin_incomplete_splits.out index 0f3ac9a04..ef346b75f 100644 --- a/src/test/modules/gin/expected/gin_incomplete_splits.out +++ b/src/test/modules/gin/expected/gin_incomplete_splits.out @@ -19,7 +19,6 @@ SELECT injection_points_set_local(); (1 row) --- Use the index for all the queries set enable_seqscan=off; -- Print a NOTICE whenever an incomplete split gets fixed SELECT injection_points_attach('gin-finish-incomplete-split', 'notice'); @@ -192,4 +191,115 @@ SELECT injection_points_detach('gin-finish-incomplete-split'); (1 row) +-- +-- Test that VACUUM does not delete the right sibling of an incompletely +-- split posting tree leaf page +-- +create extension pageinspect; +-- Create a GIN index with a posting tree that has several leaf pages +create temp table gin_posting_tree(id int4, i int4[]); +insert into gin_posting_tree select g, '{1}' from generate_series(1, 55000) g; +create index gin_posting_tree_idx on gin_posting_tree using gin (i) with (fastupdate = off); +-- Free space in the middle of the index key space/heap. The later inserts +-- will get TIDs in the middle of the posting tree's key space, splitting a +-- leaf page that is neither the leftmost nor rightmost of the posting tree. +delete from gin_posting_tree where id between 10001 and 27500; +vacuum (index_cleanup on) gin_posting_tree; +-- Insert rows until a leaf page split fails, leaving the split incomplete +SELECT injection_points_attach('gin-leave-leaf-split-incomplete', 'error'); + injection_points_attach +------------------------- + +(1 row) + +do $$ +begin + for n in 1..200000 loop + begin + insert into gin_posting_tree values (n, '{1}'); + exception when others then + return; + end; + end loop; + raise 'no leaf split after 200000 inserts'; +end; +$$; +SELECT injection_points_detach('gin-leave-leaf-split-incomplete'); + injection_points_detach +------------------------- + +(1 row) + +-- Locate the incompletely split page's new right half (reachable only +-- through its left sibling's rightlink), and the leaf to the right of +-- that (the page that VACUUM will delete). Errors out unless there is +-- exactly one incomplete split. +select o.rightlink::int as righthalf, + (gin_page_opaque_info(get_raw_page('gin_posting_tree_idx', + o.rightlink::int))).rightlink::int + as nextleaf + from generate_series(0, pg_relation_size('gin_posting_tree_idx') / + current_setting('block_size')::int - 1) blkno, + lateral gin_page_opaque_info(get_raw_page('gin_posting_tree_idx', + blkno::int)) o + where o.flags @> '{incomplete_split}' +\gset +-- Sanity check: scan will miss nothing if the right half held no live rows, +-- and VACUUM never deletes the rightmost page +select exists (select from gin_posting_tree + where ctid in (select unnest(tids) from gin_leafpage_items( + get_raw_page('gin_posting_tree_idx', :righthalf)))) + as righthalf_has_live_rows, + (gin_page_opaque_info(get_raw_page('gin_posting_tree_idx', + :nextleaf))).rightlink <> 4294967295 + as nextleaf_is_not_rightmost; + righthalf_has_live_rows | nextleaf_is_not_rightmost +-------------------------+--------------------------- + t | t +(1 row) + +-- Empty the page to the right of the right half, and have VACUUM consider +-- deleting it +delete from gin_posting_tree + where ctid in (select unnest(tids) from gin_leafpage_items( + get_raw_page('gin_posting_tree_idx', :nextleaf))); +vacuum (index_cleanup on) gin_posting_tree; +-- Verify that bitmap scan finds every remaining row +explain (costs off) +select count(*) as onecount from gin_posting_tree where i @> '{1}'; + QUERY PLAN +------------------------------------------------------- + Aggregate + -> Bitmap Heap Scan on gin_posting_tree + Recheck Cond: (i @> '{1}'::integer[]) + -> Bitmap Index Scan on gin_posting_tree_idx + Index Cond: (i @> '{1}'::integer[]) +(5 rows) + +select count(*) as onecount from gin_posting_tree where i @> '{1}'; + onecount +---------- + 39879 +(1 row) + +-- Verify that a sequential scan gives the same result +set enable_seqscan=on; +set enable_bitmapscan=off; +explain (costs off) +select count(*) as onecount from gin_posting_tree where i @> '{1}'; + QUERY PLAN +----------------------------------------- + Aggregate + -> Seq Scan on gin_posting_tree + Filter: (i @> '{1}'::integer[]) +(3 rows) + +select count(*) as onecount from gin_posting_tree where i @> '{1}'; + onecount +---------- + 39879 +(1 row) + +drop table gin_posting_tree; +drop extension pageinspect; drop extension injection_points; diff --git a/src/test/modules/gin/sql/gin_incomplete_splits.sql b/src/test/modules/gin/sql/gin_incomplete_splits.sql index d451257c2..3b8a60aff 100644 --- a/src/test/modules/gin/sql/gin_incomplete_splits.sql +++ b/src/test/modules/gin/sql/gin_incomplete_splits.sql @@ -17,7 +17,6 @@ create extension injection_points; -- Make all injection points local to this process, for concurrency. SELECT injection_points_set_local(); --- Use the index for all the queries set enable_seqscan=off; -- Print a NOTICE whenever an incomplete split gets fixed @@ -149,4 +148,84 @@ select verify(:next_i); SELECT injection_points_detach('gin-finish-incomplete-split'); +-- +-- Test that VACUUM does not delete the right sibling of an incompletely +-- split posting tree leaf page +-- +create extension pageinspect; + +-- Create a GIN index with a posting tree that has several leaf pages +create temp table gin_posting_tree(id int4, i int4[]); +insert into gin_posting_tree select g, '{1}' from generate_series(1, 55000) g; +create index gin_posting_tree_idx on gin_posting_tree using gin (i) with (fastupdate = off); + +-- Free space in the middle of the index key space/heap. The later inserts +-- will get TIDs in the middle of the posting tree's key space, splitting a +-- leaf page that is neither the leftmost nor rightmost of the posting tree. +delete from gin_posting_tree where id between 10001 and 27500; +vacuum (index_cleanup on) gin_posting_tree; + +-- Insert rows until a leaf page split fails, leaving the split incomplete +SELECT injection_points_attach('gin-leave-leaf-split-incomplete', 'error'); +do $$ +begin + for n in 1..200000 loop + begin + insert into gin_posting_tree values (n, '{1}'); + exception when others then + return; + end; + end loop; + raise 'no leaf split after 200000 inserts'; +end; +$$; +SELECT injection_points_detach('gin-leave-leaf-split-incomplete'); + +-- Locate the incompletely split page's new right half (reachable only +-- through its left sibling's rightlink), and the leaf to the right of +-- that (the page that VACUUM will delete). Errors out unless there is +-- exactly one incomplete split. +select o.rightlink::int as righthalf, + (gin_page_opaque_info(get_raw_page('gin_posting_tree_idx', + o.rightlink::int))).rightlink::int + as nextleaf + from generate_series(0, pg_relation_size('gin_posting_tree_idx') / + current_setting('block_size')::int - 1) blkno, + lateral gin_page_opaque_info(get_raw_page('gin_posting_tree_idx', + blkno::int)) o + where o.flags @> '{incomplete_split}' +\gset + +-- Sanity check: scan will miss nothing if the right half held no live rows, +-- and VACUUM never deletes the rightmost page +select exists (select from gin_posting_tree + where ctid in (select unnest(tids) from gin_leafpage_items( + get_raw_page('gin_posting_tree_idx', :righthalf)))) + as righthalf_has_live_rows, + (gin_page_opaque_info(get_raw_page('gin_posting_tree_idx', + :nextleaf))).rightlink <> 4294967295 + as nextleaf_is_not_rightmost; + +-- Empty the page to the right of the right half, and have VACUUM consider +-- deleting it +delete from gin_posting_tree + where ctid in (select unnest(tids) from gin_leafpage_items( + get_raw_page('gin_posting_tree_idx', :nextleaf))); +vacuum (index_cleanup on) gin_posting_tree; + +-- Verify that bitmap scan finds every remaining row +explain (costs off) +select count(*) as onecount from gin_posting_tree where i @> '{1}'; +select count(*) as onecount from gin_posting_tree where i @> '{1}'; + +-- Verify that a sequential scan gives the same result +set enable_seqscan=on; +set enable_bitmapscan=off; +explain (costs off) +select count(*) as onecount from gin_posting_tree where i @> '{1}'; +select count(*) as onecount from gin_posting_tree where i @> '{1}'; + +drop table gin_posting_tree; + +drop extension pageinspect; drop extension injection_points; -- 2.53.0