From: Ronan Dunklau <ronan.dunklau@aiven.io>
To: Noah Misch <noah@leadboat.com>
Cc: pgsql-bugs <pgsql-bugs@lists.postgresql.org>
Subject: Re: FSM Corruption (was: Could not read block at end of the relation)
Date: Tue, 05 Mar 2024 00:33:13 +0100
Message-ID: <3470254.QJadu78ljV@aivenlaptop> (raw)
In-Reply-To: <20240304230503.a4.nmisch@google.com>
References: <VtPLBUerbMM8BSDLQwIMV9Co_38iHS1TXE4V09x2ZVFhPkZMNE8LwgPSIzB8PHkVFXykGx04QNAzZVjJFAnU1w==@protonmail.internalid>
<3549385.iIbC2pHGDl@aivenlaptop>
<20240304230503.a4.nmisch@google.com>
Le mardi 5 mars 2024, 00:05:03 CET Noah Misch a écrit :
> Does that procedure alone reproduce the ERROR, without any replica promote,
> postmaster restart, etc.? That, I do not have reason to expect.
Oh no sorrry for not being clear: it needs to go through recovery either
through a backup restoration, a replica or anything else. This is why I've
been struggling to reproduce the issue, it didn't occur to me that the fsm was
only sane on the primary, as the extensions were not wal logged.
>
> > > I think the fix is one of:
> > >
> > > - Revoke the undocumented rule. Make FSM consumers resilient to the FSM
> > >
> > > returning a now-too-large block number.
> >
> > Performance wise, that seems to be the better answer
>
> I would guess this one is more risky from a performance perspective, since
> we'd be adding to a hotter path under RelationGetBufferForTuple(). Still,
> it's likely fine.
Makes sense.
>
> > but won't this kind of
> > built-in resilience encourage subtle bugs creeping in ?
>
> Since the !wal_log_hints case avoids WAL for FSM, we've already accepted
> subtle bugs. I bet an unlucky torn FSM page could reach the ERROR you
> reached, even if we introduced a "main-fork WAL before FSM" rule. We'll
> have plenty of need for resilience. Hence, I'd lean toward this approach.
> > > - Enforce a new "main-fork WAL before FSM" rule for logged rels. For
> > > example, in each PageIsNew() case, either don't update FSM or WAL-log an
> > > init (like lazy_scan_new_or_empty() does when PageIsEmpty()).
> >
> > The FSM is not updated by the caller: in the bulk insert case, we
> > intentionally don't add them to the FSM. So having VACUUM WAL-log the page
> > in lazy_scan_new_or_empty in the New case as you propose seems a very
> > good idea.
> >
> > Performance wise that would keep the WAL logging outside of the backend
> > performing the preventive work for itself or others, and it should not be
> > that many pages that it gives VACUUM too much work.
>
> A drawback is that this approach touches several access methods, so
> out-of-tree access methods also likely need changes. Still, this is a good
> backup if the first option measurably slows INSERT throughput.
Would it actually ? At least for the currently discussed VACUUM case, we will
fall back to a generic FPI for the new page, which should work on any AM. But
the requirement to WAL log the page before recording it in the FSM should be
publicized, and the common infrastructure should adhere to it. I'd have to
look into the other cases you mentioned where that can happen.
>
> > I can try my hand at such a patch it if looks like a good idea.
>
> Please do.
>
> > Thank you very much for your insights.
>
> Thanks for the detailed report that made it possible.
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: ronan.dunklau@aiven.io, noah@leadboat.com, pgsql-bugs@lists.postgresql.org
Subject: Re: FSM Corruption (was: Could not read block at end of the relation)
In-Reply-To: <3470254.QJadu78ljV@aivenlaptop>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox