| From: | David Geier <geidav(dot)pg(at)gmail(dot)com> |
|---|---|
| To: | KAZAR Ayoub <ma_kazar(at)esi(dot)dz> |
| Cc: | Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, "tomas(at)vondra(dot)me" <tomas(at)vondra(dot)me> |
| Subject: | Re: Add pg_stat_vfdcache view for VFD cache statistics |
| Date: | 2026-03-31 14:54:16 |
| Message-ID: | 8895dcb7-015c-4f25-9c14-0c44cbc8c0fa@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi!
On 29.03.2026 21:46, KAZAR Ayoub wrote:
>>>>> I've looked at struct vfd and some simple changes to the struct would
>>>>> already cut memory consumption in half. I can look into that.
>>>>>
>>>>> Thoughts?
>>>>
>>>> Looking forward to this.
>>>
>>> I try to come up with something the next days.
>>>> What also bothers me in that space is if a backend allocates 100K
>>> entries
>>>> in VFD cache, that cache is never shrank ever again,
>>>> the cache only grows (if it needs more than its lifetime maximum) until
>>> the
>>>> backend dies, although this is useful as entries are reused if free
>>> instead
>>>> of
>>>> allocating entries, whether a spike in files openings effects a long
>>> living
>>>> backend to keep holding a useless amount of
>>>> cache size it will need in the future, i don't imagine this to be common
>>>> though, what do you think about this issue from your experience ?
>>>
>>> Currently the cache is directly mapped by the VFD index. That means we
>>> could only resize down to the maximum used VFD index.
>>>
>>> Being able to resize independently of the maximum VFD index would
>>> require changing to a hash map like simplehash.h. I can take a look how
>>> invasive such a change would be.
That would actually be doable without too much code churn. It would,
however, add some more overhead to each cache entry: 1 byte for the
simplehash.h status and a 4 byte for the hash, if we want to avoid
rehashing on each access. Probably we can get away without storing the
hash. We would then have to monitor the cache size and recreate the hash
table when the size has shrunk by enough.
Another alternative is pallocing vfd entries so that we can freely move
them around in the vfd cache array. That would mean an ABI change (File
would be an 8-byte pointer instead of a 4-byte integer) but give us much
more flexibility for possible improvements.
For example, apart from compacting without hash map, this would allow us
to allocate variable amounts of memory per entry to, e.g.
- store the file name inline of the struct as variable length array
(instead of a pstrdup() pointer) and
- depending on type allocate more or less memory. This is because
non-temporary files don't use the ResOwner and the file size.
>> I've implemented the recommended global stats view on vfd cache, the
>> implementation should be also straightforward as it follows the same
>> cumulative shared statistics infrastructure like pgstat_bgwriter and others
>> do.
>>
>> Attached is v2 patch also contains what David suggested for global cache
>> size and entries in the view.
I'll review the patch the next days. On quick inspection I saw that you
compute the size of a struct vfd by using sizeof(). That works, except
for the filename which is stored as a pointer to a pstrdup() piece of
memory. I guess you can just say:
sizeof(VfdCache[i]) + GetMemoryChunkSpace(VfdCache[i].fileName)
--
David Geier
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Aleksander Alekseev | 2026-03-31 15:18:33 | [PATCH] Add tests for src/backend/nodes/extensible.c |
| Previous Message | Daniil Davydov | 2026-03-31 14:49:59 | Re: Get rid of redundant StringInfo accumulation |