Hi Andrey, > I was reviewing the patch about removing xl_heap_visible and found the VM\WAL > machinery very interesting. > At Yandex we had several incidents with corrupted VM and on pgconf.dev > colleagues from AWS confirmed that they saw something similar too. > So I toyed around and accidentally wrote a test that reproduces $subj. > > I think the corruption happens as follows: > 0. we create a table with one frozen tuple > 1. next heap_insert() clears VM bit and hangs immediately, nothing was logged > yet > 2. VM buffer is flushed on disk with checkpointer or bgwriter > 3. primary is killed with -9 > now we have a page that is ALL_VISIBLE\ALL_FORZEN on standby, but clear VM > bits on primary > 4. subsequent insert does not set XLH_LOCK_ALL_FROZEN_CLEARED in it's WAL > record > 5. pg_visibility detects corruption > > Interestingly, in an off-list conversation Melanie explained me how > ALL_VISIBLE is protected from this: WAL-logging depends on PD_ALL_VISIBLE > heap page bit, not a state of the VM. But for ALL_FROZEN this is not a case: > > /* Clear only the all-frozen bit on visibility map if needed */ > if (PageIsAllVisible(page) && > visibilitymap_clear(relation, block, vmbuffer, > VISIBILITYMAP_ALL_FROZEN)) > cleared_all_frozen = true; // this won't happen due to flushed VM > buffer before a crash > > Anyway, the test reproduces corruption of both bits. And also reproduces > selecting deleted data on standby.
Great find. I executed your test on a pretty much regular Linux x64 machine and indeed it failed: ``` not ok 1 - pg_check_frozen() observes corruption not ok 2 - pg_check_visible() observes corruption not ok 3 - deleted data returned by select 1..3 # test failed ----------------------------------- stderr ----------------------------------- # Failed test 'pg_check_frozen() observes corruption' # at /home/eax/projects/c/postgresql/src/test/modules/test_slru/t/001_multixact.pl line 110. # got: '(0,2) # (0,3) # (0,4)' # expected: '' # Failed test 'pg_check_visible() observes corruption' # at /home/eax/projects/c/postgresql/src/test/modules/test_slru/t/001_multixact.pl line 111. # got: '(0,2) # (0,4)' # expected: '' # Failed test 'deleted data returned by select' # at /home/eax/projects/c/postgresql/src/test/modules/test_slru/t/001_multixact.pl line 112. # got: '2' # expected: '' # Looks like you failed 3 tests of 3. ``` This is a tricky bug. Do you also have a proposal of a particular fix? > The test is not intended to be committed when we fix the problem, so some > waits are simulated with sleep(1) and test is placed at modules/test_slru > where it was easier to write. But if we ever want something like this - I can > design a less hacky version. And, probably, more generic. IMO - yes, we do need this regression test.
