Re: pgBufferUsage.blk_{read|write}_time are zero although there are pgBufferUsage.local_blks_{read|written}

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: Nazir Bilal Yavuz <byavuz81@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Melanie Plageman <melanieplageman@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2023-10-19T05:26:26Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Fix description of I/O timing info for shared buffers in EXPLAIN (BUFFERS)

  2. pg_stat_statements: Add local_blk_{read|write}_time

  3. Add local_blk_{read|write}_time I/O timing statistics for local blocks

  4. Rename I/O timing statistics columns to shared_blk_{read|write}_time

  5. Count write times when extending relation files for shared buffers

  6. Add JIT deform_counter

On Wed, Oct 18, 2023 at 02:56:42PM +0900, Michael Paquier wrote:
> Thanks for the new versions.  I have applied 0001 and backpatched it
> for now.  0002 and 0003 look in much cleaner shape than previously.

0002 and 0003 have now been applied.  I have split 0003 into two parts
at the end, mainly on clarity grounds: one for the counters with
EXPLAIN and a second for pg_stat_statements.

There were a few things in the patch set.  Per my notes:
- Some incorrect indentation.
- The additions of show_buffer_usage() did not handle correctly the
addition of a comma before/after the local timing block.  The code
area for has_local_timing needs to check for has_temp_timing, while
the area of has_shared_timing needs to check for (has_local_timing ||
has_temp_timing).
- explain.sgml was missing an update for the information related to
the read/write timings of the local blocks.

Remains what we should do about the "shared/local" string in
show_buffer_usage() for v16 and v15, as "local" is unrelated to that.
Perhaps we should just switch to "shared" in this case or just remove
the string entirely?  Still that implies changing the output of
EXPLAIN on a stable branch in this case, so there could be an argument
for leaving this stuff alone.
--
Michael