Re: pg_buffercache: Add per-relation summary stats

From: Lukas Fittl <lukas(at)fittl(dot)com>
To: Haibo Yan <tristan(dot)yim(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: pg_buffercache: Add per-relation summary stats
Date: 2026-03-25 06:52:13
Message-ID: CAP53Pkx3TkVWnMMiST3CVhu0uPqo9tdVn0s=_Our=EHtgs39nQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Haibo,

Thanks for your review!

On Mon, Mar 16, 2026 at 9:21 PM Haibo Yan <tristan(dot)yim(at)gmail(dot)com> wrote:
> Could this use RelFileLocator plus ForkNumber instead of open-coding BufferRelStatsKey? That seems closer to existing PostgreSQL abstractions for physical relation identity.

Yes, that was noted by other reviewers as well, and makes sense.

> I wonder whether pg_buffercache_relation_stats() is the best name here. The function is really aggregating by relation file identity plus fork, and it is producing a summary of the current buffer contents rather than what many readers might assume from “relation stats”. Would something with summary be clearer than stats?

Per the most recent feedback, I'll rename this to
"pg_buffercache_relations" for now.

> Why are OUT relforknumber and OUT relfilenode exposed as int2 and oid respectively? Internally these are represented as ForkNumber and RelFileNumber, so I wonder whether the SQL interface should reflect that more clearly, or at least whether the current choice should be explained.

This is consistent with how pg_buffercache_pages represents them - I
think those are the correct mappings of the int ForkNumber (which we
know to be small in practice) and RelFileNumber is a typedef of Oid.

> The comment says, “Hash key for pg_buffercache_relation_stats — groups by relation identity”, but that seems imprecise. It is really grouping by relfilenode plus fork, i.e. physical relation-file identity rather than relation identity in a more logical sense.

Good point. I'll adapt this to "groups by relation file" for now.

> Is PARALLEL SAFE actually desirable here, as opposed to merely technically safe? A parallel query could cause multiple workers to perform full shared-buffer scans independently, which does not seem obviously desirable for this kind of diagnostic function.

I see your point, but I don't think a parallel plan would happen in
practice when just the function is being queried. Since other
pg_buffercache functions are also PARALLEL SAFE, I'll keep this as is
for now - if we want to adjust it we should be consistent I think.

Thanks,
Lukas

--
Lukas Fittl

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Lukas Fittl 2026-03-25 06:54:18 Re: pg_buffercache: Add per-relation summary stats
Previous Message Postgress Cybrosys 2026-03-25 06:51:35 Fix incorrect false positive rate formatting in create_and_test_bloom()