Re: pg19b1: TRAP: failed Assert("pgstat_bktype_io_stats_valid(bktype_shstats, MyBackendType)") - Mailing list pgsql-hackers

From Andrey Rachitskiy
Subject Re: pg19b1: TRAP: failed Assert("pgstat_bktype_io_stats_valid(bktype_shstats, MyBackendType)")
Date
Msg-id CAB8bMiuTw+pVqzbBfbMcm=w5v=v4v9dfkbOy41N9cWWQ4iQUMA@mail.gmail.com
Whole thread
In response to Re: pg19b1: TRAP: failed Assert("pgstat_bktype_io_stats_valid(bktype_shstats, MyBackendType)")  (Melanie Plageman <melanieplageman@gmail.com>)
List pgsql-hackers
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@gmail.com>:
Thanks, Justin, for the report

On Thu, Jul 16, 2026 at 11:03 AM Andrey Rachitskiy <pl0h0yp1@gmail.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

pgsql-hackers by date:

Previous
From: Sami Imseih
Date:
Subject: Re: Report index currently being vacuumed in pg_stat_progress_vacuum
Next
From: Andres Freund
Date:
Subject: Re: ci: namespace ccache by PostgreSQL major version