| From: | Kirill Reshke <reshkekirill(at)gmail(dot)com> |
|---|---|
| To: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Fix GIN pending cleanup race |
| Date: | 2026-11-08 18:03:45 |
| Message-ID: | CALdSSPiPALHtGFFpy+evjXe0+XgKNLsQ4P14Zt_sxxPtde1_xg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi!
I was reviewing [0] and benchmarking/checking the correctness of GIN
indexes and did find something that is a server bug in master (and
affects all versions 8.4+). The bug is racy and doesn't hit in a
single session run.
A repro would be this data
CREATE EXTENSION injection_points;
CREATE TABLE w(id int, d tsvector);
CREATE INDEX w_gin ON w USING gin(d) WITH (fastupdate=on);
INSERT INTO w SELECT g, to_tsvector('simple','common x')
FROM generate_series(1,1) g; -- ok
INSERT INTO w SELECT g, to_tsvector('simple','common updated')
FROM generate_series(2,2) g; -- not ok
and this query
SELECT count(*) FROM w WHERE d @@ 'common & !updated'::tsquery;
-- racy result: 2
-- seqscan: 1
Shortly, rootcause is lack of rechecking main tree index entries for
recently-transferred-entries from fastupdate list.
So, a GIN index with fast update list transfers entries from pending
list to main tree deliberately without holding any lock, see this code
listing at ginfast.c:
/*
* Moving collected data into regular structure can take
* significant amount of time - so, run it without locking pending
* list.
*/
ginBeginBAScan(&accum);
while ((list = ginGetBAEntry(&accum, &attnum, &key, &category, &nlist)) != NULL)
{
ginEntryInsert(ginstate, attnum, key, category, list, nlist, NULL);
vacuum_delay_point(false);
}
So, for some rows, it can happen that some entries for this row are
already in the main GIN index while some are not. In the example,
imagine that cleanup insert the key 'common' but not yet 'updated'.
keyGetItem() passes entryRes['updated'] = GIN_FALSE ("certainly
absent") to the
consistent function, which after negation makes the query true and
passese without a recheck.
So, index scan returns the wrong row.
The following deterministic repro uses 0002 from attached
reshke=# SELECT count(*) FROM w WHERE d @@ 'common & !updated'::tsquery;
count
-------
1
(1 row)
reshke=#
-- session 2 (the cleaner, pauses before each key insert):
SELECT injection_points_attach('gin-pending-cleanup-key','wait');
SELECT gin_clean_pending_list('w_gin');
injection_points_attach
-------------------------
(1 row)
reshke=#
-- session 1: let the wave insert just the first key ('common'):
SELECT injection_points_wakeup('gin-pending-cleanup-key');
injection_points_wakeup
-------------------------
(1 row)
reshke=# SELECT count(*) FROM w WHERE d @@ 'common & !updated'::tsquery;
count
-------
2
(1 row)
reshke=# explain SELECT count(*) FROM w WHERE d @@ 'common &
!updated'::tsquery;
QUERY PLAN
---------------------------------------------------------------------------
Aggregate (cost=26.75..26.76 rows=1 width=8)
-> Bitmap Heap Scan on w (cost=17.26..26.73 rows=6 width=0)
Recheck Cond: (d @@ '''common'' & !''updated'''::tsquery)
-> Bitmap Index Scan on w_gin (cost=0.00..17.26 rows=6 width=0)
Index Cond: (d @@ '''common'' & !''updated'''::tsquery)
(5 rows)
With use of [1] patches for GIN pageinspect we can see
reshke=# select * from gin_entrypage_items(get_raw_page('w_gin', 1),
'w_gin'::regclass);
itemoffset | downlink | tids | keys
------------+----------------+-------------------+----------
1 | (2147483664,2) | {"(0,1)","(0,2)"} | d=common
(1 row)
after one more injection_points_wakeup('gin-pending-cleanup-key'):
reshke=# select * from gin_entrypage_items(get_raw_page('w_gin', 1),
'w_gin'::regclass);
itemoffset | downlink | tids | keys
------------+----------------+-------------------+-----------
1 | (2147483664,2) | {"(0,1)","(0,2)"} | d=common
2 | (2147483664,1) | {"(0,2)"} | d=updated
(2 rows)
So, to fix this, I can see two alternatives. First one is to somehow
force tsvector opclass to set a recheck flag for such cases. But
opclass know little about GIN internals so looks like this would
force to recheck query almost every time, so performance regression.
0001 attached takes different approach: we remember all
possibly-in-doubt keys in sorted array (during pending list
traversal), dedup this and force recheck for them in main tree
traversal.
[0] https://www.postgresql.org/message-id/5d366878-2007-4d31-861e-19294b7a583b%40gmail.com
[1] https://www.postgresql.org/message-id/SY7PR01MB10921891275CC615D3282326EB6B72@SY7PR01MB10921.ausprd01.prod.outlook.com
--
Best regards,
Kirill Reshke
| Attachment | Content-Type | Size |
|---|---|---|
| 0002-injection-point-repro-only.diff | application/octet-stream | 520 bytes |
| 0001-fix-gin-pending-race-2.diff | application/octet-stream | 3.9 KB |
| From | Date | Subject | |
|---|---|---|---|
| Previous Message | Tom Lane | 2026-11-08 17:49:09 | Re: Draft back-branch release notes are up |