faulty error handling around pgstat_count_io_op_time()

From: Peter Eisentraut <peter(at)eisentraut(dot)org>
To: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: faulty error handling around pgstat_count_io_op_time()
Date: 2026-06-12 16:01:45
Message-ID: 0db864e6-4477-4eba-b2be-d3523cc86564@eisentraut.org
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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?

Attachment Content-Type Size
0001-Mark-up-faulty-error-handling-around-pgstat_count_io.patch text/plain 2.5 KB

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2026-06-12 16:10:39 Re: s/pg_attribute_always_inline/pg_always_inline/?
Previous Message Haoyu Huang 2026-06-12 15:51:16 Re: Better shared data structure management and resizable shared data structures