Re: Add pg_stat_vfdcache view for VFD cache statistics

Hello,
Thanks for the review!
On Tue, Mar 31, 2026 at 8:27 PM Tomas Vondra <tomas@vondra.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