Hi,
On 2026-09-18 23:07:07 -0400, shihao zhong wrote:
> @@ -1824,6 +1824,7 @@ WaitReadBuffers(ReadBuffersOperation *operation)
> !pgaio_wref_check_done(&operation->io_wref))
> {
> instr_time io_start =
> pgstat_prepare_io_time(track_io_timing);
> + instr_time io_time;
>
> pgaio_wref_wait(&operation->io_wref);
> needed_wait = true;
> @@ -1833,8 +1834,10 @@ WaitReadBuffers(ReadBuffersOperation *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);
> + io_time = pgstat_count_io_op_time(io_object,
> io_context, IOOP_READ,
> +
> io_start, 0, 0);
> +
> pgstat_count_tablespace_blk_read_time(operation->smgr->smgr_rlocator.locator.spcOid,
> +
> io_time);
> }
> else
> {
I am not on board to add duplicate counting to add these places, particularly
not if that requires a hash table lookup every single time.
*If* we want this, it really needs to be designed to avoid all this duplicate
work, both in the amount of code added to a bunch of places, and in the amount
of work that's needed for stats lookups.
Greetings,
Andres Freund