Re: Stack-based tracking of per-node WAL/buffer usage
Lukas Fittl <lukas@fittl.com>
From: Lukas Fittl <lukas@fittl.com>
To: Zsolt Parragi <zsolt.parragi@percona.com>,
Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Tomas Vondra <tomas@vondra.me>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
Andres Freund <andres@anarazel.de>, Peter Smith <smithpb2250@gmail.com>
Date: 2026-03-23T20:03:21Z
Lists: pgsql-hackers
On Mon, Mar 23, 2026 at 12:07 PM Zsolt Parragi <zsolt.parragi@percona.com> wrote: > > > I'm looking at this finalize at resowner part of this patch, and this > > maybe a stupid question, but: > > > > Why does the instrumentation need to be "finalized" on abort? If you run > > EXPLAIN ANALYZE and the query aborts, you don't get to see the stats anyway. > > The pg_session_buffer_usage in 0009 makes the information available, I > was able to see the issue with failing triggers with that. Even if > that part doesn't get committed in the end, a 3rd party extension > could still implement the same thing, and notice the missing > statistics. (And maybe it is useful to see some statistics about > failing queries?) Right, there are basically two reasons we need the resource owner logic, or something equivalent: 1) To ensure the active stack entry gets reset correctly to where it was before, so that we don't corrupt it after an abort (if that's not done, we'll fail during regular regression tests) 2) To accumulate statistics to parent stack entries that did not participate in the abort As Zsolt noted, (2) matters for the pg_session_buffer_usage module, which is mainly intended to ensure that the logic we're adding keeps a top-level instrumentation available that includes the activity of aborted transactions/queries, since we're removing pgBufferUsage. It also matters for some edge cases in-tree today, e.g. procedures being tracked in pg_stat_statements. And I do see us being interested in tracking failed query activities in the future as Zsolt noted. --- FWIW, on the topic of resource owners and allocations, I've done a test over the weekend, and here is a question: It seems we could switch the Instrumentation allocations we're doing when inside a portal to PortalContext, and CurrentMemoryContext when outside a portal - instead of allocating in TopMemoryContext/TopTransactionContext. That works in practice, because resource owner cleanup happens before PortalContext cleanup, and simplifies the code a bit since we can skip copying into the current memory context (because the caller wants to be able to use the result after the finalize call). And if we leak we'd only leak until PortalContext gets cleaned up, instead of TopMemoryContext. To expand on that, in the previously posted v9 we have the following allocations: A) InstrStackState allocated under TopMemoryContext (long-lived, never freed) B) QueryInstrumentation allocated under TopMemoryContext (short-lived during query execution, explicitly freed up on abort or finalize call) C) NodeInstrumentation allocated under TopTransactionContext (short-lived during query execution, explicitly freed up on abort or finalize call) D) In other use cases, e.g. ANALYZE command that logs buffer usage, QueryInstrumentation allocated under TopMemoryContext (short-lived during command execution, explicitly freed up on abort or finalize call) And we could switch it instead to: A) InstrStackState allocated under TopMemoryContext (long-lived, never freed) B) QueryInstrumentation allocated under PortalContext (short-lived during query execution, *automatically* freed up on abort, manually on ExecutorEnd to avoid waiting for holdable cursors to free PortalContext) C) NodeInstrumentation allocated under PortalContext (short-lived during query execution, *automatically* freed up on abort, manually on ExecutorEnd to avoid waiting for holdable cursors to free PortalContext) D) In other use cases, e.g. ANALYZE command that logs buffer usage, QueryInstrumentation allocated under CurrentMemoryContext (short-lived during command execution, *automatically* freed up on abort and success case) However, this goes against the principle noted by Heikki over in [0] that ResOwners should use TopMemoryContext to avoid relying on the ordering of clean up operations. Thoughts? Thanks, Lukas [0]: https://www.postgresql.org/message-id/flat/a3197b31-f40d-4164-872d-906d8e9b374a%40iki.fi#526984c1be0662a7526bcfe0b358564b -- Lukas Fittl
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
instrumentation: Allocate query level instrumentation in ExecutorStart
- 2c16deee2f7d 19 (unreleased) landed
-
instrumentation: Move ExecProcNodeInstr to allow inlining
- 544000288ec8 19 (unreleased) landed
-
instrumentation: Separate per-node logic from other uses
- 5a79e78501f4 19 (unreleased) landed
-
instrumentation: Separate trigger logic from other uses
- 7d9b74df53e9 19 (unreleased) landed
-
instrumentation: Rename INSTR_TIME_LT macro to INSTR_TIME_GT
- 3218825271bd 19 (unreleased) landed
-
instrumentation: Keep time fields as instrtime, convert in callers
- e5a5e0a90750 19 (unreleased) landed