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.
--
David Geier
