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


Reply via email to