On Wed, Sep 9, 2026 at 9:22 AM David Geier <[email protected]> wrote:
> >>> 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. > > Yes. You can then make it even simpler and get rid of newVfdCache via > > VfdCache = repalloc_array(VfdCache, Vfd, newCacheSize); > > > #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? > > Yes. > > v7-0001: looks good to me. > > v7-0003: I'm wondering if we still want cache_bytes in pg_stat_vfdcache, > now where it's exposed via pg_backend_memory_contexts. It seems to me > that other stats functionality also doesn't expose memory info that is > accessible via pg_backend_memory_contexts. But I'm not completely sure > what's best here. > pg_backend_memory_contexts is local to current backend session, so AFAIK there's no other way of getting cluster-wide vfd cache memory usage. Therefore we need cache_bytes for this? Regards, Ayoub
