MIME-Version: 1.0
References: <CAKZiRmwvE4uJLKTgPXeBA4m+d4tTghayoefcaM9=z3_S7i72GA@mail.gmail.com>
 <vhzkeogzrrfzjwo3xrnq4xsjh6i37ou6xsbz7yby3lbb3rnxzz@6fpysnkjyldi>
 <CAKZiRmxNVMOV7x5c_Amqk=2mmYJOqsfHgE8N8O9jGjgfBYa8kQ@mail.gmail.com>
 <CANwKhkM1FNS1Wmc56+aunXhaP_zjjO2YKzKTJRoVW0RsM4Of3w@mail.gmail.com> <CAKZiRmxmVUpLb9HiY1MUczCLNFpP30f0+ZRS9NaeTf7DZjt4Tg@mail.gmail.com>
In-Reply-To: <CAKZiRmxmVUpLb9HiY1MUczCLNFpP30f0+ZRS9NaeTf7DZjt4Tg@mail.gmail.com>
From: Ants Aasma <ants.aasma@cybertec.at>
Date: Thu, 12 Feb 2026 12:31:31 +0200
Message-ID: <CANwKhkMwydw8P-H-tUzbmNsbk7B=buM+UDT4Rb90Y_OBhNQtdg@mail.gmail.com>
Subject: Re: pg_stat_io_histogram
To: Jakub Wartak <jakub.wartak@enterprisedb.com>
Cc: Andres Freund <andres@anarazel.de>, 
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/alternative; boundary="0000000000009ab9f3064a9dfed9"
Archived-At: <https://www.postgresql.org/message-id/CANwKhkMwydw8P-H-tUzbmNsbk7B%3DbuM%2BUDT4Rb90Y_OBhNQtdg%40mail.gmail.com>
Precedence: bulk

--0000000000009ab9f3064a9dfed9
Content-Type: text/plain; charset="UTF-8"

On Thu, 12 Feb 2026 at 10:35, Jakub Wartak <jakub.wartak@enterprisedb.com>
wrote:

> BTW how have you arrived with the "240x" number? We have 16 buckets for
> each
> of the object/context/type.
>

Sorry, I worded that poorly. I meant that we store a histogram for each
combination, 240 in total.

>
> > even then scanning an extra 25KB of mostly zeroes on every commit
> > doesn't seem great. Maybe making the histogram accumulation
> > conditional on the counter field being non-zero is enough to avoid any
> > issues? I haven't yet constructed a benchmark to see if it's actually
> > a problem or not. Select only pgbench with small shared buffers and
> > scale that fits into page cache should be an adversarial use case
> > while still being reasonably realistic.
>
> Earlier I've done some benchmarks (please see [1]) based on recommendations
> by Andres to keep low io_combine_limit for that and just tiny
> shared_buffers.
> I'm getting too much noise to derive any results, and as this is related
> to I/O even probably context switches start playing a role there... sadly
> we
> seem not to have a performance farm to answer this.
>

I glossed over the first benchmark you did. That's pretty close to what I
was talking about - exercise the stats collection part by having ~1 I/O
served from page cache per a trivial transaction. And the prewarm should
exercise the per I/O overhead. If neither of them have any measurable
overhead then I can't think of a workload where it could be worse.

TBH, I'm not sure how to progress with this, I mean we could as you say:
> - reduce PgStat_PendingIO.pending_hist_time_buckets by removing
> IOCONTEXT_NUM_TYPES
>   (not a big loss, just lack of showing BAS strategy)
>

I'm on the fence on this. For the actual problems I've had to diagnose it
wouldn't have mattered. But latency differences of bulk vs. normal access
might be useful for understanding benchmark results better. A 5x reduction
in size is pretty big.


> - we could even further reduce PgStat_PendingIO.pending_hist_time_buckets
>   by removing IOOBJECT_NUM_TYPES, but those are just 3 and they might be
>   useful
>

WAL write vs. relation write is a very useful distinction for me.

... and are You saying to try to do this below thing too?
>
> @@ -288,8 +290,9 @@ pgstat_io_flush_cb(bool nowait)
>                                 for(int b = 0; b < PGSTAT_IO_HIST_BUCKETS;
> b++)
> -
> bktype_shstats->hist_time_buckets[io_object][io_context][io_op][b] +=
> -
> PendingIOStats.pending_hist_time_buckets[io_object][io_context][io_op][b];
> +
>
> if(PendingIOStats.pending_hist_time_buckets[io_object][io_context][io_op][b]
> > 0)
> +
> bktype_shstats->hist_time_buckets[io_object][io_context][io_op][b] +=
> +
> PendingIOStats.pending_hist_time_buckets[io_object][io_context][io_op][b];
>
>
I meant this:

@@ -287,6 +287,7 @@ pgstat_io_flush_cb(bool nowait)

bktype_shstats->times[io_object][io_context][io_op] +=
                                        INSTR_TIME_GET_MICROSEC(time);

+                               if
(PendingIOStats.counts[io_object][io_context][io_op] > 0)
                                        for(int b = 0; b <
PGSTAT_IO_HIST_BUCKETS; b++)

bktype_shstats->hist_time_buckets[io_object][io_context][io_op][b] +=

PendingIOStats.pending_hist_time_buckets[io_object][io_context][io_op][b];

Most object/context/op combinations will have a 0 count, so no point in
actually looking at the histogram.


> .. but the main problem, even if I do all of that I won't be able to
> reliably measure the impact, probably the best I could say is
> "runs good as well as master, +/- 3%".
>
> Could you somehow help me with this? I mean should we reduce the
> scope(remove
> context) and add that "if"?
>

I think if we only aggregate histograms conditionally, then having a ton of
different histograms is less of a problem. Only the histograms that have
any data will get accessed. The overhead is limited to the memory usage
which I think is acceptable.

I'll run a few benchmarks on what I have available here to see if I can
tease out anything more than the no effect with a 3% error margin we have
today.


> > I'm not familiar enough with the new stats infrastructure to tell
> > whether it's a problem, but it seems odd that
> > pgstat_flush_backend_entry_io() isn't modified to aggregate the
> > histograms.
>
> Well I'm first time doing this too, and my understanding is that
> pgstat_io.c::pgstat_io_flush_cb() is flushing the global statistics
> (per backend-type) while the per-individual backend
> pgstat_flush_backend_entry_io() (from pgstat_backend.c) is more about
> per-PID-backends stats (--> for: select * from
> pg_stat_get_backend_io(PID)).
>
> In terms of this patch, the per-backend-PID-I/O histograms are not
> implemented
> yet, and I've raised this question earlier, but I'm starting to believe
> the answer is probably no, we should not implement those (more overhead
> for no apparent benefit, as most of the cases could be tracked down just
> with
> this overall per-backend-type stats ).
>

I agree that per-PID histograms are probably not worth the extra work. But
this left me wondering if we are allocating the whole set of histograms too
many times. I don't think every place that uses PgStat_BktypeIO actually
needs the histograms. I will need to dig around to understand this code a
bit better.

Regards,
Ants Aasma

--0000000000009ab9f3064a9dfed9
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr">On Thu, 12 Feb 2026 at 10:35, Jakub Warta=
k &lt;<a href=3D"mailto:jakub.wartak@enterprisedb.com">jakub.wartak@enterpr=
isedb.com</a>&gt; wrote:</div><div class=3D"gmail_quote gmail_quote_contain=
er"><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;bor=
der-left:1px solid rgb(204,204,204);padding-left:1ex">BTW how have you arri=
ved with the &quot;240x&quot; number? We have 16 buckets for each<br>
of the object/context/type.<br></blockquote><div><br></div><div>Sorry, I wo=
rded that poorly. I meant that we store a histogram for each combination, 2=
40 in total.</div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px=
 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
&gt; even then scanning an extra 25KB of mostly zeroes on every commit<br>
&gt; doesn&#39;t seem great. Maybe making the histogram accumulation<br>
&gt; conditional on the counter field being non-zero is enough to avoid any=
<br>
&gt; issues? I haven&#39;t yet constructed a benchmark to see if it&#39;s a=
ctually<br>
&gt; a problem or not. Select only pgbench with small shared buffers and<br=
>
&gt; scale that fits into page cache should be an adversarial use case<br>
&gt; while still being reasonably realistic.<br>
<br>
Earlier I&#39;ve done some benchmarks (please see [1]) based on recommendat=
ions<br>
by Andres to keep low io_combine_limit for that and just tiny shared_buffer=
s.<br>
I&#39;m getting too much noise to derive any results, and as this is relate=
d<br>
to I/O even probably context switches start playing a role there... sadly w=
e<br>
seem not to have a performance farm to answer this.<br></blockquote><div><b=
r></div><div>I glossed over the first benchmark you did. That&#39;s pretty =
close to what I was talking about - exercise the stats collection part by h=
aving ~1 I/O served from page cache per a trivial transaction. And the prew=
arm should exercise the per I/O overhead. If neither of them have any measu=
rable overhead then I can&#39;t think of a workload where it could be worse=
.</div><div><br></div><blockquote class=3D"gmail_quote" style=3D"margin:0px=
 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
TBH, I&#39;m not sure how to progress with this, I mean we could as you say=
:<br>
- reduce PgStat_PendingIO.pending_hist_time_buckets by removing<br>
IOCONTEXT_NUM_TYPES<br>
=C2=A0 (not a big loss, just lack of showing BAS strategy)<br></blockquote>=
<div><br></div><div>I&#39;m on the fence on this. For the actual problems I=
&#39;ve had to diagnose it wouldn&#39;t have mattered. But latency differen=
ces of bulk vs. normal access might be useful for understanding benchmark r=
esults better. A 5x reduction in size is pretty big.</div><div>=C2=A0</div>=
<blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-=
left:1px solid rgb(204,204,204);padding-left:1ex">
- we could even further reduce PgStat_PendingIO.pending_hist_time_buckets<b=
r>
=C2=A0 by removing IOOBJECT_NUM_TYPES, but those are just 3 and they might =
be<br>
=C2=A0 useful<br></blockquote><div><br></div><div>WAL write vs. relation wr=
ite is a very useful distinction for me.</div><div><br></div><blockquote cl=
ass=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid=
 rgb(204,204,204);padding-left:1ex">
... and are You saying to try to do this below thing too?<br>
<br>
@@ -288,8 +290,9 @@ pgstat_io_flush_cb(bool nowait)<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 for(int b =3D 0; b &lt; PGSTAT_IO_HI=
ST_BUCKETS; b++)<br>
-<br>
bktype_shstats-&gt;hist_time_buckets[io_object][io_context][io_op][b] +=3D<=
br>
-<br>
PendingIOStats.pending_hist_time_buckets[io_object][io_context][io_op][b];<=
br>
+<br>
if(PendingIOStats.pending_hist_time_buckets[io_object][io_context][io_op][b=
]<br>
&gt; 0)<br>
+<br>
bktype_shstats-&gt;hist_time_buckets[io_object][io_context][io_op][b] +=3D<=
br>
+<br>
PendingIOStats.pending_hist_time_buckets[io_object][io_context][io_op][b];<=
br>
<br></blockquote><div><br></div><div>I meant this:</div><div><br></div><div=
>@@ -287,6 +287,7 @@ pgstat_io_flush_cb(bool nowait)<br>=C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 bktype_shstats-&gt;times[io_object][io_context][io_op]=
 +=3D<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 I=
NSTR_TIME_GET_MICROSEC(time);<br>=C2=A0<br>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 if (PendingIOStats.counts[io_object][io_context][io_op] &gt; 0)<br>=C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 for(int b =
=3D 0; b &lt; PGSTAT_IO_HIST_BUCKETS; b++)<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 bktype_s=
hstats-&gt;hist_time_buckets[io_object][io_context][io_op][b] +=3D<br>=C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 PendingIOStats.pending_hist_t=
ime_buckets[io_object][io_context][io_op][b];<br></div><div><br></div><div>=
Most object/context/op combinations will have a 0 count, so no point in act=
ually looking at the histogram.</div><div>=C2=A0</div><blockquote class=3D"=
gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(20=
4,204,204);padding-left:1ex">
.. but the main problem, even if I do all of that I won&#39;t be able to<br=
>
reliably measure the impact, probably the best I could say is<br>
&quot;runs good as well as master, +/- 3%&quot;.<br>
<br>
Could you somehow help me with this? I mean should we reduce the scope(remo=
ve<br>
context) and add that &quot;if&quot;?<br></blockquote><div><br></div><div>I=
 think if we only aggregate histograms conditionally, then having a ton of =
different histograms is less of a problem. Only the histograms that have an=
y data will get accessed. The overhead is limited to the memory usage which=
 I think is acceptable.</div><div><br></div><div>I&#39;ll run a few benchma=
rks on what I have available here to see if I can tease out anything more t=
han the no effect with a 3% error margin we have today.</div><div>=C2=A0</d=
iv><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;bord=
er-left:1px solid rgb(204,204,204);padding-left:1ex">
&gt; I&#39;m not familiar enough with the new stats infrastructure to tell<=
br>
&gt; whether it&#39;s a problem, but it seems odd that<br>
&gt; pgstat_flush_backend_entry_io() isn&#39;t modified to aggregate the<br=
>
&gt; histograms.<br>
<br>
Well I&#39;m first time doing this too, and my understanding is that<br>
pgstat_io.c::pgstat_io_flush_cb() is flushing the global statistics<br>
(per backend-type) while the per-individual backend<br>
pgstat_flush_backend_entry_io() (from pgstat_backend.c) is more about<br>
per-PID-backends stats (--&gt; for: select * from pg_stat_get_backend_io(PI=
D)).<br>
<br>
In terms of this patch, the per-backend-PID-I/O histograms are not implemen=
ted<br>
yet, and I&#39;ve raised this question earlier, but I&#39;m starting to bel=
ieve<br>
the answer is probably no, we should not implement those (more overhead<br>
for no apparent benefit, as most of the cases could be tracked down just wi=
th<br>
this overall per-backend-type stats ).<br></blockquote><div><br></div><div>=
I agree that per-PID histograms are probably not worth the extra work. But =
this left me wondering if we are allocating the whole set of histograms too=
 many times. I don&#39;t think every place that uses=C2=A0PgStat_BktypeIO a=
ctually needs the histograms. I will need to dig around to understand this =
code a bit better.</div><div><br></div><div>Regards,</div><div>Ants Aasma</=
div></div></div>

--0000000000009ab9f3064a9dfed9--