Re: Proposal to add page headers to SLRU pages

From: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: "Li, Yong" <yoli(at)ebay(dot)com>, Aleksander Alekseev <aleksander(at)timescale(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "Bagga, Rishu" <bagrishu(at)amazon(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, "Debnath, Shawn" <sdn(at)ebay(dot)com>, Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, "Shyrabokau, Anton" <antons(at)ebay(dot)com>
Subject: Re: Proposal to add page headers to SLRU pages
Date: 2024-03-08 12:58:29
Message-ID: 202403081258.npyot4wpfzgf@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2024-Mar-08, Stephen Frost wrote:

> Thanks for testing! That strikes me as perfectly reasonable and seems
> unlikely that we'd get much benefit from parallelizing it, so I'd say it
> makes sense to keep this code simple.

Okay, agreed, that amount of time sounds reasonable to me too; but I
don't want to be responsible for this at least for pg17. If some other
committer wants to take it, be my guest. However, I think this is
mostly a foundation for building other things on top, so committing
during the last commitfest is perhaps not very useful.

Another aspect of this patch is the removal of the LSN groups. There's
an explanation of the LSN groups in src/backend/access/transam/README,
and while this patch removes the LSN group feature, it doesn't update
that text. That's already a problem which needs fixed, but the text
says

: In fact, we store more than one LSN for each clog page. This relates to
: the way we set transaction status hint bits during visibility tests.
: We must not set a transaction-committed hint bit on a relation page and
: have that record make it to disk prior to the WAL record of the commit.
: Since visibility tests are normally made while holding buffer share locks,
: we do not have the option of changing the page's LSN to guarantee WAL
: synchronization. Instead, we defer the setting of the hint bit if we have
: not yet flushed WAL as far as the LSN associated with the transaction.
: This requires tracking the LSN of each unflushed async commit.
: It is convenient to associate this data with clog buffers: because we
: will flush WAL before writing a clog page, we know that we do not need
: to remember a transaction's LSN longer than the clog page holding its
: commit status remains in memory. However, the naive approach of storing
: an LSN for each clog position is unattractive: the LSNs are 32x bigger
: than the two-bit commit status fields, and so we'd need 256K of
: additional shared memory for each 8K clog buffer page. We choose
: instead to store a smaller number of LSNs per page, where each LSN is
: the highest LSN associated with any transaction commit in a contiguous
: range of transaction IDs on that page. This saves storage at the price
: of some possibly-unnecessary delay in setting transaction hint bits.

In the new code we effectively store only one LSN per page, which I
understand is strictly worse. Maybe the idea of doing away with these
LSN groups should be reconsidered ... unless I completely misunderstand
the whole thing.

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2024-03-08 13:36:48 Re: Failures in constraints regression test, "read only 0 of 8192 bytes"
Previous Message Tomas Vondra 2024-03-08 12:21:09 Re: Failures in constraints regression test, "read only 0 of 8192 bytes"