Re: pg_buffercache: Add per-relation summary stats

From: Lukas Fittl <lukas(at)fittl(dot)com>
To: Tomas Vondra <tomas(at)vondra(dot)me>
Cc: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Paul A Jungwirth <pj(at)illuminatedcomputing(dot)com>, Khoa Nguyen <khoaduynguyen(at)gmail(dot)com>
Subject: Re: pg_buffercache: Add per-relation summary stats
Date: 2026-03-28 19:38:04
Message-ID: CAP53PkzP=MEXFvCgEHZbZqxm5C69L5ig3c4qdPxU82=+H8C2iw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Tomas,

Thanks for reviewing - just a quick response on your code review
comments specifically:

On Fri, Mar 27, 2026 at 3:58 PM Tomas Vondra <tomas(at)vondra(dot)me> wrote:
> I gave it a try on an azure VM with 32GB shared buffers, to make it a
> bit more realistic, and my timings are 10ms vs. 700ms. But I also wonder
> if the original timings really were from a cluster with 128MB, because
> for me that shows 0.3ms vs. 3ms (so an order of magnitude faster than
> what was reported). But I suppose that's also hw specific.

Yeah, those initial numbers were from my Apple Silicon M3 ARM laptop
without any special configuration, just for reference.

> A couple minor comments about the code:
>
> 1) Isn't this check unnecessary? All entries should have buffers > 0.
>
> if (entry->buffers == 0)
> continue;

Yeah, good point, that is there to protect the division for the avg
usage count, but I agree in practice this shouldn't be reached. I
could make it an assert, just in case.

> 2) Shouldn't this check BM_TAG_VALID too? Or is BM_VALID enough to look
> at the bufHdr->tag?
>
> /* Skip unused/invalid buffers */
> if (!(buf_state & BM_VALID))
> continue;
>

Good point, I think that makes sense to check BM_TAG_VALID here as well.

FWIW, the function as-is does not lock the buffer header with
LockBufHdr (intentionally to lower overhead), which means we can read
a stale relation reference. I think that's okay for aggregate level /
monitoring type information, but just want to call it out in this
context.

> 3) I think "buffers" argument should be renamed to "buffers_unused" for
> consistency with pg_buffercache_summary.

I assume you meant "buffers_used" instead of "buffers_unused" -
assuming yes, that makes sense for consistency.

---

I'll hold off on posting a new version for now, since I think we'd
have to figure out a solution to the work_mem question at the very
least, and it sounds like right now its also a toss-up in terms of
overall interest to get this committed.

Thanks,
Lukas

--
Lukas Fittl

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2026-03-28 19:46:01 Re: astreamer fixes
Previous Message Mihail Nikalayeu 2026-03-28 19:17:00 Re: Revisiting {CREATE INDEX, REINDEX} CONCURRENTLY improvements