From 4280047a1f147a14ada5bc85fef3a755dcb9514d Mon Sep 17 00:00:00 2001 From: Bryan Green Date: Mon, 26 Oct 2026 11:03:35 -0500 Subject: [PATCH v1] Don't close relation files when the visibility map is extended When vm_extend() grows the visibility map it sends an smgr invalidation. Every backend that has the relation open then closes all of its files, and reopens each segment the next time it touches the table. VACUUM extends the map about once per 255MB of heap, and on-access pruning can extend it too. On a large table with many backends writing to it that is a lot of opens, and none of them are needed. The other backends only have to learn that the map got bigger. This assumes open() is cheap, and it isn't always. Under a filesystem encryption layer that hooks open(), it shows up as inserts stalling while VACUUM runs. Add an invalidation message that just resets the cached fork sizes, and send that from vm_extend() instead. Readers see new map pages when they do now. Dropping the invalidation entirely would leave a backend that doesn't prune the pages itself with a stale map size until VACUUM finishes. --- src/backend/access/heap/visibilitymap.c | 13 +++++----- src/backend/access/rmgrdesc/standbydesc.c | 3 +++ src/backend/storage/smgr/smgr.c | 27 ++++++++++++++++++-- src/backend/utils/cache/inval.c | 30 +++++++++++++++++++++++ src/include/storage/sinval.h | 8 ++++-- src/include/storage/smgr.h | 7 ++++-- src/include/utils/inval.h | 1 + 7 files changed, 77 insertions(+), 12 deletions(-) diff --git a/src/backend/access/heap/visibilitymap.c b/src/backend/access/heap/visibilitymap.c index fe5ce437e1..dd95a08709 100644 --- a/src/backend/access/heap/visibilitymap.c +++ b/src/backend/access/heap/visibilitymap.c @@ -652,13 +652,14 @@ vm_extend(Relation rel, BlockNumber vm_nblocks) RBM_ZERO_ON_ERROR); /* - * Send a shared-inval message to force other backends to close any smgr - * references they may have for this rel, which we are about to change. - * This is a useful optimization because it means that backends don't have - * to keep checking for creation or extension of the file, which happens - * infrequently. + * Send a shared-inval message to make other backends forget their cached + * fork sizes for this rel, including the map's. This is a useful + * optimization because it means that backends don't have to keep checking + * for creation or extension of the file, which happens infrequently. We + * don't use CacheInvalidateSmgr(), because that makes every backend that + * has the rel open close all of its files. */ - CacheInvalidateSmgr(RelationGetSmgr(rel)->smgr_rlocator); + CacheInvalidateSmgrNblocks(RelationGetSmgr(rel)->smgr_rlocator); return buf; } diff --git a/src/backend/access/rmgrdesc/standbydesc.c b/src/backend/access/rmgrdesc/standbydesc.c index 0a291354ae..0ec2bfcb06 100644 --- a/src/backend/access/rmgrdesc/standbydesc.c +++ b/src/backend/access/rmgrdesc/standbydesc.c @@ -128,6 +128,9 @@ standby_desc_invalidations(StringInfo buf, else if (msg->id == SHAREDINVALSMGR_ID) appendStringInfoString(buf, " smgr"); /* not expected, but print something anyway */ + else if (msg->id == SHAREDINVALSMGRNBLOCKS_ID) + appendStringInfoString(buf, " smgr nblocks"); + /* not expected, but print something anyway */ else if (msg->id == SHAREDINVALRELMAP_ID) appendStringInfo(buf, " relmap db %u", msg->rm.dbId); else if (msg->id == SHAREDINVALSNAPSHOT_ID) diff --git a/src/backend/storage/smgr/smgr.c b/src/backend/storage/smgr/smgr.c index 5391640d86..8443fa46ab 100644 --- a/src/backend/storage/smgr/smgr.c +++ b/src/backend/storage/smgr/smgr.c @@ -455,6 +455,28 @@ smgrreleaserellocator(RelFileLocatorBackend rlocator) smgrrelease(reln); } +/* + * smgrforgetnblocks() -- Forget the cached fork sizes for given + * RelFileLocator, if it's open. + */ +void +smgrforgetnblocks(RelFileLocatorBackend rlocator) +{ + SMgrRelation reln; + + if (SMgrRelationHash == NULL) + return; + + reln = (SMgrRelation) hash_search(SMgrRelationHash, + &rlocator, + HASH_FIND, NULL); + if (reln != NULL) + { + for (int forknum = 0; forknum <= MAX_FORKNUM; forknum++) + reln->smgr_cached_nblocks[forknum] = InvalidBlockNumber; + } +} + /* * smgrexists() -- Does the underlying file for a fork exist? */ @@ -848,8 +870,9 @@ smgrnblocks_cached(SMgrRelation reln, ForkNumber forknum) { /* * For now, this function uses cached values only in recovery due to lack - * of a shared invalidation mechanism for changes in file size. Code - * elsewhere reads smgr_cached_nblocks and copes with stale data. + * of a shared invalidation mechanism for changes in file size, other than + * for visibility map extension. Code elsewhere reads smgr_cached_nblocks + * and copes with stale data. */ if (InRecovery && reln->smgr_cached_nblocks[forknum] != InvalidBlockNumber) return reln->smgr_cached_nblocks[forknum]; diff --git a/src/backend/utils/cache/inval.c b/src/backend/utils/cache/inval.c index 81a5d433bc..720ece9aa7 100644 --- a/src/backend/utils/cache/inval.c +++ b/src/backend/utils/cache/inval.c @@ -873,6 +873,14 @@ LocalExecuteInvalidationMessage(SharedInvalidationMessage *msg) rlocator.backend = (msg->sm.backend_hi << 16) | (int) msg->sm.backend_lo; smgrreleaserellocator(rlocator); } + else if (msg->id == SHAREDINVALSMGRNBLOCKS_ID) + { + RelFileLocatorBackend rlocator; + + rlocator.locator = msg->sm.rlocator; + rlocator.backend = (msg->sm.backend_hi << 16) | (int) msg->sm.backend_lo; + smgrforgetnblocks(rlocator); + } else if (msg->id == SHAREDINVALRELMAP_ID) { /* We only care about our own database and shared catalogs */ @@ -1764,6 +1772,28 @@ CacheInvalidateSmgr(RelFileLocatorBackend rlocator) SendSharedInvalidMessages(&msg, 1); } +/* + * CacheInvalidateSmgrNblocks + * Make other backends forget their cached sizes of a physical relation's + * forks, without closing its files. + * + * Like CacheInvalidateSmgr, this is nontransactional and sent immediately. + */ +void +CacheInvalidateSmgrNblocks(RelFileLocatorBackend rlocator) +{ + SharedInvalidationMessage msg; + + msg.sm.id = SHAREDINVALSMGRNBLOCKS_ID; + msg.sm.backend_hi = rlocator.backend >> 16; + msg.sm.backend_lo = rlocator.backend & 0xffff; + msg.sm.rlocator = rlocator.locator; + /* check AddCatcacheInvalidationMessage() for an explanation */ + VALGRIND_MAKE_MEM_DEFINED(&msg, sizeof(msg)); + + SendSharedInvalidMessages(&msg, 1); +} + /* * CacheInvalidateRelmap * Register invalidation of the relation mapping for a database, diff --git a/src/include/storage/sinval.h b/src/include/storage/sinval.h index baf21dfe97..5a02f42704 100644 --- a/src/include/storage/sinval.h +++ b/src/include/storage/sinval.h @@ -28,6 +28,7 @@ * * invalidate the mapped-relation mapping for a given database * * invalidate any saved snapshot that might be used to scan a given relation * * invalidate a RelationSyncCache entry for a specific relation + * * forget the cached fork sizes of an smgr cache entry * More types could be added if needed. The message type is identified by * the first "int8" field of the message struct. Zero or positive means a * specific-catcache inval message (and also serves as the catcache ID field). @@ -54,8 +55,8 @@ * the new state. The generating backend also has to process them at abort, * to flush out any cache state it's loaded from no-longer-valid entries. * - * smgr and relation mapping invalidations are non-transactional: they are - * sent immediately when the underlying file change is made. + * smgr, smgr size, and relation mapping invalidations are non-transactional: + * they are sent immediately when the underlying file change is made. */ typedef struct @@ -94,6 +95,9 @@ typedef struct RelFileLocator rlocator; /* spcOid, dbOid, relNumber */ } SharedInvalSmgrMsg; +/* uses SharedInvalSmgrMsg too, but only resets cached fork sizes */ +#define SHAREDINVALSMGRNBLOCKS_ID (-7) + #define SHAREDINVALRELMAP_ID (-4) typedef struct diff --git a/src/include/storage/smgr.h b/src/include/storage/smgr.h index 09bd42fcf4..11b21dd325 100644 --- a/src/include/storage/smgr.h +++ b/src/include/storage/smgr.h @@ -40,8 +40,10 @@ typedef struct SMgrRelationData /* * The following fields are reset to InvalidBlockNumber upon a cache flush * event, and hold the last known size for each fork. This information is - * currently only reliable during recovery, since there is no cache - * invalidation for fork extension. + * currently only reliable during recovery, since in general there is no + * cache invalidation for fork extension. The exception is the visibility + * map, whose extension outside recovery is announced with + * CacheInvalidateSmgrNblocks(). */ BlockNumber smgr_targblock; /* current insertion target block */ BlockNumber smgr_cached_nblocks[MAX_FORKNUM + 1]; /* last known size */ @@ -86,6 +88,7 @@ extern void smgrdestroyall(void); extern void smgrrelease(SMgrRelation reln); extern void smgrreleaseall(void); extern void smgrreleaserellocator(RelFileLocatorBackend rlocator); +extern void smgrforgetnblocks(RelFileLocatorBackend rlocator); extern void smgrcreate(SMgrRelation reln, ForkNumber forknum, bool isRedo); extern void smgrdosyncall(SMgrRelation *rels, int nrels); extern void smgrdounlinkall(SMgrRelation *rels, int nrels, bool isRedo); diff --git a/src/include/utils/inval.h b/src/include/utils/inval.h index 735e42f731..67d792026a 100644 --- a/src/include/utils/inval.h +++ b/src/include/utils/inval.h @@ -80,6 +80,7 @@ extern void CacheInvalidateRelSync(Oid relid); extern void CacheInvalidateRelSyncAll(void); extern void CacheInvalidateSmgr(RelFileLocatorBackend rlocator); +extern void CacheInvalidateSmgrNblocks(RelFileLocatorBackend rlocator); extern void CacheInvalidateRelmap(Oid databaseId); -- 2.49.0