agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: David Geier <geidav.pg@gmail.com>
To: Ayoub Kazar <kazarayoub2004@gmail.com>
Cc: KAZAR Ayoub <ma_kazar@esi.dz>
Cc: Tomas Vondra <tomas@vondra.me>
Cc: Jakub Wartak <jakub.wartak@enterprisedb.com>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Add pg_stat_vfdcache view for VFD cache statistics
Date: Tue, 8 Sep 2026 18:33:18 +0200
Message-ID: <b009d07a-e271-4eba-804c-d6a6b86dcc64@gmail.com> (raw)
In-Reply-To: <CADu+CpSY57xpQsGr1CAESUHxGv_Y3_aABmkHC-Gh9GpEG3tpPw@mail.gmail.com>
References: <CA+K2RumP33Cpj--88E+rNADa8fzSBBiav=rvzyaMM=sYNaOkfA@mail.gmail.com>
<d305fd37-346b-4512-808e-5dd7968eb569@gmail.com>
<CA+K2RuncGCW55b-XUe8gJGwdMSOgTFKG-uSCgbVDZ+HewiT5EQ@mail.gmail.com>
<CA+K2RumDZ05pru3-YSZxe3Y==vO0hsahiweyvJw6QsjuGR5WsA@mail.gmail.com>
<42776281-3603-4161-b47d-d4ffd2029e8c@vondra.me>
<CA+K2RumSp-kTw_YHXs_qN_RLt6cWfFR=LMq9coLgu8eyGydpHQ@mail.gmail.com>
<02cfc5e7-e152-4d2d-8b4b-e899d9901ed5@gmail.com>
<CA+K2Ruk55=2fBftAMg3Y=--+6uSNF05UVmu8w8S8FdJ+ektQcg@mail.gmail.com>
<c94c385a-cdf6-44c5-9768-b8c47ab75868@gmail.com>
<CADu+CpTwQXRdoKVSnoVRLp5m0UbA_cAU6s+_Og5=hCwKp0JR2Q@mail.gmail.com>
<52677fc8-f57d-48c2-9415-6beb3ea6fa01@gmail.com>
<CADu+CpSyR3fnHGwTRAbULyBtxPZbkR2Y41Dd36ombAUwG4TT3g@mail.gmail.com>
<11d0df16-94b8-4223-87f7-2e84084a28af@gmail.com>
<CADu+CpT=ZYM2+zP41Dt8PRo=pmoY7TfN5Yc-Npp2T9tPOAvzbA@mail.gmail.com>
<3b394e5f-ac4e-446c-b422-92be2b984722@gmail.com>
<CADu+CpSY57xpQsGr1CAESUHxGv_Y3_aABmkHC-Gh9GpEG3tpPw@mail.gmail.com>
>>>> 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.
--
David Geier
From fd53e2b8c46581301c000b17ae4f67ba425c41f6 Mon Sep 17 00:00:00 2001
From: David Geier <geidav.pg@gmail.com>
Date: Tue, 8 Sep 2026 18:22:33 +0200
Subject: [PATCH v1] Vfd cache uses memory context instead of malloc
---
src/backend/storage/file/fd.c | 57 +++++++++++++++++------------------
1 file changed, 27 insertions(+), 30 deletions(-)
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 190c9974494..5fae65148a4 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -99,6 +99,7 @@
#include "storage/ipc.h"
#include "utils/guc.h"
#include "utils/guc_hooks.h"
+#include "utils/memutils.h"
#include "utils/resowner.h"
#include "utils/varlena.h"
#include "utils/wait_event.h"
@@ -217,6 +218,7 @@ typedef struct vfd
* needed. 'File' values are indexes into this array.
* Note that VfdCache[0] is not a usable VFD, just a list header.
*/
+static MemoryContext VfdCxt;
static Vfd *VfdCache;
static Size SizeVfdCache = 0;
@@ -905,12 +907,13 @@ InitFileAccess(void)
{
Assert(SizeVfdCache == 0); /* call me only once */
+ if (VfdCxt == NULL)
+ VfdCxt = AllocSetContextCreate(TopMemoryContext,
+ "Vfd cache context",
+ ALLOCSET_DEFAULT_SIZES);
+
/* initialize cache header entry */
- VfdCache = (Vfd *) malloc(sizeof(Vfd));
- if (VfdCache == NULL)
- ereport(FATAL,
- (errcode(ERRCODE_OUT_OF_MEMORY),
- errmsg("out of memory")));
+ VfdCache = MemoryContextAlloc(VfdCxt, sizeof(Vfd));
MemSet(&(VfdCache[0]), 0, sizeof(Vfd));
VfdCache->fd = VFD_CLOSED;
@@ -1403,6 +1406,7 @@ AllocateVfd(void)
{
Index i;
File file;
+ MemoryContext oldcontext;
DO_DB(elog(LOG, "AllocateVfd. Size %zu", SizeVfdCache));
@@ -1422,13 +1426,11 @@ AllocateVfd(void)
newCacheSize = 32;
/*
- * Be careful not to clobber VfdCache ptr if realloc fails.
+ * Be careful not to clobber VfdCache ptr if allocation fails.
*/
- newVfdCache = (Vfd *) realloc(VfdCache, sizeof(Vfd) * newCacheSize);
- if (newVfdCache == NULL)
- ereport(ERROR,
- (errcode(ERRCODE_OUT_OF_MEMORY),
- errmsg("out of memory")));
+ oldcontext = MemoryContextSwitchTo(VfdCxt);
+ newVfdCache = repalloc_array(VfdCache, Vfd, newCacheSize);
+ MemoryContextSwitchTo(oldcontext);
VfdCache = newVfdCache;
/*
@@ -1466,7 +1468,7 @@ FreeVfd(File file)
if (vfdP->fileName != NULL)
{
- free(vfdP->fileName);
+ pfree(vfdP->fileName);
vfdP->fileName = NULL;
}
vfdP->fdstate = 0x0;
@@ -1582,14 +1584,7 @@ PathNameOpenFilePerm(const char *fileName, int fileFlags, mode_t fileMode)
DO_DB(elog(LOG, "PathNameOpenFilePerm: %s %x %o",
fileName, fileFlags, fileMode));
- /*
- * We need a malloc'd copy of the file name; fail cleanly if no room.
- */
- fnamecopy = strdup(fileName);
- if (fnamecopy == NULL)
- ereport(ERROR,
- (errcode(ERRCODE_OUT_OF_MEMORY),
- errmsg("out of memory")));
+ fnamecopy = MemoryContextStrdup(VfdCxt, fileName);
file = AllocateVfd();
vfdP = &VfdCache[file];
@@ -1612,7 +1607,7 @@ PathNameOpenFilePerm(const char *fileName, int fileFlags, mode_t fileMode)
int save_errno = errno;
FreeVfd(file);
- free(fnamecopy);
+ pfree(fnamecopy);
errno = save_errno;
return -1;
}
@@ -2555,6 +2550,11 @@ reserveAllocatedDesc(void)
AllocateDesc *newDescs;
int newMax;
+ if (VfdCxt == NULL)
+ VfdCxt = AllocSetContextCreate(TopMemoryContext,
+ "Vfd cache context",
+ ALLOCSET_DEFAULT_SIZES);
+
/* Quick out if array already has a free slot. */
if (numAllocatedDescs < maxAllocatedDescs)
return true;
@@ -2568,12 +2568,8 @@ reserveAllocatedDesc(void)
if (allocatedDescs == NULL)
{
newMax = FD_MINFREE / 3;
- newDescs = (AllocateDesc *) malloc(newMax * sizeof(AllocateDesc));
- /* Out of memory already? Treat as fatal error. */
- if (newDescs == NULL)
- ereport(ERROR,
- (errcode(ERRCODE_OUT_OF_MEMORY),
- errmsg("out of memory")));
+ newDescs = MemoryContextAlloc(VfdCxt,
+ newMax * sizeof(AllocateDesc));
allocatedDescs = newDescs;
maxAllocatedDescs = newMax;
return true;
@@ -2593,11 +2589,12 @@ reserveAllocatedDesc(void)
newMax = max_safe_fds / 3;
if (newMax > maxAllocatedDescs)
{
- newDescs = (AllocateDesc *) realloc(allocatedDescs,
- newMax * sizeof(AllocateDesc));
- /* Treat out-of-memory as a non-fatal error. */
+ newDescs = MemoryContextAllocExtended(VfdCxt,
+ newMax * sizeof(AllocateDesc), MCXT_ALLOC_NO_OOM);
if (newDescs == NULL)
return false;
+ memcpy(newDescs, allocatedDescs, maxAllocatedDescs * sizeof(AllocateDesc));
+ pfree(allocatedDescs);
allocatedDescs = newDescs;
maxAllocatedDescs = newMax;
return true;
--
2.50.1 (Apple Git-155)
Attachments:
[text/plain] v1-0001-Vfd-cache-uses-memory-context-instead-of-malloc.patch (4.6K, ../b009d07a-e271-4eba-804c-d6a6b86dcc64@gmail.com/2-v1-0001-Vfd-cache-uses-memory-context-instead-of-malloc.patch)
download | inline diff:
From fd53e2b8c46581301c000b17ae4f67ba425c41f6 Mon Sep 17 00:00:00 2001
From: David Geier <geidav.pg@gmail.com>
Date: Tue, 8 Sep 2026 18:22:33 +0200
Subject: [PATCH v1] Vfd cache uses memory context instead of malloc
---
src/backend/storage/file/fd.c | 57 +++++++++++++++++------------------
1 file changed, 27 insertions(+), 30 deletions(-)
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 190c9974494..5fae65148a4 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -99,6 +99,7 @@
#include "storage/ipc.h"
#include "utils/guc.h"
#include "utils/guc_hooks.h"
+#include "utils/memutils.h"
#include "utils/resowner.h"
#include "utils/varlena.h"
#include "utils/wait_event.h"
@@ -217,6 +218,7 @@ typedef struct vfd
* needed. 'File' values are indexes into this array.
* Note that VfdCache[0] is not a usable VFD, just a list header.
*/
+static MemoryContext VfdCxt;
static Vfd *VfdCache;
static Size SizeVfdCache = 0;
@@ -905,12 +907,13 @@ InitFileAccess(void)
{
Assert(SizeVfdCache == 0); /* call me only once */
+ if (VfdCxt == NULL)
+ VfdCxt = AllocSetContextCreate(TopMemoryContext,
+ "Vfd cache context",
+ ALLOCSET_DEFAULT_SIZES);
+
/* initialize cache header entry */
- VfdCache = (Vfd *) malloc(sizeof(Vfd));
- if (VfdCache == NULL)
- ereport(FATAL,
- (errcode(ERRCODE_OUT_OF_MEMORY),
- errmsg("out of memory")));
+ VfdCache = MemoryContextAlloc(VfdCxt, sizeof(Vfd));
MemSet(&(VfdCache[0]), 0, sizeof(Vfd));
VfdCache->fd = VFD_CLOSED;
@@ -1403,6 +1406,7 @@ AllocateVfd(void)
{
Index i;
File file;
+ MemoryContext oldcontext;
DO_DB(elog(LOG, "AllocateVfd. Size %zu", SizeVfdCache));
@@ -1422,13 +1426,11 @@ AllocateVfd(void)
newCacheSize = 32;
/*
- * Be careful not to clobber VfdCache ptr if realloc fails.
+ * Be careful not to clobber VfdCache ptr if allocation fails.
*/
- newVfdCache = (Vfd *) realloc(VfdCache, sizeof(Vfd) * newCacheSize);
- if (newVfdCache == NULL)
- ereport(ERROR,
- (errcode(ERRCODE_OUT_OF_MEMORY),
- errmsg("out of memory")));
+ oldcontext = MemoryContextSwitchTo(VfdCxt);
+ newVfdCache = repalloc_array(VfdCache, Vfd, newCacheSize);
+ MemoryContextSwitchTo(oldcontext);
VfdCache = newVfdCache;
/*
@@ -1466,7 +1468,7 @@ FreeVfd(File file)
if (vfdP->fileName != NULL)
{
- free(vfdP->fileName);
+ pfree(vfdP->fileName);
vfdP->fileName = NULL;
}
vfdP->fdstate = 0x0;
@@ -1582,14 +1584,7 @@ PathNameOpenFilePerm(const char *fileName, int fileFlags, mode_t fileMode)
DO_DB(elog(LOG, "PathNameOpenFilePerm: %s %x %o",
fileName, fileFlags, fileMode));
- /*
- * We need a malloc'd copy of the file name; fail cleanly if no room.
- */
- fnamecopy = strdup(fileName);
- if (fnamecopy == NULL)
- ereport(ERROR,
- (errcode(ERRCODE_OUT_OF_MEMORY),
- errmsg("out of memory")));
+ fnamecopy = MemoryContextStrdup(VfdCxt, fileName);
file = AllocateVfd();
vfdP = &VfdCache[file];
@@ -1612,7 +1607,7 @@ PathNameOpenFilePerm(const char *fileName, int fileFlags, mode_t fileMode)
int save_errno = errno;
FreeVfd(file);
- free(fnamecopy);
+ pfree(fnamecopy);
errno = save_errno;
return -1;
}
@@ -2555,6 +2550,11 @@ reserveAllocatedDesc(void)
AllocateDesc *newDescs;
int newMax;
+ if (VfdCxt == NULL)
+ VfdCxt = AllocSetContextCreate(TopMemoryContext,
+ "Vfd cache context",
+ ALLOCSET_DEFAULT_SIZES);
+
/* Quick out if array already has a free slot. */
if (numAllocatedDescs < maxAllocatedDescs)
return true;
@@ -2568,12 +2568,8 @@ reserveAllocatedDesc(void)
if (allocatedDescs == NULL)
{
newMax = FD_MINFREE / 3;
- newDescs = (AllocateDesc *) malloc(newMax * sizeof(AllocateDesc));
- /* Out of memory already? Treat as fatal error. */
- if (newDescs == NULL)
- ereport(ERROR,
- (errcode(ERRCODE_OUT_OF_MEMORY),
- errmsg("out of memory")));
+ newDescs = MemoryContextAlloc(VfdCxt,
+ newMax * sizeof(AllocateDesc));
allocatedDescs = newDescs;
maxAllocatedDescs = newMax;
return true;
@@ -2593,11 +2589,12 @@ reserveAllocatedDesc(void)
newMax = max_safe_fds / 3;
if (newMax > maxAllocatedDescs)
{
- newDescs = (AllocateDesc *) realloc(allocatedDescs,
- newMax * sizeof(AllocateDesc));
- /* Treat out-of-memory as a non-fatal error. */
+ newDescs = MemoryContextAllocExtended(VfdCxt,
+ newMax * sizeof(AllocateDesc), MCXT_ALLOC_NO_OOM);
if (newDescs == NULL)
return false;
+ memcpy(newDescs, allocatedDescs, maxAllocatedDescs * sizeof(AllocateDesc));
+ pfree(allocatedDescs);
allocatedDescs = newDescs;
maxAllocatedDescs = newMax;
return true;
--
2.50.1 (Apple Git-155)
view thread (42+ messages) latest in thread
Message-ID: <b009d07a-e271-4eba-804c-d6a6b86dcc64@gmail.com>
Permalink: ../b009d07a-e271-4eba-804c-d6a6b86dcc64@gmail.com/
Also on: postgresql.org/message-id/b009d07a-e271-4eba-804c-d6a6b86dcc64@gmail.com
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: geidav.pg@gmail.com, kazarayoub2004@gmail.com, ma_kazar@esi.dz, tomas@vondra.me, jakub.wartak@enterprisedb.com
Subject: Re: Add pg_stat_vfdcache view for VFD cache statistics
In-Reply-To: <b009d07a-e271-4eba-804c-d6a6b86dcc64@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox