Thread
-
Re: eliminate xl_heap_visible to reduce WAL (and eventually set VM on-access)
Kirill Reshke <reshkekirill@gmail.com> — 2025-12-17T18:27:05Z
Hi! in v27-0001: > Melanie Plageman <melanieplageman(at)gmail(dot)com> wrote: > > The last vacuum is expected to set vm bits, but the test doesn’t verify that. Should we verify that like: > > ``` > > evantest=# SELECT blkno, all_visible, all_frozen FROM pg_visibility_map('test_vac_unmodified_heap'); > > blkno | all_visible | all_frozen > > -------+-------------+------------ > > 0 | t | t > > (1 row) > I've done this. I've actually added three such verifications -- one > after each step where the VM is expected to change. It shouldn't be > very expensive, so I think it is okay. The way the test would fail if > the buffer wasn't correctly dirtied is that it would assert out -- so > the visibility map test wouldn't even have a chance to fail. But, I > think it is also okay to confirm that the expected things are > happening with the VM -- it just gives us extra coverage. +1 on extra coverage. Should we also do sql-level check that the VM indeed does not need to set PD_ALL_VISIBLE (check header bytes using pageinspect?). v27-0003 & v27-0004: I did not get the exact reason we introduced `identify_and_fix_vm_corruption` in 0003 and moved code in 0004 to another place. I can see we have this starting v25 of patch set. Well, maybe this is not an issue at all... in v27-0005. This patch changes code which is not exercised in tests[0]. I spent some time understanding the conditions when we entered this. There is a comment about non-finished relation extension, but I got no success trying to reproduce this. I ended up modifying code to lose PageSetAllVisible in proper places and running vacuum. Looks like everything works as expected. I will spend some more time on this, maybe I will be successful in writing an injection-point-based TAP test which hits this... [0] https://coverage.postgresql.org/src/backend/access/heap/vacuumlazy.c.gcov.html#1902 -- Best regards, Kirill Reshke