From: Noah Misch <noah@leadboat.com>
To: Ronan Dunklau <ronan.dunklau@aiven.io>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-bugs <pgsql-bugs@lists.postgresql.org>
Subject: Re: FSM Corruption (was: Could not read block at end of the relation)
Date: Wed, 6 Mar 2024 10:42:28 -0800
Message-ID: <20240306184228.a7.nmisch@google.com> (raw)
In-Reply-To: <1958756.PYKUYFuaPT@aivenlaptop>
References: <VtPLBUerbMM8BSDLQwIMV9Co_38iHS1TXE4V09x2ZVFhPkZMNE8LwgPSIzB8PHkVFXykGx04QNAzZVjJFAnU1w==@protonmail.internalid>
<TlHm41R71dghOEOJPrzhuxhrDI1R50MRIXHcVzU9BuJpSPKXypIYH9m5BrdxSGM_tE3DlXKFXLesUnd7vR1pxg==@protonmail.internalid>
<3609363.R56niFO833@aivenlaptop>
<1958756.PYKUYFuaPT@aivenlaptop>
On Wed, Mar 06, 2024 at 10:31:17AM +0100, Ronan Dunklau wrote:
> Le mardi 5 mars 2024, 00:05:03 CET Noah Misch a écrit :
> > 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.
>
> I ended up implementing this in the attached patch. The idea is that we detect
> if the FSM returns a page past the end of the relation, and ignore it.
> In that case we will fallback through the extension mechanism.
>
> For the corrupted-FSM case it is not great performance wise, as we will extend
> the relation in small steps every time we find a non existing block in the FSM,
> until the actual relation size matches what is recorded in the FSM. But since
> those seldom happen, I figured it was better to keep the code really simple for
> a bugfix.
That could be fine. Before your message, I assumed we would treat this like
the case of a full page. That is, we'd call RecordAndGetPageWithFreeSpace(),
which would remove the FSM entry and possibly return another. That could be
problematic if another backend is extending the relation; we don't want to
erase the extender's new FSM entry. I'm parking this topic for the moment.
> I wanted to test the impact in terms of performance, and I thought about the
> worst possible case for this.
>
[test details]
>
> I noticed no difference running with or without the patch, but maybe someone
> else can try to run that or find another adversarial case ?
The patch currently adds an lseek() whenever the FSM finds a block. I see
relevant-looking posts from mailing list searches with subsets of these terms:
RelationGetNumberOfBlocks lseek benchmark overhead. I bet we should reduce
this cost add. Some ideas:
- No need for a system call if the FSM-returned value is low enough with
respect to any prior RelationGetNumberOfBlocks() call.
- We'd be happy and efficient with a ReadBufferMode such that
ReadBufferExtended() returns NULL after a 0-byte read, with all other errors
handled normally. That sounds like the specification of RBM_NORMAL_NO_LOG.
Today's implementation of RBM_NORMAL_NO_LOG isn't that, but perhaps one RBM
could satisfy both sets of needs.
On Wed, Mar 06, 2024 at 12:08:38PM +0100, Ronan Dunklau wrote:
> Subject: [PATCH 2/2] Detect invalid FSM when finding a block.
>
> Whenever the FSM points to a block past the end of the relation, detect
> it and fallback to the relation extension mechanism.
>
> This may arise when a FPI for the FSM has been triggered, and we end up
> WAL-logging a page of the FSM pointing to newly extended blocks in the
> relation which have never been WAL-logged.
> ---
> src/backend/access/heap/hio.c | 20 +++++++++++++++++++-
Each GetPageWithFreeSpace() caller would need this, not just heap. Please
evaluate whether this logic belongs inside freespace.c vs. in each caller.
freespace.c comments and/or freespace/README may need updates. Please
evaluate header comments of freespace.c functions whose callers you are
changing, and evaluate comments about truncation.
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: noah@leadboat.com, ronan.dunklau@aiven.io, michael@paquier.xyz, pgsql-bugs@lists.postgresql.org
Subject: Re: FSM Corruption (was: Could not read block at end of the relation)
In-Reply-To: <20240306184228.a7.nmisch@google.com>
* 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