| From: | KAZAR Ayoub <ma_kazar(at)esi(dot)dz> |
|---|---|
| To: | Tomas Vondra <tomas(at)vondra(dot)me> |
| Cc: | David Geier <geidav(dot)pg(at)gmail(dot)com>, Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Add pg_stat_vfdcache view for VFD cache statistics |
| Date: | 2026-04-03 13:53:47 |
| Message-ID: | CA+K2RumSp-kTw_YHXs_qN_RLt6cWfFR=LMq9coLgu8eyGydpHQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hello,
Thanks for the review!
On Tue, Mar 31, 2026 at 8:27 PM Tomas Vondra <tomas(at)vondra(dot)me> wrote:
> Hi,
>
> Thanks for working on this, I think having some stats about the vfd
> cache would be quite helpful. I took a quick look at the patch, and in
> general it goes in the right direction.
>
> Here's a couple comments / suggestions:
>
>
6) pgstat.h
>
> - Aren't evictions mostly the same as misses, at least after a while?
>
Correct, the time where they are not the same is pretty much meaningless
info, i removed it.
>
> - I think it would be useful to report how many file descriptors we
> are allowed to open (it's less than max_files_per_process, depending
> on the ulimits etc.)
>
Agree, This should be max_safe_fds calculated by postmaster, I added this
but let me know if its acceptable to export max_safe_fds in the way I did.
>
> - I know io_uring can consume quite a few descriptors, and it can cause
> issues, I wonder if this would make it easier to observe
>
>
> I also suggest to split the patch into smaller patches, to make it
> easier to review and evaluate. Not because of size - the patch is fairly
> small. But it's better to not mix multiple features with different
> cost/benefit trade offs, because then it's possible to evaluate them
> separately. Maybe even commit the first part and continue discussion
> about the following one(s).
>
> This patch seems to mix two different types of stats - global stats of
> the vfd cache, and then also per-backend stats. Those seems like very
> different things, both in terms of overhead and benefits.
>
> The global cache stats is going to be virtually free (at least the
> hits/misses, I'm not sure about the number of entries and bytes), and
> it's obviously useful for tuning the max_files_per_process GUC. I'd even
> contemplate getting this into PG19, maybe.
>
> The per-backend stats seem like a much harder sell to me, but I can be
> convinced. Maybe it's not an issue in terms of overhead, maybe the stats
> we get from that are worth it. Not sure. But I'd keep it in a separate
> 0002 patch, on top of 0001 with just the "global" stats.
>
>
>
> regards
>
> --
> Tomas Vondra
>
> I fixed style related issues and followed your suggestions on splitting
the patch to do global stats first then the per-backend stats of cache size
and entries count reporting, attached is v3-0001 that does just the global
stats counting.
When we make sure this is correct i'll proceed with the per-backend stats
patch.
Regards,
Ayoub
| Attachment | Content-Type | Size |
|---|---|---|
| v3-0001-Add-pg_stat_vfdcache-view-for-VFD-cache-statistics.patch | text/x-patch | 24.1 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Nathan Bossart | 2026-04-03 13:54:36 | Re: vectorized CRC on ARM64 |
| Previous Message | Daniil Davydov | 2026-04-03 13:45:39 | Re: POC: Parallel processing of indexes in autovacuum |