On Thu, Sep 24, 2026 at 7:37 PM Melanie Plageman <[email protected]> wrote: > > On Tue, Sep 22, 2026 at 2:54 AM Andrey Borodin <[email protected]> wrote: > > > Also, if we accept a possible torn page from this repair, what happens > > on the next recovery? The new reader uses RBM_NORMAL_NO_LOG, not the > > VM's usual RBM_ZERO_ON_ERROR, so a checksum failure could stop recovery > > before it gets to the repair. > > Yea, I did this on purpose because since ed62d26caca, I didn't read > the VM with ZERO_ON_ERROR, which I thought was okay since we were > always registering the VM blocks in the wal record for both setting > and clearing. > > However, this isn't a good idea when 17 and 18 have a fallback path > that lets us read the VM in recovery (when clearing it) in a way that > could result in a torn page. Then we've created a way for the VM to > get corrupt and made it error out reading it during recovery. > > > For this repair, shouldn't we either use RBM_ZERO_ON_ERROR on every > > relevant recovery read path, or require an FPI for the VM page since > > the last checkpoint's redo point before allowing the modification? > > Yea, I've thought about this a lot the last few days. On back branches > (and as a stop-gap in master), I think we have to use > RBM_ZERO_ON_ERROR everywhere and be okay with tearing pages when > reading VM pages during recovery.
I've thought about this more, and I've changed my mind. I see no way that using RBM_NORMAL in the way that I do in heap_xlog_vm_clear() can lead to halting recovery from a VM page that we corrupted ourselves (of course disk corruption can always happen). The exception is that other VM truncation thread [1], but that can be fixed with XLogTruncateRelation(). If you can come up with a repro, perhaps I'm wrong, but, even in the case of divergence between primary and standby, the normal visibilitymap_clear() fallback path does RBM_ZERO_ON_ERROR. So, even if you corrupted a VM page by crashing on the standby, any record replayed with XLogReadBufferForRedo() is guaranteed to have an FPI if it needs one. heap_xlog_vm_clear only reaches RBM_NORMAL on the registered branch, i.e. when the primary actually cleared a set VM bit. That record is FPI-free only if the VM page was already modified since the primary's redo point — meaning an earlier FPI-bearing record for that same page exists in the stream. For redo to read a torn page, it must resume past that earlier record, which needs a restartpoint between them. But restartpoints only ever derive from a replayed primary checkpoint record. A checkpoint there moves the redo point, which makes the clear first-since-redo-point again — so it carries its own FPI. Either the redo resumes before the protecting record (which re-images the page) or the clear itself gets an FPI. The RBM_NORMAL read never sees an unhealed torn page, so it can't halt recovery. > v2-0001 is 19-only and fixes the divergent VM bits on the standby by > reading the VM when the VM block wasn't registered > master-v2-0001 is the stop-gap for master that does the same thing but > in a different way because it doesn't have a fake relcache entry These should still be applied. I plan to fix the CREATE DATABASE USING STRATEGY WAL_LOG issue as well. But, if divergence can happen another way, we'd still need this failsafe clear-the-vm patches on 19 and master. It doesn't help with tuple locking clearing all-frozen though. > v2-0002 fixes VM clear to read the VM with RBM_ZERO_ON_ERROR. This > should apply master -> 17 I no longer think we should do this. > v2-0003 is a fix for 19 and master for an issue I found while working > on this where the VM wasn't guaranteed to be dirty by the time we > logged setting it in heap_page_prune_and_freeze(). This can happen if > it was already set all-frozen and all-visible but it contained any > non-frozen tuples (i.e. VM is corrupt with incorrect all-frozen bit). > The VM set operation would be a no-op and no one would fix the corrupt > all-frozen bit. This corruption was never fixed, but in 19 the > vm-setting code was rearranged to expect that if we try and set the > VM, we will dirty it. For that to be true, we have to fix this > corruption. And, since the VM corruption detection function is new in > 19, it is easiest to just backpatch it that far. This is still worth doing. - Melanie [1] https://www.postgresql.org/message-id/CAGRkXqSZXoazu1x48pz2SuW_xHYQDdRcbp2HNCfRNm_WLF8aYA%40mail.gmail.com
