| From: | Melanie Plageman <melanieplageman(at)gmail(dot)com> |
|---|---|
| To: | Andrey Borodin <x4mmm(at)yandex-team(dot)ru> |
| Cc: | Andres Freund <andres(at)anarazel(dot)de>, Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
| Subject: | Re: Why clearing the VM doesn't require registering vm buffer in wal record |
| Date: | 2026-10-13 19:58:18 |
| Message-ID: | CAAKRu_bAWOm4Sf-bd6pXot6-b5k_S8i9MP=KNiJVC_QEr6MkDA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, Sep 24, 2026 at 7:37 PM Melanie Plageman
<melanieplageman(at)gmail(dot)com> wrote:
>
> On Tue, Sep 22, 2026 at 2:54 AM Andrey Borodin <x4mmm(at)yandex-team(dot)ru> 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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Harrison Booth | 2026-10-13 20:58:52 | [PATCH] pg_upgrade: handle failed nonblocking connects on Windows |
| Previous Message | Zsolt Parragi | 2026-10-13 19:43:41 | Re: find_all_inheritors() with NoLock fails on concurrently dropped sub-partitions |