Hi, I found a race between index-only scans and heap_delete()/heap_update() that lets two SERIALIZABLE transactions both commit even though their reads are inconsistent with any serial execution order. I reproduced it on master at 374522aa63a.
heap_delete() and heap_update() check for conflicts before changing the tuple and clearing the page's visibility-map bit. Their comments assume that holding the heap buffer lock excludes readers until the change is visible. An index-only scan can still read the old index entry while the VM bit is set, without taking that buffer lock. Here is the DELETE case. Each table has one row, and both have been vacuumed so that the reader can use an index-only scan: CREATE TABLE ios_a (id int PRIMARY KEY); CREATE TABLE ios_b (id int, present bool DEFAULT true); CREATE INDEX ios_b_idx ON ios_b (id) WHERE present; INSERT INTO ios_a VALUES (1); INSERT INTO ios_b VALUES (1); VACUUM (FREEZE, ANALYZE) ios_a; VACUUM (FREEZE, ANALYZE) ios_b; Both sessions start with BEGIN ISOLATION LEVEL SERIALIZABLE, with enable_seqscan and enable_bitmapscan off. Using GDB, I paused s2 in heap_delete() just after CheckForSerializableConflictIn() returned, before clearing the VM bit: s2: SELECT id FROM ios_a; -- returns 1 s2: DELETE FROM ios_b; -- paused s1: SELECT id FROM ios_b WHERE present; -- returns 1 (index-only) s1: DELETE FROM ios_a; s1: COMMIT; -- Resume s2. s2: COMMIT; -- also succeeds Neither serial order produces these reads: running s1 first would make s2's SELECT return no rows, and running s2 first would make s1's SELECT return no rows. There is also a reader-side window. On fresh copies of the tables, I paused s1 in heapam_index_getnext_slot(), after VM_ALL_VISIBLE returned true but before PredicateLockPage(): s2: SELECT id FROM ios_a; -- returns 1 s1: SELECT id FROM ios_b WHERE present; -- paused s2: DELETE FROM ios_b; s2: COMMIT; -- Resume s1: its SELECT returns 1 without checking the heap. s1: DELETE FROM ios_a; s1: COMMIT; -- also succeeds Replacing s2's DELETE with UPDATE ios_b SET present = false reproduces both cases too. The new row no longer satisfies ios_b_idx's WHERE present condition, so no new index entry is inserted. This avoids an index insertion's conflict check hiding the heap race. The attached patch adds a second conflict-in check to heap_delete() and heap_update() after clearing an all-visible bit, outside the critical section and before releasing the heap buffer lock. In heapam_index_getnext_slot(), a SERIALIZABLE index-only scan rechecks the VM bit after PredicateLockPage() and fetches the heap if it was cleared. The writer rechecks for predicate locks acquired since its first check. The reader rechecks for a VM bit cleared before it acquired its predicate lock. Both changes are needed. The patch includes injection-point tests for both orderings, with DELETE and UPDATE. With the fix, all four cases abort one transaction with 40001. The same schedules still allow both transactions to commit under READ COMMITTED and REPEATABLE READ. make check, the core isolation suite, and injection_points tests pass. Regards, Rui
v1-0001-Fix-SSI-conflicts-between-index-only-scans-and-he.patch
Description: Binary data
