From c9a669492719db2fe45cd17259ae0fc3bc4d91c5 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Fri, 31 Jul 2026 17:45:45 -0400 Subject: [PATCH] Allow IO time to be counted without a matching IO operation in pg_stat_io Since 999dec9ec6a816680, pg_stat_io can show read time with zero reads for an IO Context: a foreign IO is counted as a read only in the initiating backend, while other waiters record only the wait time. That violates pgstat_bktype_io_stats_valid(). Relax the check to allow time without a matching operation count, since we want to count read wait time even in backends that did not initiate the read. This also enables future accounting of waits on IO resources (e.g., AIO handles) in backends that didn't start the IO. Author: Andrey Rachitskiy Reported-by: Justin Pryzby Reviewed-by: Melanie Plageman Reviewed-by: Andrey Borodin Discussion: https://postgr.es/m/ak5lccE4qiQpOBHn@pryzbyj2023 Backpatch-through: 19 --- doc/src/sgml/monitoring.sgml | 4 +++- src/backend/storage/buffer/bufmgr.c | 8 ++++---- src/backend/utils/activity/pgstat_io.c | 22 +++++++--------------- 3 files changed, 14 insertions(+), 20 deletions(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index f1422826d29..099e9b6f4e9 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -3205,7 +3205,9 @@ description | Waiting for a newly initialized WAL file to reach durable storage object is not wal, or if is enabled and object is wal, - otherwise zero) + otherwise zero). This may include time spent waiting for a + read started by another backend. In that case + reads may still be zero. diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 3908529872a..169829eb020 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -1829,8 +1829,9 @@ WaitReadBuffers(ReadBuffersOperation *operation) needed_wait = true; /* - * The IO operation itself was already counted earlier, in - * AsyncReadBuffers(), this just accounts for the wait time. + * This just accounts for the wait time. The IO operation + * itself was already counted earlier in AsyncReadBuffers() -- + * either by us or by another backend if this is a foreign IO. */ pgstat_count_io_op_time(io_object, io_context, IOOP_READ, io_start, 0, 0); @@ -2018,8 +2019,7 @@ AsyncReadBuffers(ReadBuffersOperation *operation, int *nblocks_progress) * A secondary benefit is that this would allow us to measure the time in * pgaio_io_acquire() without causing undue timer overhead in the common, * non-blocking, case. However, currently the pgstats infrastructure - * doesn't really allow that, as it a) asserts that an operation can't - * have time without operations b) doesn't have an API to report + * doesn't really allow that because it doesn't have an API to report * "accumulated" time. */ ioh = pgaio_io_acquire_nb(CurrentResourceOwner, &operation->io_return); diff --git a/src/backend/utils/activity/pgstat_io.c b/src/backend/utils/activity/pgstat_io.c index 4f7a39aaa0e..8ec1aad5078 100644 --- a/src/backend/utils/activity/pgstat_io.c +++ b/src/backend/utils/activity/pgstat_io.c @@ -25,9 +25,10 @@ static bool have_iostats = false; /* * Check that stats have not been counted for any combination of IOObject, - * IOContext, and IOOp which are not tracked for the passed-in BackendType. If - * stats are tracked for this combination and IO times are non-zero, counts - * should be non-zero. + * IOContext, and IOOp which are not tracked for the passed-in BackendType. + * Non-zero time with a zero operation count is allowed as there are cases + * where this may be appropriate -- like when a backend is waiting on IO + * initiated by another backend. * * The passed-in PgStat_BktypeIO must contain stats from the BackendType * specified by the second parameter. Caller is responsible for locking the @@ -43,19 +44,10 @@ pgstat_bktype_io_stats_valid(PgStat_BktypeIO *backend_io, { for (int io_op = 0; io_op < IOOP_NUM_TYPES; io_op++) { - /* we do track it */ - if (pgstat_tracks_io_op(bktype, io_object, io_context, io_op)) - { - /* ensure that if IO times are non-zero, counts are > 0 */ - if (backend_io->times[io_object][io_context][io_op] != 0 && - backend_io->counts[io_object][io_context][io_op] <= 0) - return false; - - continue; - } - /* we don't track it, and it is not 0 */ - if (backend_io->counts[io_object][io_context][io_op] != 0) + if (!pgstat_tracks_io_op(bktype, io_object, io_context, io_op) && + (backend_io->counts[io_object][io_context][io_op] != 0 || + backend_io->times[io_object][io_context][io_op] != 0)) return false; } } -- 2.47.3