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

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

  1. Fix pgstat_count_io_op_time() calls passing incorrect information