faulty error handling around pgstat_count_io_op_time()
Peter Eisentraut <peter@eisentraut.org>
From: Peter Eisentraut <peter@eisentraut.org>
To: pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2026-06-12T16:01:45Z
Lists: pgsql-hackers
Attachments
- 0001-Mark-up-faulty-error-handling-around-pgstat_count_io.patch (text/plain) patch 0001
There are several places where the return value of pg_pread() or pg_pwrite() is passed directly as the byte count to pgstat_count_io_op_time(). The bytes argument of pgstat_count_io_op_time() is of type uint64, and so error returns of -1 are going to passed as UINT64_MAX and added as such to the internal statistics. In the attached patch, I have marked up those places. I think the correction here would be to move the pgstat_count_io_op_time() calls to after the error returns are handled. This is effectively how most other code already behaves. For example, most smgr calls don't return on error, so you don't get a chance to make any pgstat calls afterwards. It's only the open-coded places where we can even do that. However, XLogPageRead() even goes out of its way to make an explicit pgstat_count_io_op_time() call in the error branch. I suppose this could be useful to record short reads, but a) this particular instance is still faulty regarding -1, and b) other places don't do that. So it's a bit unclear what the preferred behavior on error should be. An alternative would be to call pgstat_count_io_op_time() with like Max(byteswritten, 0), but that seems kind of ugly. Another alternative would be to change the bytes argument of pgstat_count_io_op_time() to ssize_t. POSIX file system operations can't operate on sizes larger than ssize_t, so this type should be sufficient. And then error returns could be handled centrally in pgstat_count_io_op_time(). (Record them, don't record them, or even count errors separately, etc.) Thoughts?
Commits
-
Fix pgstat_count_io_op_time() calls passing incorrect information
- 13f940b4b56f 18 (unreleased) landed
- 3048e81308f9 19 (unreleased) landed