public inbox for [email protected]
help / color / mirror / Atom feedFrom: Andres Freund <[email protected]>
To: Melanie Plageman <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Kirill Reshke <[email protected]>
Cc: Andrey Borodin <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Heikki Linnakangas <[email protected]>
Subject: Re: eliminate xl_heap_visible to reduce WAL (and eventually set VM on-access)
Date: Thu, 18 Sep 2025 12:48:45 -0400
Message-ID: <yn4zp35kkdsjx6wf47zcfmxgexxt4h2og47pvnw2x5ifyrs3qc@7uw6jyyxuyf7> (raw)
In-Reply-To: <CAAKRu_YOJ3VTKo4Z9vB2hGeTnwVWsL39gXH09vyBUQ7bGtDnKA@mail.gmail.com>
References: <CAAKRu_Yz9x0sejBa5ov_LJ5sMOSKM3AeKOFUg+fQpNqyMmxwRA@mail.gmail.com>
<CAAKRu_Y=QZ5iD7zt1AHcG3_G_iMR0w6ApGPgr8FKcDn-YLFvuQ@mail.gmail.com>
<CA+TgmoasgmY7mzZutGisD2=3y7BwwPUS=oNsQoORKRg1r69fEA@mail.gmail.com>
<CAAKRu_Y7X=0UAQa5b_2Z20z5+UPBtDbjazYD9228jmj-d9NpQA@mail.gmail.com>
<CA+Tgmob05A07mtzeUGwxQKU9KZSf4BhJU9CXgcy4Pe3ZHxZrcw@mail.gmail.com>
<CAAKRu_YX0NP_yhXvPnvDRjVxxprsRBM-_MZzAJskfMydMQ=ETA@mail.gmail.com>
<CA+TgmoZef8XqRujP1NN=wJdV4SxOtu7rxRozsyAtaEvuVMZhEw@mail.gmail.com>
<CAAKRu_YxD3UC3BXxS55jPjBC_yj_vn3FVoLvBMwQuHXGDXacGg@mail.gmail.com>
<CA+TgmobYY2URHKBMh1NHo1zF3Z28TiS_+0aSyRYyBfvauCPZzA@mail.gmail.com>
<CAAKRu_YOJ3VTKo4Z9vB2hGeTnwVWsL39gXH09vyBUQ7bGtDnKA@mail.gmail.com>
Hi,
On 2025-09-17 20:10:07 -0400, Melanie Plageman wrote:
> 0001 is RFC but waiting on one other reviewer
> From cacff6c95e38d370b87148bc48cf6ac5f086ed07 Mon Sep 17 00:00:00 2001
> From: Melanie Plageman <[email protected]>
> Date: Tue, 17 Jun 2025 17:22:10 -0400
> Subject: [PATCH v14 01/24] Eliminate COPY FREEZE use of XLOG_HEAP2_VISIBLE
> diff --git a/src/backend/access/heap/heapam_xlog.c b/src/backend/access/heap/heapam_xlog.c
> index cf843277938..faa7c561a8a 100644
> --- a/src/backend/access/heap/heapam_xlog.c
> +++ b/src/backend/access/heap/heapam_xlog.c
> @@ -551,6 +551,7 @@ heap_xlog_multi_insert(XLogReaderState *record)
> int i;
> bool isinit = (XLogRecGetInfo(record) & XLOG_HEAP_INIT_PAGE) != 0;
> XLogRedoAction action;
> + Buffer vmbuffer = InvalidBuffer;
>
> /*
> * Insertion doesn't overwrite MVCC data, so no conflict processing is
> @@ -571,11 +572,11 @@ heap_xlog_multi_insert(XLogReaderState *record)
> if (xlrec->flags & XLH_INSERT_ALL_VISIBLE_CLEARED)
> {
> Relation reln = CreateFakeRelcacheEntry(rlocator);
> - Buffer vmbuffer = InvalidBuffer;
>
> visibilitymap_pin(reln, blkno, &vmbuffer);
> visibilitymap_clear(reln, blkno, vmbuffer, VISIBILITYMAP_VALID_BITS);
> ReleaseBuffer(vmbuffer);
> + vmbuffer = InvalidBuffer;
> FreeFakeRelcacheEntry(reln);
> }
>
> @@ -662,6 +663,57 @@ heap_xlog_multi_insert(XLogReaderState *record)
> if (BufferIsValid(buffer))
> UnlockReleaseBuffer(buffer);
>
> + buffer = InvalidBuffer;
> +
> + /*
> + * Now read and update the VM block.
> + *
> + * Note that the heap relation may have been dropped or truncated, leading
> + * us to skip updating the heap block due to the LSN interlock.
I don't fully understand this - how does dropping/truncating the relation lead
to skipping due to the LSN interlock?
> + * even in that case, it's still safe to update the visibility map. Any
> + * WAL record that clears the visibility map bit does so before checking
> + * the page LSN, so any bits that need to be cleared will still be
> + * cleared.
> + *
> + * Note that the lock on the heap page was dropped above. In normal
> + * operation this would never be safe because a concurrent query could
> + * modify the heap page and clear PD_ALL_VISIBLE -- violating the
> + * invariant that PD_ALL_VISIBLE must be set if the corresponding bit in
> + * the VM is set.
> + *
> + * In recovery, we expect no other writers, so writing to the VM page
> + * without holding a lock on the heap page is considered safe enough. It
> + * is done this way when replaying xl_heap_visible records (see
> + * heap_xlog_visible()).
> + */
> + if (xlrec->flags & XLH_INSERT_ALL_FROZEN_SET &&
> + XLogReadBufferForRedoExtended(record, 1, RBM_ZERO_ON_ERROR, false,
> + &vmbuffer) == BLK_NEEDS_REDO)
> + {
Why are we using RBM_ZERO_ON_ERROR here? I know it's copied from
heap_xlog_visible(), but I don't immediately understand (or remember) why we
do so there either?
> + Page vmpage = BufferGetPage(vmbuffer);
> + Relation reln = CreateFakeRelcacheEntry(rlocator);
Hm. Do we really need to continue doing this ugly fake relcache stuff? I'd
really like to eventually get rid of that and given that the new "code shape"
delegates a lot more responsibility to the redo routines, they should have a
fairly easy time not needing a fake relcache? Afaict the relation already is
not used outside of debugging paths?
> + /* initialize the page if it was read as zeros */
> + if (PageIsNew(vmpage))
> + PageInit(vmpage, BLCKSZ, 0);
> +
> + visibilitymap_set_vmbits(reln, blkno,
> + vmbuffer,
> + VISIBILITYMAP_ALL_VISIBLE |
> + VISIBILITYMAP_ALL_FROZEN);
> +
> + /*
> + * It is not possible that the VM was already set for this heap page,
> + * so the vmbuffer must have been modified and marked dirty.
> + */
I assume that's because we a) checked the LSN interlock b) are replaying
something that needed to newly set the bit?
Except for the above comments, this looks pretty good to me.
Seems 0002 should just be applied...
Re 0003: I wonder if it's getting to the point that a struct should be used as
the argument.
Greetings,
Andres Freund
view thread (143+ messages) latest in thread
reply
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: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: eliminate xl_heap_visible to reduce WAL (and eventually set VM on-access)
In-Reply-To: <yn4zp35kkdsjx6wf47zcfmxgexxt4h2og47pvnw2x5ifyrs3qc@7uw6jyyxuyf7>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox