Re: FSM Corruption (was: Could not read block at end of the relation)
Noah Misch <noah@leadboat.com>
From: Noah Misch <noah@leadboat.com>
To: Ronan Dunklau <ronan.dunklau@aiven.io>
Cc: pgsql-bugs <pgsql-bugs@lists.postgresql.org>
Date: 2024-03-05T00:13:46Z
Lists: pgsql-bugs
On Tue, Mar 05, 2024 at 12:33:13AM +0100, Ronan Dunklau wrote:
> Le mardi 5 mars 2024, 00:05:03 CET Noah Misch a écrit :
> > > > - 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 agree each AM can use generic FPI code. I'm looking at code like this as
needing change under this candidate design:
static void
spgvacuumpage(spgBulkDeleteState *bds, BlockNumber blkno)
{
...
if (PageIsNew(page) || PageIsEmpty(page))
{
RecordFreeIndexPage(index, blkno);
The change there would be fairly simple, but non-core access methods may
require similar simple changes. That's fine if this approach has other
reasons to win, but it is a drawback. A grep of pgxn code shows "rum" as the
only module containing a RecordFreeIndexPage() call. No module contains a
RecordPageWithFreeSpace() call. So this drawback is all but negligible.
Commits
-
freespace: Don't return blocks past the end of the main fork.
- 08059fc049ff 14.12 landed
- 7c490a18b75b 15.7 landed
- 4e62ba21a921 16.3 landed
- 935829743151 17.0 landed
-
Fix WAL-logging of FSM and VM truncation.
- 917dc7d2393c 10.0 cited
-
Extend relations multiple blocks at a time to improve scalability.
- 719c84c1be51 9.6.0 cited