From 8a79fcb5ef7b2379855619b5e75a8b612f103ca2 Mon Sep 17 00:00:00 2001 From: Lukas Fittl Date: Thu, 22 Oct 2026 15:21:56 +0200 Subject: [PATCH v18 5/9] instrumentation: Retire pgBufferUsage in favor of stack-based approach The stack-based instrumentation left the plain Instrumentation API (InstrAlloc/InstrInitOptions with InstrStart/InstrStop) measuring by diffing the global pgBufferUsage/pgWalUsage counters, and kept pgBufferUsage updated alongside the stack for that purpose. Convert the remaining users of that API to the stack: VACUUM and ANALYZE logging as well as the planning-time measurement of EXPLAIN and EXECUTE use QueryInstrumentation, which handles aborts through the resource owner, while pg_stat_statements wraps its planner and utility statement timing in PG_TRY/PG_FINALLY and the EXPLAIN (SERIALIZE) destination receiver registers its entry as a child of the query instrumentation. With that, Instrumentation entries always track WAL/buffer usage through the stack, so remove the diff mode along with the global pgBufferUsage and BufferUsageAccumDiff, and update comments that referred to it. Extensions that read pgBufferUsage directly should use InstrStart and InstrStop around the code section they want to measure instead. The related global pgWalUsage is kept for now due to its use in pgstat to track aggregate WAL activity and heap_page_prune_and_freeze for measuring FPIs. Author: Lukas Fittl Reviewed-by: Discussion: https://postgr.es/m/CAP53PkzdBK8VJ1fS4AZ481LgMN8f9mJiC39ZRHqkFUSYq6KWmg%40mail.gmail.com --- .../pg_stat_statements/pg_stat_statements.c | 6 +- src/backend/access/brin/brin.c | 4 +- src/backend/access/gin/gininsert.c | 4 +- src/backend/access/heap/vacuumlazy.c | 12 +-- src/backend/access/nbtree/nbtsort.c | 4 +- src/backend/commands/analyze.c | 12 +-- src/backend/commands/explain.c | 25 +++-- src/backend/commands/explain_dr.c | 29 +++++ src/backend/commands/prepare.c | 21 ++-- src/backend/commands/vacuumparallel.c | 4 +- src/backend/executor/execParallel.c | 2 +- src/backend/executor/instrument.c | 100 ++---------------- src/backend/storage/buffer/bufmgr.c | 6 +- src/backend/utils/activity/pgstat_io.c | 6 +- src/include/commands/explain_dr.h | 2 + src/include/executor/instrument.h | 47 +++----- 16 files changed, 114 insertions(+), 170 deletions(-) diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c index 3f3604cda84..248cee584ff 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.c +++ b/contrib/pg_stat_statements/pg_stat_statements.c @@ -943,12 +943,11 @@ pgss_planner(Query *parse, } PG_FINALLY(); { + InstrStopFinalize(&instr); nesting_level--; } PG_END_TRY(); - InstrStop(&instr); - pgss_store(query_string, parse->queryId, parse->stmt_location, @@ -1177,6 +1176,7 @@ pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString, } PG_FINALLY(); { + InstrStopFinalize(&instr); nesting_level--; } PG_END_TRY(); @@ -1192,8 +1192,6 @@ pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString, */ pstmt = NULL; - InstrStop(&instr); - /* * Track the total number of rows retrieved or affected by the utility * statements of COPY, FETCH, CREATE TABLE AS, CREATE MATERIALIZED diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c index 8c2c69605ce..730b17d232f 100644 --- a/src/backend/access/brin/brin.c +++ b/src/backend/access/brin/brin.c @@ -2434,8 +2434,8 @@ _brin_begin_parallel(BrinBuildState *buildstate, Relation heap, Relation index, * and PARALLEL_KEY_BUFFER_USAGE. * * If there are no extensions loaded that care, we could skip this. We - * have no way of knowing whether anyone's looking at pgWalUsage or - * pgBufferUsage, so do it unconditionally. + * have no way of knowing whether anyone's looking at instrumentation, so + * do it unconditionally. */ shm_toc_estimate_chunk(&pcxt->estimator, mul_size(sizeof(WalUsage), pcxt->nworkers)); diff --git a/src/backend/access/gin/gininsert.c b/src/backend/access/gin/gininsert.c index b3b90ee6685..af04d8643b0 100644 --- a/src/backend/access/gin/gininsert.c +++ b/src/backend/access/gin/gininsert.c @@ -991,8 +991,8 @@ _gin_begin_parallel(GinBuildState *buildstate, Relation heap, Relation index, * and PARALLEL_KEY_BUFFER_USAGE. * * If there are no extensions loaded that care, we could skip this. We - * have no way of knowing whether anyone's looking at pgWalUsage or - * pgBufferUsage, so do it unconditionally. + * have no way of knowing whether anyone's looking at instrumentation, so + * do it unconditionally. */ shm_toc_estimate_chunk(&pcxt->estimator, mul_size(sizeof(WalUsage), pcxt->nworkers)); diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index f2ee5dccc9f..169fd82aec2 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -636,7 +636,7 @@ heap_vacuum_rel(Relation rel, const VacuumParams *params, new_rel_allfrozen; PGRUsage ru0; TimestampTz starttime = 0; - Instrumentation *instr = NULL; + QueryInstrumentation *instr = NULL; ErrorContextCallback errcallback; char **indnames = NULL; Size dead_items_max_bytes = 0; @@ -647,8 +647,8 @@ heap_vacuum_rel(Relation rel, const VacuumParams *params, if (instrument) { pg_rusage_init(&ru0); - instr = InstrAlloc(INSTRUMENT_BUFFERS | INSTRUMENT_WAL); - InstrStart(instr); + instr = InstrQueryAlloc(INSTRUMENT_BUFFERS | INSTRUMENT_WAL); + InstrQueryStart(instr); } /* Used for instrumentation and stats report */ @@ -989,7 +989,7 @@ heap_vacuum_rel(Relation rel, const VacuumParams *params, { TimestampTz endtime = GetCurrentTimestamp(); - InstrStop(instr); + InstrQueryStopFinalize(instr); if (verbose || params->log_vacuum_min_duration == 0 || TimestampDifferenceExceeds(starttime, endtime, @@ -1005,8 +1005,8 @@ heap_vacuum_rel(Relation rel, const VacuumParams *params, int64 total_blks_hit; int64 total_blks_read; int64 total_blks_dirtied; - BufferUsage bufferusage = instr->bufusage; - WalUsage walusage = instr->walusage; + BufferUsage bufferusage = instr->instr.bufusage; + WalUsage walusage = instr->instr.walusage; TimestampDifference(starttime, endtime, &secs_dur, &usecs_dur); diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c index 3d88c2556bb..2d7b7cef912 100644 --- a/src/backend/access/nbtree/nbtsort.c +++ b/src/backend/access/nbtree/nbtsort.c @@ -1466,8 +1466,8 @@ _bt_begin_parallel(BTBuildState *buildstate, bool isconcurrent, int request) * and PARALLEL_KEY_BUFFER_USAGE. * * If there are no extensions loaded that care, we could skip this. We - * have no way of knowing whether anyone's looking at pgWalUsage or - * pgBufferUsage, so do it unconditionally. + * have no way of knowing whether anyone's looking at instrumentation, so + * do it unconditionally. */ shm_toc_estimate_chunk(&pcxt->estimator, mul_size(sizeof(WalUsage), pcxt->nworkers)); diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index 382b3181374..371b56b339d 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -354,7 +354,7 @@ do_analyze_rel(Relation onerel, const VacuumParams *params, Oid save_userid; int save_sec_context; int save_nestlevel; - Instrumentation *instr = NULL; + QueryInstrumentation *instr = NULL; PgStat_Counter startreadtime = 0; PgStat_Counter startwritetime = 0; @@ -406,8 +406,8 @@ do_analyze_rel(Relation onerel, const VacuumParams *params, pg_rusage_init(&ru0); - instr = InstrAlloc(INSTRUMENT_BUFFERS | INSTRUMENT_WAL); - InstrStart(instr); + instr = InstrQueryAlloc(INSTRUMENT_BUFFERS | INSTRUMENT_WAL); + InstrQueryStart(instr); } /* Used for instrumentation and stats report */ @@ -770,7 +770,7 @@ do_analyze_rel(Relation onerel, const VacuumParams *params, { TimestampTz endtime = GetCurrentTimestamp(); - InstrStop(instr); + InstrQueryStopFinalize(instr); if (verbose || params->log_analyze_min_duration == 0 || TimestampDifferenceExceeds(starttime, endtime, @@ -784,8 +784,8 @@ do_analyze_rel(Relation onerel, const VacuumParams *params, int64 total_blks_hit; int64 total_blks_read; int64 total_blks_dirtied; - BufferUsage bufusage = instr->bufusage; - WalUsage walusage = instr->walusage; + BufferUsage bufusage = instr->instr.bufusage; + WalUsage walusage = instr->instr.walusage; total_blks_hit = bufusage.shared_blks_hit + bufusage.local_blks_hit; diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index ca8c071ec04..97d56f2bd93 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -327,7 +327,7 @@ standard_ExplainOneQuery(Query *query, int cursorOptions, QueryEnvironment *queryEnv) { PlannedStmt *plan; - Instrumentation plan_instr = {0}; + QueryInstrumentation *plan_instr = NULL; int instrument_options = INSTRUMENT_TIMER; MemoryContextCounters mem_counters; MemoryContext planner_ctx = NULL; @@ -336,7 +336,7 @@ standard_ExplainOneQuery(Query *query, int cursorOptions, if (es->buffers) instrument_options |= INSTRUMENT_BUFFERS; - InstrInitOptions(&plan_instr, instrument_options); + plan_instr = InstrQueryAlloc(instrument_options); if (es->memory) { @@ -354,22 +354,27 @@ standard_ExplainOneQuery(Query *query, int cursorOptions, saved_ctx = MemoryContextSwitchTo(planner_ctx); } - InstrStart(&plan_instr); + InstrQueryStart(plan_instr); /* plan the query */ plan = pg_plan_query(query, queryString, cursorOptions, params, es); - InstrStop(&plan_instr); - if (es->memory) - { MemoryContextSwitchTo(saved_ctx); + + /* + * Finalize only after switching back, so that the instrumentation's + * memory context gets reparented to our context rather than the planner + * one, where it would be counted as planner memory. + */ + InstrQueryStopFinalize(plan_instr); + + if (es->memory) MemoryContextMemConsumed(planner_ctx, &mem_counters); - } /* run it (if needed) and produce output */ ExplainOnePlan(plan, into, es, queryString, params, queryEnv, - &plan_instr.total, (es->buffers ? &plan_instr.bufusage : NULL), + &plan_instr->instr.total, (es->buffers ? &plan_instr->instr.bufusage : NULL), es->memory ? &mem_counters : NULL); } @@ -565,6 +570,10 @@ ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es, /* call ExecutorStart to prepare the plan for execution */ ExecutorStart(queryDesc, eflags); + /* Point the SERIALIZE receiver at the now-created query instrumentation. */ + if (es->serialize != EXPLAIN_SERIALIZE_NONE) + SetExplainSerializeQueryInstr(dest, queryDesc->estate->es_query_instr); + /* Execute the plan for statistics if asked for */ if (es->analyze) { diff --git a/src/backend/commands/explain_dr.c b/src/backend/commands/explain_dr.c index 9f9e73f14a3..9c90902cfae 100644 --- a/src/backend/commands/explain_dr.c +++ b/src/backend/commands/explain_dr.c @@ -41,6 +41,7 @@ typedef struct SerializeDestReceiver MemoryContext tmpcontext; /* per-row temporary memory context */ StringInfoData buf; /* buffer to hold the constructed message */ SerializeMetrics metrics; /* collected metrics */ + QueryInstrumentation *qinstr; /* parent instrumentation to accumulate to */ } SerializeDestReceiver; /* @@ -226,6 +227,14 @@ serializeAnalyzeStartup(DestReceiver *self, int operation, TupleDesc typeinfo) if (receiver->es->buffers) instrument_options |= INSTRUMENT_BUFFERS; InstrInitOptions(&receiver->metrics.instr, instrument_options); + + /* + * Register the metrics as a child of the query instrumentation, so the + * usage measured below reaches the session totals even on error, when + * serializeAnalyzeShutdown (which finalizes it on success) is not called. + */ + Assert(receiver->qinstr != NULL); + InstrQueryRememberChild(receiver->qinstr, &receiver->metrics.instr); } /* @@ -236,6 +245,9 @@ serializeAnalyzeShutdown(DestReceiver *self) { SerializeDestReceiver *receiver = (SerializeDestReceiver *) self; + /* Accumulate what we measured to the query instrumentation. */ + InstrFinalizeChild(&receiver->metrics.instr, &receiver->qinstr->instr); + if (receiver->finfos) pfree(receiver->finfos); receiver->finfos = NULL; @@ -279,6 +291,23 @@ CreateExplainSerializeDestReceiver(ExplainState *es) return (DestReceiver *) self; } +/* + * Set the query instrumentation that a serialize receiver accumulates to. + * + * Must be called after ExecutorStart has created the instrumentation and + * before the query runs. A no-op for other receivers. + */ +void +SetExplainSerializeQueryInstr(DestReceiver *self, QueryInstrumentation *qinstr) +{ + SerializeDestReceiver *receiver = (SerializeDestReceiver *) self; + + if (receiver->pub.mydest != DestExplainSerialize) + return; + + receiver->qinstr = qinstr; +} + /* * GetSerializationMetrics - get serialization metrics * diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c index bf9f2eb6149..9a673a9971d 100644 --- a/src/backend/commands/prepare.c +++ b/src/backend/commands/prepare.c @@ -581,7 +581,7 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es, ListCell *p; ParamListInfo paramLI = NULL; EState *estate = NULL; - Instrumentation plan_instr = {0}; + QueryInstrumentation *plan_instr = NULL; int instrument_options = INSTRUMENT_TIMER; MemoryContextCounters mem_counters; MemoryContext planner_ctx = NULL; @@ -590,7 +590,7 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es, if (es->buffers) instrument_options |= INSTRUMENT_BUFFERS; - InstrInitOptions(&plan_instr, instrument_options); + plan_instr = InstrQueryAlloc(instrument_options); if (es->memory) { @@ -602,7 +602,7 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es, saved_ctx = MemoryContextSwitchTo(planner_ctx); } - InstrStart(&plan_instr); + InstrQueryStart(plan_instr); /* Look it up in the hash table */ entry = FetchPreparedStatement(execstmt->name, true); @@ -637,13 +637,18 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es, cplan = GetCachedPlan(entry->plansource, paramLI, CurrentResourceOwner, pstate->p_queryEnv); - InstrStop(&plan_instr); - if (es->memory) - { MemoryContextSwitchTo(saved_ctx); + + /* + * Finalize only after switching back, so that the instrumentation's + * memory context gets reparented to our context rather than the planner + * one, where it would be counted as planner memory. + */ + InstrQueryStopFinalize(plan_instr); + + if (es->memory) MemoryContextMemConsumed(planner_ctx, &mem_counters); - } plan_list = cplan->stmt_list; @@ -654,7 +659,7 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es, if (pstmt->commandType != CMD_UTILITY) ExplainOnePlan(pstmt, into, es, query_string, paramLI, pstate->p_queryEnv, - &plan_instr.total, (es->buffers ? &plan_instr.bufusage : NULL), + &plan_instr->instr.total, (es->buffers ? &plan_instr->instr.bufusage : NULL), es->memory ? &mem_counters : NULL); else ExplainOneUtility(pstmt->utilityStmt, into, es, pstate, paramLI); diff --git a/src/backend/commands/vacuumparallel.c b/src/backend/commands/vacuumparallel.c index 3f64934db86..31096808ee5 100644 --- a/src/backend/commands/vacuumparallel.c +++ b/src/backend/commands/vacuumparallel.c @@ -372,8 +372,8 @@ parallel_vacuum_init(Relation rel, Relation *indrels, int nindexes, * PARALLEL_VACUUM_KEY_BUFFER_USAGE and PARALLEL_VACUUM_KEY_WAL_USAGE. * * If there are no extensions loaded that care, we could skip this. We - * have no way of knowing whether anyone's looking at pgBufferUsage or - * pgWalUsage, so do it unconditionally. + * have no way of knowing whether anyone's looking at instrumentation, so + * do it unconditionally. */ shm_toc_estimate_chunk(&pcxt->estimator, mul_size(sizeof(BufferUsage), pcxt->nworkers)); diff --git a/src/backend/executor/execParallel.c b/src/backend/executor/execParallel.c index 1ef88a951d5..b31508037d9 100644 --- a/src/backend/executor/execParallel.c +++ b/src/backend/executor/execParallel.c @@ -739,7 +739,7 @@ ExecInitParallelPlan(PlanState *planstate, EState *estate, * * If EXPLAIN is not in use and there are no extensions loaded that care, * we could skip this. But we have no way of knowing whether anyone's - * looking at pgBufferUsage, so do it unconditionally. + * looking at instrumentation, so do it unconditionally. */ shm_toc_estimate_chunk(&pcxt->estimator, mul_size(sizeof(BufferUsage), pcxt->nworkers)); diff --git a/src/backend/executor/instrument.c b/src/backend/executor/instrument.c index 9b660f6f210..e5580395e8b 100644 --- a/src/backend/executor/instrument.c +++ b/src/backend/executor/instrument.c @@ -24,7 +24,6 @@ #include "utils/memutils.h" #include "utils/resowner.h" -BufferUsage pgBufferUsage; WalUsage pgWalUsage; Instrumentation instr_top; InstrStackState instr_stack = { @@ -62,39 +61,21 @@ InstrStackGrow(void) } /* General purpose instrumentation handling */ -Instrumentation * -InstrAlloc(int instrument_options) +static inline bool +InstrNeedStack(int instrument_options) { - Instrumentation *instr = palloc0_object(Instrumentation); - - InstrInitOptions(instr, instrument_options); - return instr; + return (instrument_options & (INSTRUMENT_BUFFERS | INSTRUMENT_WAL)) != 0; } void InstrInitOptions(Instrumentation *instr, int instrument_options) { - instr->need_bufusage = (instrument_options & INSTRUMENT_BUFFERS) != 0; - instr->need_walusage = (instrument_options & INSTRUMENT_WAL) != 0; + instr->need_stack = InstrNeedStack(instrument_options); instr->need_timer = (instrument_options & INSTRUMENT_TIMER) != 0; } -static inline bool -InstrNeedStack(int instrument_options) -{ - return (instrument_options & (INSTRUMENT_BUFFERS | INSTRUMENT_WAL)) != 0; -} - -/* - * Common part of InstrStart and InstrStartNode: start the timer, and enter - * the entry onto the instrumentation stack. - * - * The diff-mode handling (see struct comment in instrument.h) lives only in - * InstrStart: node instrumentation never measures by diffing, so keeping the - * checks out of here keeps them out of the per-tuple ExecProcNodeInstr path. - */ -static inline void -InstrStartCommon(Instrumentation *instr) +inline void +InstrStart(Instrumentation *instr) { if (instr->need_timer) { @@ -108,26 +89,9 @@ InstrStartCommon(Instrumentation *instr) InstrPushStack(instr); } -inline void -InstrStart(Instrumentation *instr) -{ - InstrStartCommon(instr); - - if (!instr->need_stack) - { - /* save buffer usage totals at start, if needed */ - if (instr->need_bufusage) - instr->bufusage_start = pgBufferUsage; - - if (instr->need_walusage) - instr->walusage_start = pgWalUsage; - } -} - /* * Helper for InstrStop() and InstrStopNode(), to avoid code duplication - * despite slightly different needs about how time is accumulated. Like - * InstrStartCommon, this omits the diff-mode handling. + * despite slightly different needs about how time is accumulated. */ static inline void InstrStopCommon(Instrumentation *instr, instr_time *accum_time) @@ -155,18 +119,6 @@ void InstrStop(Instrumentation *instr) { InstrStopCommon(instr, &instr->total); - - if (!instr->need_stack) - { - /* Add delta of buffer usage since InstrStart to the totals */ - if (instr->need_bufusage) - BufferUsageAccumDiff(&instr->bufusage, - &pgBufferUsage, &instr->bufusage_start); - - if (instr->need_walusage) - WalUsageAccumDiff(&instr->walusage, - &pgWalUsage, &instr->walusage_start); - } } /* @@ -351,7 +303,6 @@ InstrQueryAlloc(int instrument_options) instr->instr_cxt = instr_cxt; InstrInitOptions(&instr->instr, instrument_options); - instr->instr.need_stack = InstrNeedStack(instrument_options); dlist_init(&instr->unfinalized_entries); return instr; @@ -451,12 +402,11 @@ InstrEndParallelQuery(QueryInstrumentation *qinstr, BufferUsage *bufusage, WalUs } /* - * Record WAL/buffer usage imported from a parallel worker. + * Record WAL usage imported from a parallel worker. */ void InstrAccumWorkerUsage(const BufferUsage *bufusage, const WalUsage *walusage) { - BufferUsageAdd(&pgBufferUsage, bufusage); WalUsageAdd(&pgWalUsage, walusage); } @@ -509,7 +459,6 @@ InstrInitNode(NodeInstrumentation *instr, int instrument_options, bool async_mod { memset(instr, 0, sizeof(NodeInstrumentation)); InstrInitOptions(&instr->instr, instrument_options); - instr->instr.need_stack = InstrNeedStack(instrument_options); instr->async_mode = async_mode; } @@ -517,7 +466,7 @@ InstrInitNode(NodeInstrumentation *instr, int instrument_options, bool async_mod inline void InstrStartNode(NodeInstrumentation *instr) { - InstrStartCommon(&instr->instr); + InstrStart(&instr->instr); } @@ -653,7 +602,6 @@ InstrAllocTrigger(QueryInstrumentation *qinstr, int instrument_options, int n) for (i = 0; i < n; i++) { InstrInitOptions(&tginstr[i].instr, instrument_options); - tginstr[i].instr.need_stack = InstrNeedStack(instrument_options); InstrQueryRememberChild(qinstr, &tginstr[i].instr); } @@ -713,36 +661,6 @@ BufferUsageAdd(BufferUsage *dst, const BufferUsage *add) INSTR_TIME_ADD(dst->temp_blk_write_time, add->temp_blk_write_time); } -/* dst += add - sub */ -inline void -BufferUsageAccumDiff(BufferUsage *dst, - const BufferUsage *add, - const BufferUsage *sub) -{ - dst->shared_blks_hit += add->shared_blks_hit - sub->shared_blks_hit; - dst->shared_blks_read += add->shared_blks_read - sub->shared_blks_read; - dst->shared_blks_dirtied += add->shared_blks_dirtied - sub->shared_blks_dirtied; - dst->shared_blks_written += add->shared_blks_written - sub->shared_blks_written; - dst->local_blks_hit += add->local_blks_hit - sub->local_blks_hit; - dst->local_blks_read += add->local_blks_read - sub->local_blks_read; - dst->local_blks_dirtied += add->local_blks_dirtied - sub->local_blks_dirtied; - dst->local_blks_written += add->local_blks_written - sub->local_blks_written; - dst->temp_blks_read += add->temp_blks_read - sub->temp_blks_read; - dst->temp_blks_written += add->temp_blks_written - sub->temp_blks_written; - INSTR_TIME_ACCUM_DIFF(dst->shared_blk_read_time, - add->shared_blk_read_time, sub->shared_blk_read_time); - INSTR_TIME_ACCUM_DIFF(dst->shared_blk_write_time, - add->shared_blk_write_time, sub->shared_blk_write_time); - INSTR_TIME_ACCUM_DIFF(dst->local_blk_read_time, - add->local_blk_read_time, sub->local_blk_read_time); - INSTR_TIME_ACCUM_DIFF(dst->local_blk_write_time, - add->local_blk_write_time, sub->local_blk_write_time); - INSTR_TIME_ACCUM_DIFF(dst->temp_blk_read_time, - add->temp_blk_read_time, sub->temp_blk_read_time); - INSTR_TIME_ACCUM_DIFF(dst->temp_blk_write_time, - add->temp_blk_write_time, sub->temp_blk_write_time); -} - /* helper functions for WAL usage accumulation */ static inline void WalUsageAdd(WalUsage *dst, const WalUsage *add) diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 13327f5de8e..f9f36e0354b 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -1259,9 +1259,9 @@ PinBufferForBlock(Relation rel, if (rel) { /* - * While pgBufferUsage's "read" counter isn't bumped unless we reach - * WaitReadBuffers() (so, not for hits, and not for buffers that are - * zeroed instead), the per-relation stats always count them. + * While the current buffer usage "read" counter isn't bumped unless + * we reach WaitReadBuffers() (so, not for hits, and not for buffers + * that are zeroed instead), the per-relation stats always count them. */ pgstat_count_buffer_read(rel); } diff --git a/src/backend/utils/activity/pgstat_io.c b/src/backend/utils/activity/pgstat_io.c index b6b1e7b0b06..8c02cd2d40f 100644 --- a/src/backend/utils/activity/pgstat_io.c +++ b/src/backend/utils/activity/pgstat_io.c @@ -106,9 +106,9 @@ pgstat_prepare_io_time(bool track_io_guc) * pg_stat_database only counts block read and write times, these are done for * IOOP_READ, IOOP_WRITE and IOOP_EXTEND. * - * pgBufferUsage is used for EXPLAIN. pgBufferUsage has write and read stats - * for shared, local and temporary blocks. pg_stat_io does not track the - * activity of temporary blocks, so these are ignored here. + * Executor instrumentation is used for EXPLAIN. Buffer usage tracked there has + * write and read stats for shared, local and temporary blocks. pg_stat_io + * does not track the activity of temporary blocks, so these are ignored here. */ void pgstat_count_io_op_time(IOObject io_object, IOContext io_context, IOOp io_op, diff --git a/src/include/commands/explain_dr.h b/src/include/commands/explain_dr.h index ab5c53023e1..e3ef82c3ea4 100644 --- a/src/include/commands/explain_dr.h +++ b/src/include/commands/explain_dr.h @@ -27,6 +27,8 @@ typedef struct SerializeMetrics } SerializeMetrics; extern DestReceiver *CreateExplainSerializeDestReceiver(ExplainState *es); +extern void SetExplainSerializeQueryInstr(DestReceiver *dest, + QueryInstrumentation *qinstr); extern SerializeMetrics *GetSerializationMetrics(DestReceiver *dest); #endif diff --git a/src/include/executor/instrument.h b/src/include/executor/instrument.h index e63af41089f..17d560fb982 100644 --- a/src/include/executor/instrument.h +++ b/src/include/executor/instrument.h @@ -58,7 +58,7 @@ typedef struct WalUsage int64 wal_buffers_full; /* # of times the WAL buffers became full */ } WalUsage; -/* Flag bits included in InstrAlloc's instrument_options bitmask */ +/* Flag bits included in the instrument_options bitmask */ typedef enum InstrumentOption { INSTRUMENT_TIMER = 1 << 0, /* needs timer (and row counts) */ @@ -72,36 +72,31 @@ typedef enum InstrumentOption /* * Instrumentation base class for capturing time and WAL/buffer usage * - * If used directly (InstrAlloc, or a zero initialized struct followed by - * InstrInitOptions), WAL/buffer usage is measured by diffing the global - * pgBufferUsage/pgWalUsage counters between InstrStart and InstrStop. This - * is independent of the instrumentation stack and needs no special handling - * for transaction aborts. + * If used directly: + * - Allocate on the stack and zero initialize the struct + * - Call InstrInitOptions to set instrumentation options + * - Call InstrStart before the activity you want to measure + * - Call InstrStop / InstrStopFinalize after the activity to capture totals * - * Entries with need_stack set, as used by QueryInstrumentation and the node and - * trigger instrumentation, are instead pushed onto the instrumentation stack by - * InstrStart, so that all activity gets recorded directly in them (the diff - * flags are ignored). The last stop call must then be to InstrStopFinalize, to - * ensure parent stack entries get the accumulated totals. If there is risk of - * transaction aborts you must call InstrStopFinalize in a PG_TRY/PG_FINALLY - * block to avoid corrupting the instrumentation stack, or use - * QueryInstrumentation, which handles aborts using the resource owner logic. + * InstrStart/InstrStop may be called multiple times. The last stop call must + * be to InstrStopFinalize to ensure parent stack entries get the accumulated + * totals. If there is risk of transaction aborts you must call + * InstrStopFinalize in a PG_TRY/PG_FINALLY block to avoid corrupting the + * instrumentation stack. + * + * In a query context use QueryInstrumentation instead, which handles aborts + * using the resource owner logic. */ typedef struct Instrumentation { /* Parameters set at creation: */ bool need_timer; /* true if we need timer data */ - bool need_bufusage; /* true if we need buffer usage data */ - bool need_walusage; /* true if we need WAL usage data */ - bool need_stack; /* true if WAL/buffer usage is tracked via - * instr_stack instead */ + bool need_stack; /* true if we need WAL/buffer usage data */ /* Internal state keeping: */ bool on_stack; /* true if currently on instr_stack */ bool finalized; /* true once accumulated to a parent via * InstrFinalizeChild */ instr_time starttime; /* start time of last InstrStart */ - BufferUsage bufusage_start; /* buffer usage at start (diff mode) */ - WalUsage walusage_start; /* WAL usage at start (diff mode) */ /* Accumulated statistics: */ instr_time total; /* total runtime */ BufferUsage bufusage; /* total buffer usage */ @@ -218,11 +213,6 @@ typedef struct InstrStackState Instrumentation *current; /* top of stack, or &instr_top when empty */ } InstrStackState; -/* - * Running totals of the current backend's buffer/WAL usage, kept updated - * alongside the instrumentation stack. - */ -extern PGDLLIMPORT BufferUsage pgBufferUsage; extern PGDLLIMPORT WalUsage pgWalUsage; /* @@ -285,7 +275,6 @@ InstrPopStack(Instrumentation *instr) instr->on_stack = false; } -extern Instrumentation *InstrAlloc(int instrument_options); extern void InstrInitOptions(Instrumentation *instr, int instrument_options); extern void InstrStart(Instrumentation *instr); extern void InstrStop(Instrumentation *instr); @@ -321,25 +310,19 @@ extern TriggerInstrumentation *InstrAllocTrigger(QueryInstrumentation *qinstr, extern void InstrStartTrigger(TriggerInstrumentation *tginstr); extern void InstrStopTrigger(TriggerInstrumentation *tginstr, int64 firings); -extern void BufferUsageAccumDiff(BufferUsage *dst, - const BufferUsage *add, const BufferUsage *sub); extern void WalUsageAccumDiff(WalUsage *dst, const WalUsage *add, const WalUsage *sub); #define INSTR_BUFUSAGE_INCR(fld) do { \ - pgBufferUsage.fld++; \ instr_stack.current->bufusage.fld++; \ } while(0) #define INSTR_BUFUSAGE_ADD(fld,val) do { \ - pgBufferUsage.fld += (val); \ instr_stack.current->bufusage.fld += (val); \ } while(0) #define INSTR_BUFUSAGE_TIME_ADD(fld,val) do { \ - INSTR_TIME_ADD(pgBufferUsage.fld, val); \ INSTR_TIME_ADD(instr_stack.current->bufusage.fld, val); \ } while (0) #define INSTR_BUFUSAGE_TIME_ACCUM_DIFF(fld,endval,startval) do { \ - INSTR_TIME_ACCUM_DIFF(pgBufferUsage.fld, endval, startval); \ INSTR_TIME_ACCUM_DIFF(instr_stack.current->bufusage.fld, endval, startval); \ } while (0) -- 2.47.1