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 17:02:28 -0400
Message-ID: <pgvy7lsoa5jldwgyay2g757xivzbmgyan547wealc7cwtduvra@dysyw6dtqdgs> (raw)
In-Reply-To: <CAP53Pky4zHCueAyPkWE-cH6SqE_m+Kppmy=zE5K36X3HvCFZLw@mail.gmail.com>
References: <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>
<mtjyijvuv7xavvcdy3rosg43ycy3t5sluioxkef46se25ajtxp@e3e2xvesqhzu>
<CAP53Pky4zHCueAyPkWE-cH6SqE_m+Kppmy=zE5K36X3HvCFZLw@mail.gmail.com>
Hi,
On 2026-04-05 12:38:58 -0700, Lukas Fittl wrote:
> On Sun, Apr 5, 2026 at 11:13 AM Andres Freund <[email protected]> wrote:
> > 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}.
>
> Argh, good catch. That's on me for not manually testing it when I
> factored it out.
>
> I've confirmed this works now, both with 0001 only, and with 0001+0002.
I made 'firings' an an int64, rather than int. Could have made it unsigned,
but ExplainPropertyInteger accepts an int64...
Because the patch did change those lines anyway, I replaced
palloc0(sizeof(Instrumentation)) with palloc_object(), and
palloc0(n * sizeof(TriggerInstrumentation)) with palloc_array().
It also seemed silly to have an if around the assingments of need_*:
if (instrument_options & (INSTRUMENT_BUFFERS | INSTRUMENT_TIMER | INSTRUMENT_WAL))
{
instr->need_bufusage = (instrument_options & INSTRUMENT_BUFFERS) != 0;
instr->need_walusage = (instrument_options & INSTRUMENT_WAL) != 0;
instr->need_timer = (instrument_options & INSTRUMENT_TIMER) != 0;
instr->async_mode = async_mode;
but that gets cleared up in 0002 anyway.
But it did lead me to notice a pre-existing bug: We only set async_mode in the
if (INSTRUMENT_BUFFERS | INSTRUMENT_TIMER | INSTRUMENT_WAL)
branch.
It looks like that doesn't matter today, because all async_mode is used for is
/*
* In async mode, if the plan node hadn't emitted any tuples before,
* this might be the first tuple
*/
if (instr->async_mode && save_tuplecount < 1.0)
instr->firsttuple = instr->counter;
and without INSTRUMENT_TIMER instr->counter would be zero anyway.
But I guess it's worth noting that in the commit message for 0002?
I felt a bit silly leaving the instr->need_* stuff in InstrAlloc(), when there
is InstrInit() directly afterwards that does the same thing, but then that
leads to removing the redundant memset etc, so I left it for 0002.
With that I pushed 0001.
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: <pgvy7lsoa5jldwgyay2g757xivzbmgyan547wealc7cwtduvra@dysyw6dtqdgs>
* 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