Re: Add pg_stat_vfdcache view for VFD cache statistics

From: Ayoub Kazar <kazarayoub2004(at)gmail(dot)com>
To: David Geier <geidav(dot)pg(at)gmail(dot)com>
Cc: 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-09 02:37:54
Message-ID: CADu+CpT-XR8MGhhHS4TPztm58TwVNEZ1jvO9S6_xkLgTL3PB4g@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Sep 8, 2026 at 6:33 PM David Geier <geidav(dot)pg(at)gmail(dot)com> wrote:

> >>>> However, I'm wondering if the better approach wouldn't be to change
> fd.c
> >>>> to use a long-lived memory context. Then all bookkeeping would happen
> >>>> automatically and the memory size could simply be reported via
> existing
> >>>> memory context stats infrastructure.
> >>>>
> >>>> Not entirely sure though if there's some roadblock when switching to a
> >>>> memory context.
> >>> I don't see any issue with this either. However, the only benefit we
> >> would
> >>> gain is using existing infrastructure but only for backend vfd cache
> >> memory
> >>> (i.e cache_bytes).
> >>> Everything else stays the same (counters, cluster-wide memory);
> >> therefore,
> >>> if there's no other benefit to replacing with memory contexts, maybe
> it's
> >>> not worth it.
> >>
> >> The biggest benefit in my view is consistency with the rest of
> PostgreSQL.
> >> That is from a usage point of view as well as from a coding point of
> view.
> >> If you want, I can give that a try and share a patch with you if
> >> successful.
> >>
> > Yes of course, I’d be happy to take a look.
> Attached is the patch. It's pretty small and passes regress tests.
>
Nice one.
Few comments on it:
#1:
+ oldcontext = MemoryContextSwitchTo(VfdCxt);
+ newVfdCache = repalloc_array(VfdCache, Vfd, newCacheSize);
+ MemoryContextSwitchTo(oldcontext);

Can't we just do this?
+ newVfdCache = repalloc_array(VfdCache, Vfd, newCacheSize);

because repalloc doesn't need CurrentMemoryContext.

#2:
+ newDescs = MemoryContextAllocExtended(VfdCxt,
+ newMax * sizeof(AllocateDesc), MCXT_ALLOC_NO_OOM);
if (newDescs == NULL)
return false;
+ memcpy(newDescs, allocatedDescs, maxAllocatedDescs *
sizeof(AllocateDesc));
+ pfree(allocatedDescs);

We can also just replace it with:
+ newDescs = repalloc_array_extended(allocatedDescs, AllocateDesc,
+ newMax, MCXT_ALLOC_NO_OOM);
Correct?

Attached are v7 patches containing your patch for memory contexts with
these small suggestions and pg_stat_vfdcache patches like before.

Regards,
Ayoub

Attachment Content-Type Size
v7-0002-Add-pg_stat_vfdcache-view-for-VFD-cache-statistics.patch text/x-patch 25.7 KB
v7-0001-Vfd-cache-uses-memory-context-instead-of-malloc.patch text/x-patch 4.7 KB
v7-0003-Add-VFD-cache-footprint-metrics-to-pg_stat_vfdcache.patch text/x-patch 17.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message cca5507 2026-09-09 02:49:19 Re: [PATCH] Report userid to PgBackendStatus for standalone backends
Previous Message Haibo Yan 2026-09-09 02:18:13 Re: Global temporary tables