Re: Add pg_stat_vfdcache view for VFD cache statistics

From: Ayoub Kazar <kazarayoub2004(at)gmail(dot)com>
To: solai v <solai(dot)cdac(at)gmail(dot)com>
Cc: David Geier <geidav(dot)pg(at)gmail(dot)com>, KAZAR Ayoub <ma_kazar(at)esi(dot)dz>, Tomas Vondra <tomas(at)vondra(dot)me>, 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-09-04 01:20:12
Message-ID: CADu+CpQeaXJBCwnAZWgsLKt=ew3GwZ8XDXApesAS6dp01WR3oQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello again,

On Wed, Aug 26, 2026 at 3:03 PM Ayoub Kazar <kazarayoub2004(at)gmail(dot)com>
wrote:

> Hello Solai,
>
> On Mon, Aug 24, 2026 at 8:31 AM solai v <solai(dot)cdac(at)gmail(dot)com> wrote:
>
>>
>> Thank you for the updated patches. I have completed the review and
>> testing of the v5 patch series for introducing the pg_stat_vfdcache
>> view and VFD cache footprint metrics. I tested the patch on PostgreSQL
>> 20devel with the debug and assertion-enabled build and verified the
>> following:
>>
>> 1. Successfully built and ran the patched PostgreSQL tree.
>> 2. Verified the newly introduced pg_stat_vfdcache view and its
>> reported statistics.
>> 3. Tested VFD hit/miss accounting through repeated table accesses.
>> 4. Tested VFD cache behavior using a partitioned table with 1500
>> partitions and 1.5 million rows.
>> 5. Tested VFD pressure with a reduced max_files_per_process setting.
>> 6. Verified the behavior of: hits, misses, open_entries,
>> allocated_entries, cache_bytes, max_open_fds, and hit_ratio.
>> 7. Tested repeated scans and observed the corresponding changes in VFD
>> cache statistics.
>> 8. Tested statistics reset using pg_stat_reset_shared('vfdcache').
>> 9. Verified the behavior across multiple backend sessions.
>> 10. Tested the statistics behavior across server restart.
>> 11. Inspected the implementation of VFD cache footprint accounting in
>> fd.c, including VFD allocation, reuse, filename allocation/freeing,
>> and error handling.
>>
> Thanks for the thorough review.
>
>>
>> And also reviewed the statistics flush mechanism and confirmed that
>> VFD hit/miss counters are maintained locally and flushed through the
>> existing fixed-statistics infrastructure rather than taking a
>> shared-memory lock on every VFD access.
>> The main point I noticed is related to regression test coverage. The
>> current regression test verifies that pg_stat_reset_shared('vfdcache')
>> updates the stats_reset timestamp, but it does not directly exercise
>> and verify the newly introduced VFD statistics such as hits, misses,
>> open_entries, allocated_entries, and cache_bytes. I suggest adding a
>> focused regression test covering the actual VFD activity and the newly
>> exposed statistics, while avoiding assertions on fragile exact counter
>> values where appropriate.
>> Apart from these, I did not observe any functional issues during my
>> testing and the patch looks good to me.
>
>
> I rebased the patch (attached).
> I agree that the patch needs some activity tests.
>
> For hits & missed, i find a before/after test following the `pg_stat_wal`
> pattern is good:
>
> SELECT hits + misses AS vfd_accesses_before FROM pg_stat_vfdcache \gset
> CREATE TEMP TABLE vfd_activity_test (i int);
> INSERT INTO vfd_activity_test SELECT generate_series(1, 100);
> SELECT count(*) FROM vfd_activity_test;
> SELECT pg_stat_force_next_flush();
> SELECT hits + misses > :vfd_accesses_before FROM pg_stat_vfdcache;
> DROP TABLE vfd_activity_test;
>
> For open_entries, allocated_entries, cache_bytes delta tests are not
> reliable, open_entries can decrease between activity and measurement as
> files are closed. `allocated_entries` is non-decreasing within a single
> backend's lifetime, but the view sums across all backends ; any other
> activity's effects (like vacuum) might mislead the test a bit.
>
> For the latter counters, we can do some asserts like:
>
> SELECT open_entries <= allocated_entries AS footprint_invariant
> FROM pg_stat_vfdcache;
>
> This for example means we never report more open FDs than allocated slots,
> which should always be true (and perhaps other tests similar to this).
>
> Do you agree with this, or do you have other recommendations? I will
> proceed with v6 once we have alignment on this.
>
Attached is v6 patch that adds a test for hits and misses activity. As
mentioned before, I found this to be the only meaningful (and possible)
test.

Regards,
Ayoub

Attachment Content-Type Size
v6-0002-Add-VFD-cache-footprint-metrics-to-pg_stat_vfdcache.patch application/x-patch 17.6 KB
v6-0001-Add-pg_stat_vfdcache-view-for-VFD-cache-statistics.patch application/x-patch 25.7 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2026-09-04 01:20:18 Re: Rename of varatt_external to varatt_external_oid
Previous Message Peter Geoghegan 2026-09-04 01:07:44 Re: index prefetching