Thread

  1. 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