From 61f3e0513efdc26625820a04ec5af185f3049398 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Wed, 22 Feb 2023 15:28:34 -0500 Subject: [PATCH v1 4/4] add vacuum option to specify nbuffers --- contrib/amcheck/verify_heapam.c | 2 +- contrib/amcheck/verify_nbtree.c | 2 +- contrib/bloom/blscan.c | 2 +- contrib/pg_visibility/pg_visibility.c | 4 +- contrib/pgstattuple/pgstatapprox.c | 2 +- contrib/pgstattuple/pgstatindex.c | 4 +- contrib/pgstattuple/pgstattuple.c | 2 +- src/backend/access/heap/heapam.c | 4 +- src/backend/commands/dbcommands.c | 2 +- src/backend/commands/vacuum.c | 50 ++++++++++++++++++++++- src/backend/commands/vacuumparallel.c | 8 +++- src/backend/postmaster/autovacuum.c | 5 ++- src/backend/storage/buffer/bufmgr.c | 4 +- src/backend/storage/buffer/freelist.c | 58 ++++++++++++++++++++++++--- src/include/commands/vacuum.h | 1 + src/include/storage/bufmgr.h | 2 +- 16 files changed, 127 insertions(+), 25 deletions(-) diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c index 4fcfd6df72..e3aa89975d 100644 --- a/contrib/amcheck/verify_heapam.c +++ b/contrib/amcheck/verify_heapam.c @@ -330,7 +330,7 @@ verify_heapam(PG_FUNCTION_ARGS) PG_RETURN_NULL(); } - ctx.bstrategy = GetAccessStrategy(BAS_BULKREAD); + ctx.bstrategy = GetAccessStrategy(BAS_BULKREAD, -1); ctx.buffer = InvalidBuffer; ctx.page = NULL; diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c index 257cff671b..b0552a94ef 100644 --- a/contrib/amcheck/verify_nbtree.c +++ b/contrib/amcheck/verify_nbtree.c @@ -547,7 +547,7 @@ bt_check_every_level(Relation rel, Relation heaprel, bool heapkeyspace, state->targetcontext = AllocSetContextCreate(CurrentMemoryContext, "amcheck context", ALLOCSET_DEFAULT_SIZES); - state->checkstrategy = GetAccessStrategy(BAS_BULKREAD); + state->checkstrategy = GetAccessStrategy(BAS_BULKREAD, -1); /* Get true root block from meta-page */ metapage = palloc_btree_page(state, BTREE_METAPAGE); diff --git a/contrib/bloom/blscan.c b/contrib/bloom/blscan.c index 6cc7d07164..10e3086e39 100644 --- a/contrib/bloom/blscan.c +++ b/contrib/bloom/blscan.c @@ -119,7 +119,7 @@ blgetbitmap(IndexScanDesc scan, TIDBitmap *tbm) * We're going to read the whole index. This is why we use appropriate * buffer access strategy. */ - bas = GetAccessStrategy(BAS_BULKREAD); + bas = GetAccessStrategy(BAS_BULKREAD, -1); npages = RelationGetNumberOfBlocks(scan->indexRelation); for (blkno = BLOOM_HEAD_BLKNO; blkno < npages; blkno++) diff --git a/contrib/pg_visibility/pg_visibility.c b/contrib/pg_visibility/pg_visibility.c index 2a4acfd1ee..47a113db67 100644 --- a/contrib/pg_visibility/pg_visibility.c +++ b/contrib/pg_visibility/pg_visibility.c @@ -476,7 +476,7 @@ collect_visibility_data(Oid relid, bool include_pd) vbits *info; BlockNumber blkno; Buffer vmbuffer = InvalidBuffer; - BufferAccessStrategy bstrategy = GetAccessStrategy(BAS_BULKREAD); + BufferAccessStrategy bstrategy = GetAccessStrategy(BAS_BULKREAD, -1); rel = relation_open(relid, AccessShareLock); @@ -554,7 +554,7 @@ collect_corrupt_items(Oid relid, bool all_visible, bool all_frozen) corrupt_items *items; BlockNumber blkno; Buffer vmbuffer = InvalidBuffer; - BufferAccessStrategy bstrategy = GetAccessStrategy(BAS_BULKREAD); + BufferAccessStrategy bstrategy = GetAccessStrategy(BAS_BULKREAD, -1); TransactionId OldestXmin = InvalidTransactionId; rel = relation_open(relid, AccessShareLock); diff --git a/contrib/pgstattuple/pgstatapprox.c b/contrib/pgstattuple/pgstatapprox.c index f601dc6121..e71a468988 100644 --- a/contrib/pgstattuple/pgstatapprox.c +++ b/contrib/pgstattuple/pgstatapprox.c @@ -72,7 +72,7 @@ statapprox_heap(Relation rel, output_type *stat) TransactionId OldestXmin; OldestXmin = GetOldestNonRemovableTransactionId(rel); - bstrategy = GetAccessStrategy(BAS_BULKREAD); + bstrategy = GetAccessStrategy(BAS_BULKREAD, -1); nblocks = RelationGetNumberOfBlocks(rel); scanned = 0; diff --git a/contrib/pgstattuple/pgstatindex.c b/contrib/pgstattuple/pgstatindex.c index d69ac1c93d..53c6e99bfa 100644 --- a/contrib/pgstattuple/pgstatindex.c +++ b/contrib/pgstattuple/pgstatindex.c @@ -219,7 +219,7 @@ pgstatindex_impl(Relation rel, FunctionCallInfo fcinfo) BlockNumber nblocks; BlockNumber blkno; BTIndexStat indexStat; - BufferAccessStrategy bstrategy = GetAccessStrategy(BAS_BULKREAD); + BufferAccessStrategy bstrategy = GetAccessStrategy(BAS_BULKREAD, -1); if (!IS_INDEX(rel) || !IS_BTREE(rel)) ereport(ERROR, @@ -612,7 +612,7 @@ pgstathashindex(PG_FUNCTION_ARGS) nblocks = RelationGetNumberOfBlocks(rel); /* prepare access strategy for this index */ - bstrategy = GetAccessStrategy(BAS_BULKREAD); + bstrategy = GetAccessStrategy(BAS_BULKREAD, -1); /* Start from blkno 1 as 0th block is metapage */ for (blkno = 1; blkno < nblocks; blkno++) diff --git a/contrib/pgstattuple/pgstattuple.c b/contrib/pgstattuple/pgstattuple.c index 93b7834b77..587d79bb38 100644 --- a/contrib/pgstattuple/pgstattuple.c +++ b/contrib/pgstattuple/pgstattuple.c @@ -518,7 +518,7 @@ pgstat_index(Relation rel, BlockNumber start, pgstat_page pagefn, pgstattuple_type stat = {0}; /* prepare access strategy for this index */ - bstrategy = GetAccessStrategy(BAS_BULKREAD); + bstrategy = GetAccessStrategy(BAS_BULKREAD, -1); blkno = start; for (;;) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 7eb79cee58..6a3efe3d2d 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -280,7 +280,7 @@ initscan(HeapScanDesc scan, ScanKey key, bool keep_startblock) { /* During a rescan, keep the previous strategy object. */ if (scan->rs_strategy == NULL) - scan->rs_strategy = GetAccessStrategy(BAS_BULKREAD); + scan->rs_strategy = GetAccessStrategy(BAS_BULKREAD, -1); } else { @@ -1772,7 +1772,7 @@ GetBulkInsertState(void) BulkInsertState bistate; bistate = (BulkInsertState) palloc(sizeof(BulkInsertStateData)); - bistate->strategy = GetAccessStrategy(BAS_BULKWRITE); + bistate->strategy = GetAccessStrategy(BAS_BULKWRITE, -1); bistate->current_buf = InvalidBuffer; return bistate; } diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c index a0259cc593..51f3d64de7 100644 --- a/src/backend/commands/dbcommands.c +++ b/src/backend/commands/dbcommands.c @@ -280,7 +280,7 @@ ScanSourceDatabasePgClass(Oid tbid, Oid dbid, char *srcpath) smgrclose(smgr); /* Use a buffer access strategy since this is a bulk read operation. */ - bstrategy = GetAccessStrategy(BAS_BULKREAD); + bstrategy = GetAccessStrategy(BAS_BULKREAD, -1); /* * As explained in the function header comments, we need a snapshot that diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 78980cf19c..b3fded878b 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -126,6 +126,10 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) /* By default parallel vacuum is enabled */ params.nworkers = 0; + /* default value of buffers buffer access strategy */ + params.buffers = -1; + + /* Parse options list */ foreach(lc, vacstmt->options) { @@ -207,6 +211,45 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) skip_database_stats = defGetBoolean(opt); else if (strcmp(opt->defname, "only_database_stats") == 0) only_database_stats = defGetBoolean(opt); + else if (strcmp(opt->defname, "buffer_usage_limit") == 0) + { + // TODO: default_ring_size calculated here and GetAccessStrategy() == bad + int vac_buffers; + int default_ring_size = 256 * 1024 / BLCKSZ; + int max_ring_size = NBuffers / 8; + + if (opt->arg == NULL) + { + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("buffer_usage_limit option requires an integer value"), + parser_errposition(pstate, opt->location))); + } + + vac_buffers = defGetInt64(opt); + + /* out-of-bounds cases */ + if (vac_buffers < -1 || vac_buffers > max_ring_size) + { + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("buffer_usage_limit for a vacuum must be between -1 and %d", + max_ring_size), + parser_errposition(pstate, opt->location))); + } + + if (vac_buffers == -1) + vac_buffers = default_ring_size; + else if (vac_buffers < default_ring_size) + { + elog(WARNING, "buffer_usage_limit %d is below the minimum buffer_usage_limit of %d. setting it to %d", + vac_buffers, default_ring_size, default_ring_size); + + vac_buffers = default_ring_size; + } + + params.buffers = vac_buffers; + } else ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), @@ -393,8 +436,7 @@ vacuum(List *relations, VacuumParams *params, if (bstrategy == NULL) { MemoryContext old_context = MemoryContextSwitchTo(vac_context); - - bstrategy = GetAccessStrategy(BAS_VACUUM); + bstrategy = GetAccessStrategy(BAS_VACUUM, params->buffers); MemoryContextSwitchTo(old_context); } @@ -2066,7 +2108,11 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, bool skip_privs, cluster_rel(relid, InvalidOid, &cluster_params); } else + { + if (params->buffers == 0) + bstrategy = NULL; table_relation_vacuum(rel, params, bstrategy); + } /* Roll back any GUC changes executed by index functions */ AtEOXact_GUC(false, save_nestlevel); diff --git a/src/backend/commands/vacuumparallel.c b/src/backend/commands/vacuumparallel.c index bcd40c80a1..2c8380bd2e 100644 --- a/src/backend/commands/vacuumparallel.c +++ b/src/backend/commands/vacuumparallel.c @@ -1012,8 +1012,12 @@ parallel_vacuum_main(dsm_segment *seg, shm_toc *toc) pvs.indname = NULL; pvs.status = PARALLEL_INDVAC_STATUS_INITIAL; - /* Each parallel VACUUM worker gets its own access strategy */ - pvs.bstrategy = GetAccessStrategy(BAS_VACUUM); + /* + * Each parallel VACUUM worker gets its own access strategy + * For now, use the default buffer access strategy ring size. + * TODO: should this work differently even though they are only for indexes + */ + pvs.bstrategy = GetAccessStrategy(BAS_VACUUM, -1); /* Setup error traceback support for ereport() */ errcallback.callback = parallel_vacuum_error_callback; diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 673ff99767..3d88d72973 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -2291,8 +2291,11 @@ do_autovacuum(void) * Create a buffer access strategy object for VACUUM to use. We want to * use the same one across all the vacuum operations we perform, since the * point is for VACUUM not to blow out the shared cache. + * If we later enter failsafe mode, we will cease use of the + * BufferAccessStrategy. Either way, we clean up the BufferAccessStrategy + * object at the end of this function. */ - bstrategy = GetAccessStrategy(BAS_VACUUM); + bstrategy = GetAccessStrategy(BAS_VACUUM, -1); /* * create a memory context to act as fake PortalContext, so that the diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 98904a7c05..62ef088306 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -3818,8 +3818,8 @@ RelationCopyStorageUsingBuffer(RelFileLocator srclocator, buf.data, true); /* This is a bulk operation, so use buffer access strategies. */ - bstrategy_src = GetAccessStrategy(BAS_BULKREAD); - bstrategy_dst = GetAccessStrategy(BAS_BULKWRITE); + bstrategy_src = GetAccessStrategy(BAS_BULKREAD, -1); + bstrategy_dst = GetAccessStrategy(BAS_BULKWRITE, -1); /* Iterate over each block of the source relation file. */ for (blkno = 0; blkno < nblocks; blkno++) diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c index c690d5f15f..24d21cc4f1 100644 --- a/src/backend/storage/buffer/freelist.c +++ b/src/backend/storage/buffer/freelist.c @@ -531,23 +531,61 @@ StrategyInitialize(bool init) * ---------------------------------------------------------------- */ +static const char * +btype_get_name(BufferAccessStrategyType btype) +{ + switch (btype) + { + case BAS_NORMAL: + return "normal"; + case BAS_BULKREAD: + return "bulkread"; + case BAS_BULKWRITE: + return "bulkwrite"; + case BAS_VACUUM: + return "vacuum"; + } + + elog(ERROR, "unrecognized BufferAccessStrategy: %d", btype); + pg_unreachable(); +} + + /* * GetAccessStrategy -- create a BufferAccessStrategy object * * The object is allocated in the current memory context. */ +// TODO: make a macro or something for nbuffers = -1 BufferAccessStrategy -GetAccessStrategy(BufferAccessStrategyType btype) +GetAccessStrategy(BufferAccessStrategyType btype, int buffers) { BufferAccessStrategy strategy; int ring_size; + const char *strategy_name = btype_get_name(btype); + + if (btype != BAS_VACUUM) + { + if (buffers == 0) + elog(ERROR, "Use of shared buffers unsupported for buffer access strategy: %s. nbuffers must be -1.", + strategy_name); + + if (buffers > 0) + elog(ERROR, "Specification of ring size in buffers unsupported for buffer access strategy: %s. nbuffers must be -1.", + strategy_name); + } + + // TODO: DEBUG logging message for dev? + if (buffers == 0) + btype = BAS_NORMAL; /* * Select ring size to use. See buffer/README for rationales. * * Note: if you change the ring size for BAS_BULKREAD, see also * SYNC_SCAN_REPORT_INTERVAL in access/heap/syncscan.c. + * TODO: update README */ switch (btype) { @@ -562,16 +600,26 @@ GetAccessStrategy(BufferAccessStrategyType btype) ring_size = 16 * 1024 * 1024 / BLCKSZ; break; case BAS_VACUUM: - ring_size = 256 * 1024 / BLCKSZ; - break; + { + int default_ring_size = 256 * 1024 / BLCKSZ; + if (buffers == -1) + ring_size = default_ring_size; + else + ring_size = Max(buffers, default_ring_size); + // TODO: log if we end up setting ring_size bigger than nbuffers because it would have been smaller than the default + break; + } + /* unreachable as long as we get the string representation of the type above */ default: elog(ERROR, "unrecognized buffer access strategy: %d", - (int) btype); - return NULL; /* keep compiler quiet */ + (int) btype); + + pg_unreachable(); } /* Make sure ring isn't an undue fraction of shared buffers */ + // TODO: log message that tells you if your specified number of buffers is clamped ring_size = Min(NBuffers / 8, ring_size); /* Allocate the object and initialize all elements to zeroes */ diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h index 689dbb7702..341f1e2525 100644 --- a/src/include/commands/vacuum.h +++ b/src/include/commands/vacuum.h @@ -235,6 +235,7 @@ typedef struct VacuumParams * disabled. */ int nworkers; + int buffers; } VacuumParams; /* diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h index b8a18b8081..2157200973 100644 --- a/src/include/storage/bufmgr.h +++ b/src/include/storage/bufmgr.h @@ -195,7 +195,7 @@ extern Size BufferShmemSize(void); extern void AtProcExit_LocalBuffers(void); /* in freelist.c */ -extern BufferAccessStrategy GetAccessStrategy(BufferAccessStrategyType btype); +extern BufferAccessStrategy GetAccessStrategy(BufferAccessStrategyType btype, int nbuffers); extern void FreeAccessStrategy(BufferAccessStrategy strategy); -- 2.37.2