From: Ronan Dunklau <ronan.dunklau@aiven.io>
To: Noah Misch <noah@leadboat.com>, 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, 06 Mar 2024 10:31:17 +0100
Message-ID: <1958756.PYKUYFuaPT@aivenlaptop>
In-Reply-To: <3609363.R56niFO833@aivenlaptop>
References: 
 <VtPLBUerbMM8BSDLQwIMV9Co_38iHS1TXE4V09x2ZVFhPkZMNE8LwgPSIzB8PHkVFXykGx04QNAzZVjJFAnU1w==@protonmail.internalid>
 <TlHm41R71dghOEOJPrzhuxhrDI1R50MRIXHcVzU9BuJpSPKXypIYH9m5BrdxSGM_tE3DlXKFXLesUnd7vR1pxg==@protonmail.internalid>
 <3609363.R56niFO833@aivenlaptop>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="nextPart22206681.EfDdHjke4D"
Content-Transfer-Encoding: 7Bit
Archived-At: 
 <https://www.postgresql.org/message-id/1958756.PYKUYFuaPT%40aivenlaptop>
Precedence: bulk

This is a multi-part message in MIME format.

--nextPart22206681.EfDdHjke4D
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="iso-8859-1"

Le mardi 5 mars 2024, 00:05:03 CET Noah Misch a =E9crit :
> 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 det=
ect=20
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.

=46or the corrupted-FSM case it is not great performance wise, as we will e=
xtend=20
the relation in small steps every time we find a non existing block in the =
=46SM,=20
until the actual relation size matches what is recorded in the FSM. But sin=
ce=20
those seldom happen, I figured it was better to keep the code really simple=
 for=20
a bugfix.

I wanted to test the impact in terms of performance, and I thought about th=
e=20
worst possible case for this.

Then, run a pgbench doing insertions in the table. With the attached patch =
the=20
worst case I could come up with is:=20
  - remember which page we last inserted into
  - notice we don't have enough space
  - ask the FSM for a block
  - now have to compare that to the actual relation size

So I came up with the following initialization steps:

 - create a table with vacuum_truncate =3D off, with a tuple size big enoug=
h that=20
it's impossible to fit two tuples on the same page
 - insert lots of tuple in it until it reaches a decent size
 - delete them all
 - vacuum
 - all of this fitting in shared_buffers

As in:

CREATE TABLE test_perf (c1 char(5000));
ALTER TABLE test_perf ALTER c1 SET STORAGE PLAIN;
ALTER TABLE test_perf SET (VACUUM_TRUNCATE =3D off);
INSERT INTO test_perf (c1) SELECT 'c' FROM generate_series(1, 1000000);
DELETE FROM test_perf;
VACUUM test_perf;

Then I ran pgbench with a single client, with a script only inserting the s=
ame=20
value over and over again, for 1000000 transactions (initial table size).

I noticed no difference running with or without the patch, but maybe someon=
e=20
else can try to run that or find another adversarial case ?=20

Best regards,

=2D-
Ronan Dunklau
--nextPart22206681.EfDdHjke4D
Content-Disposition: attachment;
 filename="0001-Detect-invalid-FSM-when-finding-a-block.patch"
Content-Transfer-Encoding: 7Bit
Content-Type: text/x-patch; charset="x-UTF_8J";
 name="0001-Detect-invalid-FSM-when-finding-a-block.patch"