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 <[email protected]>: > Thanks, Justin, for the report > > On Thu, Jul 16, 2026 at 11:03 AM Andrey Rachitskiy <[email protected]> > 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 >
