public inbox for [email protected]
help / color / mirror / Atom feedFrom: Andres Freund <[email protected]>
To: Lukas Fittl <[email protected]>
Cc: Heikki Linnakangas <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Zsolt Parragi <[email protected]>
Cc: Tomas Vondra <[email protected]>
Cc: Peter Smith <[email protected]>
Subject: Re: Stack-based tracking of per-node WAL/buffer usage
Date: Sun, 5 Apr 2026 14:13:51 -0400
Message-ID: <mtjyijvuv7xavvcdy3rosg43ycy3t5sluioxkef46se25ajtxp@e3e2xvesqhzu> (raw)
In-Reply-To: <CAP53PkxDupm5U6TV0LF_YWHQ2wfcSiLsgnDBcO6b5AchLJhp=A@mail.gmail.com>
References: <[email protected]>
<CAN4CZFOUTHdXuEAwrST8ueDxGJRe69zVgeVAY0osTjVkRZi_Lw@mail.gmail.com>
<CAP53Pkx+HZ2OSGDHmNgSDisXdjGp4KFygaX+GaQcFEuXHRXg=g@mail.gmail.com>
<CAP53PkznofNg+ii363QQGoje30nhssuSz_hV5U4YANAt-Yr_Yg@mail.gmail.com>
<[email protected]>
<CAP53PkwCk7X_ryOak_3x6ek2f+4kCgGjWe8aVy39D59Q8y9wTg@mail.gmail.com>
<CAP53Pky2=31B1AS9vg=Ca9308_hb0H4g968cSKxiFgT0moJfYg@mail.gmail.com>
<CAP53PkybGzJTAo7V07ssUae5047pBY3jc_F07tGY13cNhqe+AQ@mail.gmail.com>
<57biou6l65r7gr4nunoe6lignz2x6m3w45gihoypaez4pc46di@txj3bakhj66l>
<CAP53PkxDupm5U6TV0LF_YWHQ2wfcSiLsgnDBcO6b5AchLJhp=A@mail.gmail.com>
Hi,
Not a real reply to your email, just looking at committing 0001/0002 to get
them out of the way.
Unfortunately I think 0001 on its own doesn't actually work correctly. I
luckily tried an EXPLAIN ANALYZE with triggers and noticed that the time is
reported as zeroes.
The only reason I tried is because I misread the diff and though you'd changed
the calls=%.3f to calls=%d, even though the old state is calls=%.0f...
The reason it doesn't work is that explain shows tginstr->instr.total, but
with the patch the trigger instrumentation just computes
tginstr->instr.{counter,firsttuple}.
And of course we don't have any tests even showing trigger output. Not that
such a test would have been likely to catch this issue, as something like the
the amount of time is nontrivial to test.
This is actually fixed by 0002, as it makes InstrStop() update ->total,
rather than ->counter as before.
But I'd prefer not to break the intermediary state ;).
I guess we could squash both patches?
But probably the least bad solution is to add an InstrEndLoop() to in 0001 and
remove it again in 0002.
Re 0002
In passing, drop the "n" argument to InstrAlloc, as all remaining callers
need exactly one Instrumentation struct.
I think that probably should be in 0001?
I'm kinda wondering whether, to keep the line lenghts manageable,
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -1837,7 +1837,7 @@ ExplainNode(PlanState *planstate, List *ancestors,
{
double nloops = planstate->instrument->nloops;
double startup_ms = INSTR_TIME_GET_MILLISEC(planstate->instrument->startup) / nloops;
- double total_ms = INSTR_TIME_GET_MILLISEC(planstate->instrument->total) / nloops;
+ double total_ms = INSTR_TIME_GET_MILLISEC(planstate->instrument->instr.total) / nloops;
double rows = planstate->instrument->ntuples / nloops;
Should store planstate->instrument in a local var and wrap after =.
But not sure it's worth bothering with.
Greetings,
Andres Freund
view thread (42+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Stack-based tracking of per-node WAL/buffer usage
In-Reply-To: <mtjyijvuv7xavvcdy3rosg43ycy3t5sluioxkef46se25ajtxp@e3e2xvesqhzu>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox