Thread

  1. Re: Changing the state of data checksums in a running cluster

    Ayush Tiwari <ayushtiwari.slg01@gmail.com> — 2026-05-05T15:21:22Z

    Hi,
    
    On Tue, 5 May 2026 at 19:16, Daniel Gustafsson <daniel@yesql.se> wrote:
    
    > > While further testing this feature, I realized that
    > ProcessSingleRelationFork()
    > > unconditionally called log_newpage_buffer() for every page of every
    > relation
    > > during pg_enable_data_checksums(). This included unlogged relations,
    > > which by definition never generate WAL for data changes and are reset to
    > their
    > > init fork on any recovery.
    >
    >
    > I've tested your patch, and also expanded the test you wrote a little with
    > a
    > persistence change.
    >
    >
    I tested the patches, it mostly looks good.
    
    I've a small concern in 0001.  The new guard uses only
    RelationNeedsWAL(reln),
    but ProcessSingleRelationByOid() iterates all forks.  For unlogged
    relations,
    the init fork is special, there are several existing call sites that
    preserve
    WAL for INIT_FORKNUM, for example using
    
      RelationNeedsWAL(rel) || forknum == INIT_FORKNUM
    
    and catalog/storage.c notes that unlogged init forks need WAL and sync.
    
    So I think the condition in ProcessSingleRelationFork() should preserve the
    init-fork case, e.g.
    
      if (RelationNeedsWAL(reln) || forkNum == INIT_FORKNUM)
          log_newpage_buffer(buf, false);
    
    It would also be good to extend the new test so it exercises a non-empty
    init
    fork, perhaps with an unlogged table that has an index, and then verifies
    the
    standby/recovery behavior.
    
    0002 and 0003 look good to me.
    
    Regards,
    Ayush