Hi,
On Fri, Jun 12, 2026 at 06:01:45PM +0200, Peter Eisentraut wrote:
> 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.
Nice catch!
> In the attached patch, I have marked up those places.
I agree with those places and did not find others.
> 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.
Sounds reasonable to me and done that way in the attached.
> 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.
What about keeping the intent (record short reads) by discarding r <= 0? (done
in the attached).
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
