Re: pg19b1: TRAP: failed Assert("pgstat_bktype_io_stats_valid(bktype_shstats, MyBackendType)")

Hi Melanie,
Thanks for the review.

I agree that IOOP_WAIT is not the right approach if read_time is
essentially wait time for asynchronous reads, and that
relaxing pgstat_bktype_io_stats_valid() is not attractive either.

Between the two options you suggested for 19 — pass cnt = 1 for foreign IO
in WaitReadBuffers(), or skip wait-time accounting on that path as in the
patch I sent — I prefer the latter.
Passing cnt = 1 would preserve the wait time, but it would also
inflate reads for backends that only join an in-progress IO and never start
that read.

I think that is worse than omitting the foreign waiter's read_time:
inflating reads would make read_time / reads harder to interpret, while
omitting foreign wait time loses some per-backend attribution but does not
change who started the I/O. reads remains tied to who actually initiated
the IO.

So for 19 I would go with not counting wait time for foreign IOs.

---
Regards,
Rachitskiy Andrey

пт, 17 июл. 2026 г. в 00:59, Melanie Plageman <melanieplageman(at)gmail(dot)com>:

> Thanks, Justin, for the report
>
> On Thu, Jul 16, 2026 at 11:03 AM Andrey Rachitskiy <pl0h0yp1(at)gmail(dot)com>
> wrote:
> >
> > With track_io_timing=on, foreign IO wait time is currently recorded:
> when WaitReadBuffers() waits on foreign_io, it calls
> pgstat_count_io_op_time() with cnt=0. So we do write non-zero IOOP_READ
> time into this backend's pending pg_stat_io stats, without incrementing the
> read count in this backend.
> >
> > Thus, this fix introduces a real change: it disables recording foreign
> IO wait time in pg_stat_io statistics for the backend that is performing
> the wait.
> >
> > What we lose is only that wait time. The initiating backend still
> records the read count and its own IO time, and the waiting backend still
> records a buffer hit once the IO completes.
>
> Yea, so, I don't think we can just not record the wait time for
> foreign IOs. For asynchronous reads, read time in pg_stat_io is almost
> entirely wait time. That's also why it doesn't make sense to have a
> separate IOOP_WAIT -- unless we start measuring and displaying read
> latency (how long the actual reads took) in pg_stat_io and then you
> could separate that from wait time.
>
> My first thought is that we should relax the restriction in
> pgstat_bktype_io_stats_valid() that requires non-zero counts if time
> is non-zero. But that seems silly: 1) we lose lots of validation for
> cases where this would be bogus and 2) anyone looking at the view that
> seems non-zero IO time and 0 count will be very confused and 3) it
> makes any math dividing read time by read operations meaningless
> (though I don't know how meaningful that is now).
>
> If read time is truly just wait time, we could just pass
> pg_stat_count_io_op_time() a count of 1 for foreign IO in
> WaitReadBuffers(). In 19, we should likely either do this or do as
> Andrey suggested and not count the wait time for foreign IOs.
>
> For master, I'd like to do something more satisfying, but I'm not sure
> what.
>
> It did get me thinking that there is 0 way for the user (or developer)
> to know about foreign IOs. Maybe that's okay...?
>
> - Melanie
>