On 03.04.2026 15:53, KAZAR Ayoub wrote: >> - 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.
An alternative to including it in the view would be using a GUC of type PGC_INTERNAL. That seems more inline with how we expose other PostgreSQL internal read-only variables that don't change.
Or is there an advantage to including max_safe_fds in the view?
There's no significant advantage other than seeing all info related to vfdcache together in one view
Also in fd.h i just remembered:
/* * This is private to fd.c, but exported for save/restore_backend_variables() */ extern PGDLLIMPORT int max_safe_fds;
I don't think it's intended to be exported for reads like i'm doing (which can be fine?), or maybe update this comment about its export.
>> 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 number of used entries already exists, see nfile in fd.c.
Would one want the number of all entries (i.e SizeVfdCache see fd.c) or the number of used entries (i.e entries with fds in use, which is nfile) ? I thought of the first, that's what 0002 patch contains for the moment.
Including the total cache size would also be virtually free if we don't iterate over all VFDs each time, but update the size as we go. That would have to happen when resizing the cache and when populating / freeing a cache entry because extra memory is allocated / freed for Vfd::fileName.
Is it a big deal if we miss some bytes of filename globally ?
I'm happy to code this up if there's agreement that it's sensible to include it, in the current version of the patch or a follow-up patch.
Beyond that:
While looking through the code I saw a mistake (repetition of "that") in a comment in existing code. Maybe you want to fix that as well right away?
Noted.
/* * For variable-numbered stats: flush pending stats. Required if pending * data is used. See flush_static_cb when dealing with stats data that * that cannot use PgStat_EntryRef->pending. */ bool (*flush_pending_cb) (PgStat_EntryRef *sr, bool nowait);
The indentation of the type at the end of the following two structs is inconsistent with the rest of the files.
Fixed.
That's pg_indent doing me dirty, although i know it's wrong i didn't understand why it kept indenting like this, only in those two structs.
