From 2dd2ac793e5267598a7bdc4c9276ac65a7d0e2b3 Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot Date: Thu, 15 Oct 2026 12:17:42 +0000 Subject: [PATCH v1 1/4] Move buffer strategy ownership out of BulkInsertState Buffer access strategies are generally owned by the operation that creates them. BulkInsertState is an exception: it owns both its BAS_BULKWRITE strategy and relation local insertion state. Those objects have different useful lifetimes: one bulk operation may need several relation local states while all of them should share the same buffer ring. Pass the strategy to GetBulkInsertState(), leaving its allocation and release to the caller. Author: Bertrand Drouvot Reviewed-by: Discussion: https://postgr.es/m/ --- src/backend/access/heap/heapam.c | 9 ++++++--- src/backend/access/heap/heapam_handler.c | 12 +++++++++++- src/backend/commands/copyfrom.c | 23 +++++++++++++++++++++-- src/backend/commands/createas.c | 11 ++++++++++- src/backend/commands/matview.c | 6 +++++- src/backend/commands/tablecmds.c | 6 +++++- src/include/access/heapam.h | 2 +- src/include/access/hio.h | 4 ++-- 8 files changed, 61 insertions(+), 12 deletions(-) 22.1% src/backend/access/heap/ 63.2% src/backend/commands/ 14.5% src/include/access/ diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 6a09a1a1517..d52468c90f9 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -1935,14 +1935,18 @@ UpdateXmaxHintBits(HeapTupleHeader tuple, Buffer buffer, TransactionId xid) /* * GetBulkInsertState - prepare status object for a bulk insert + * + * The caller owns strategy and may share it with other bulk insert states. */ BulkInsertState -GetBulkInsertState(void) +GetBulkInsertState(BufferAccessStrategy strategy) { BulkInsertState bistate; + Assert(strategy != NULL); + bistate = (BulkInsertState) palloc_object(BulkInsertStateData); - bistate->strategy = GetAccessStrategy(BAS_BULKWRITE); + bistate->strategy = strategy; bistate->current_buf = InvalidBuffer; bistate->next_free = InvalidBlockNumber; bistate->last_free = InvalidBlockNumber; @@ -1958,7 +1962,6 @@ FreeBulkInsertState(BulkInsertState bistate) { if (bistate->current_buf != InvalidBuffer) ReleaseBuffer(bistate->current_buf); - FreeAccessStrategy(bistate->strategy); pfree(bistate); } diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 7d2065dec37..e33b9a82c74 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -596,6 +596,7 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap, double *tups_recently_dead) { RewriteState rwstate; + BufferAccessStrategy strategy; BulkInsertState bistate; IndexScanDesc indexScan; TableScanDesc tableScan; @@ -638,9 +639,15 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap, /* In concurrent mode, prepare for bulk-insert operation. */ if (concurrent) - bistate = GetBulkInsertState(); + { + strategy = GetAccessStrategy(BAS_BULKWRITE); + bistate = GetBulkInsertState(strategy); + } else + { + strategy = NULL; bistate = NULL; + } /* Set up sorting if wanted */ if (use_sort) @@ -941,7 +948,10 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap, if (rwstate) end_heap_rewrite(rwstate); if (bistate) + { FreeBulkInsertState(bistate); + FreeAccessStrategy(strategy); + } /* Clean up */ pfree(values); diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c index 746b4fdf6c3..8729706bdbe 100644 --- a/src/backend/commands/copyfrom.c +++ b/src/backend/commands/copyfrom.c @@ -44,6 +44,7 @@ #include "optimizer/optimizer.h" #include "pgstat.h" #include "rewrite/rewriteHandler.h" +#include "storage/bufmgr.h" #include "storage/fd.h" #include "tcop/tcopprot.h" #include "utils/lsyscache.h" @@ -81,6 +82,7 @@ typedef struct CopyMultiInsertBuffer { TupleTableSlot *slots[MAX_BUFFERED_TUPLES]; /* Array to store tuples */ ResultRelInfo *resultRelInfo; /* ResultRelInfo for 'relid' */ + BufferAccessStrategy strategy; /* bulk-write buffer strategy */ BulkInsertState bistate; /* BulkInsertState for this rel if plain * table; NULL if foreign table */ int nused; /* number of 'slots' containing tuples */ @@ -369,7 +371,16 @@ CopyMultiInsertBufferInit(ResultRelInfo *rri) buffer = palloc_object(CopyMultiInsertBuffer); memset(buffer->slots, 0, sizeof(TupleTableSlot *) * MAX_BUFFERED_TUPLES); buffer->resultRelInfo = rri; - buffer->bistate = (rri->ri_FdwRoutine == NULL) ? GetBulkInsertState() : NULL; + if (rri->ri_FdwRoutine == NULL) + { + buffer->strategy = GetAccessStrategy(BAS_BULKWRITE); + buffer->bistate = GetBulkInsertState(buffer->strategy); + } + else + { + buffer->strategy = NULL; + buffer->bistate = NULL; + } buffer->nused = 0; return buffer; @@ -635,9 +646,13 @@ CopyMultiInsertBufferCleanup(CopyMultiInsertInfo *miinfo, { Assert(buffer->bistate != NULL); FreeBulkInsertState(buffer->bistate); + FreeAccessStrategy(buffer->strategy); } else + { Assert(buffer->bistate == NULL); + Assert(buffer->strategy == NULL); + } /* Since we only create slots on demand, just drop the non-null ones. */ for (i = 0; i < MAX_BUFFERED_TUPLES && buffer->slots[i] != NULL; i++) @@ -793,6 +808,7 @@ CopyFrom(CopyFromState cstate) ErrorContextCallback errcallback; CommandId mycid = GetCurrentCommandId(true); uint32 ti_options = 0; /* start with default options for insert */ + BufferAccessStrategy strategy = NULL; BulkInsertState bistate = NULL; CopyInsertMethod insertMethod; CopyMultiInsertInfo multiInsertInfo = {0}; /* pacify compiler */ @@ -1086,7 +1102,8 @@ CopyFrom(CopyFromState cstate) { singleslot = table_slot_create(resultRelInfo->ri_RelationDesc, &estate->es_tupleTable); - bistate = GetBulkInsertState(); + strategy = GetAccessStrategy(BAS_BULKWRITE); + bistate = GetBulkInsertState(strategy); } has_before_insert_row_trig = (resultRelInfo->ri_TrigDesc && @@ -1484,6 +1501,8 @@ CopyFrom(CopyFromState cstate) if (bistate != NULL) FreeBulkInsertState(bistate); + if (strategy != NULL) + FreeAccessStrategy(strategy); MemoryContextSwitchTo(oldcontext); diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c index 6dbb831ca89..8b336e0a3f8 100644 --- a/src/backend/commands/createas.c +++ b/src/backend/commands/createas.c @@ -42,6 +42,7 @@ #include "nodes/queryjumble.h" #include "parser/analyze.h" #include "rewrite/rewriteHandler.h" +#include "storage/bufmgr.h" #include "tcop/tcopprot.h" #include "utils/builtins.h" #include "utils/lsyscache.h" @@ -57,6 +58,7 @@ typedef struct ObjectAddress reladdr; /* address of rel, for ExecCreateTableAs */ CommandId output_cid; /* cmin to insert in output tuples */ uint32 ti_options; /* table_tuple_insert performance options */ + BufferAccessStrategy strategy; /* bulk-write buffer strategy */ BulkInsertState bistate; /* bulk insert state */ } DR_intorel; @@ -565,9 +567,15 @@ intorel_startup(DestReceiver *self, int operation, TupleDesc typeinfo) * bulk inserts as there are no tuples to insert. */ if (!into->skipData) - myState->bistate = GetBulkInsertState(); + { + myState->strategy = GetAccessStrategy(BAS_BULKWRITE); + myState->bistate = GetBulkInsertState(myState->strategy); + } else + { + myState->strategy = NULL; myState->bistate = NULL; + } /* * Valid smgr_targblock implies something already wrote to the relation. @@ -619,6 +627,7 @@ intorel_shutdown(DestReceiver *self) if (!into->skipData) { FreeBulkInsertState(myState->bistate); + FreeAccessStrategy(myState->strategy); table_finish_bulk_insert(myState->rel, myState->ti_options); } diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c index 40748958eaf..d231004bd80 100644 --- a/src/backend/commands/matview.c +++ b/src/backend/commands/matview.c @@ -33,6 +33,7 @@ #include "miscadmin.h" #include "pgstat.h" #include "rewrite/rewriteHandler.h" +#include "storage/bufmgr.h" #include "storage/lmgr.h" #include "tcop/tcopprot.h" #include "utils/builtins.h" @@ -50,6 +51,7 @@ typedef struct Relation transientrel; /* relation to write to */ CommandId output_cid; /* cmin to insert in output tuples */ uint32 ti_options; /* table_tuple_insert performance options */ + BufferAccessStrategy strategy; /* bulk-write buffer strategy */ BulkInsertState bistate; /* bulk insert state */ } DR_transientrel; @@ -493,7 +495,8 @@ transientrel_startup(DestReceiver *self, int operation, TupleDesc typeinfo) myState->transientrel = transientrel; myState->output_cid = GetCurrentCommandId(true); myState->ti_options = TABLE_INSERT_SKIP_FSM | TABLE_INSERT_FROZEN; - myState->bistate = GetBulkInsertState(); + myState->strategy = GetAccessStrategy(BAS_BULKWRITE); + myState->bistate = GetBulkInsertState(myState->strategy); /* * Valid smgr_targblock implies something already wrote to the relation. @@ -539,6 +542,7 @@ transientrel_shutdown(DestReceiver *self) DR_transientrel *myState = (DR_transientrel *) self; FreeBulkInsertState(myState->bistate); + FreeAccessStrategy(myState->strategy); table_finish_bulk_insert(myState->transientrel, myState->ti_options); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 93ad49e59fd..7f40a6e1299 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -6207,6 +6207,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap) ListCell *l; EState *estate; CommandId mycid; + BufferAccessStrategy strategy; BulkInsertState bistate; uint32 ti_options; ExprState *partqualstate = NULL; @@ -6235,13 +6236,15 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap) if (newrel) { mycid = GetCurrentCommandId(true); - bistate = GetBulkInsertState(); + strategy = GetAccessStrategy(BAS_BULKWRITE); + bistate = GetBulkInsertState(strategy); ti_options = TABLE_INSERT_SKIP_FSM; } else { /* keep compiler quiet about using these uninitialized */ mycid = 0; + strategy = NULL; bistate = NULL; ti_options = 0; } @@ -6620,6 +6623,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap) if (newrel) { FreeBulkInsertState(bistate); + FreeAccessStrategy(strategy); table_finish_bulk_insert(newrel, ti_options); diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h index 9e35961fb9e..1d5cdecc74b 100644 --- a/src/include/access/heapam.h +++ b/src/include/access/heapam.h @@ -371,7 +371,7 @@ extern bool heap_fetch(Relation relation, Snapshot snapshot, extern void heap_get_latest_tid(TableScanDesc sscan, ItemPointer tid); -extern BulkInsertState GetBulkInsertState(void); +extern BulkInsertState GetBulkInsertState(BufferAccessStrategy strategy); extern void FreeBulkInsertState(BulkInsertState); extern void ReleaseBulkInsertStatePin(BulkInsertState bistate); diff --git a/src/include/access/hio.h b/src/include/access/hio.h index 60cfc375fd5..be17351b2d2 100644 --- a/src/include/access/hio.h +++ b/src/include/access/hio.h @@ -21,8 +21,8 @@ /* * state for bulk inserts --- private to heapam.c and hio.c * - * If current_buf isn't InvalidBuffer, then we are holding an extra pin - * on that buffer. + * The caller owns the buffer access strategy. If current_buf isn't + * InvalidBuffer, then we are holding an extra pin on that buffer. * * "typedef struct BulkInsertStateData *BulkInsertState" is in heapam.h */ -- 2.34.1