Horton

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

Nazir Bilal Yavuz <byavuz81@gmail.com>

View thread
From: Nazir Bilal Yavuz <byavuz81@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Robert Haas <robertmhaas@gmail.com>, Melanie Plageman <melanieplageman@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2023-10-17T13:44:25Z
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

Attachments

Hi,

Thanks for the review!

On Tue, 17 Oct 2023 at 11:40, Michael Paquier <michael@paquier.xyz> wrote:
>
> On Mon, Oct 16, 2023 at 01:07:07PM +0300, Nazir Bilal Yavuz wrote:
> > Yes, that could be a better solution. Also, having more detailed stats
> > for shared and local buffers is helpful. I updated patches in line
> > with that:
> >
> > 0001: Counts extends same way as a write.
>
> It can change existing query results on an already-released branch,
> but we already count the number of blocks when doing a relation
> extension, so counting the write time is something I'd rather fix in
> v16.  If you have any objections, let me know.

I agree.

>
> > 0002: Rename blk_{read|write}_time as shared_blk_{read|write}_time.
>
> Note that `git diff --check` complains here.
>
> --- a/contrib/pg_stat_statements/pg_stat_statements--1.1--1.2.sql
> +++ b/contrib/pg_stat_statements/pg_stat_statements--1.1--1.2.sql
> @@ -30,8 +30,8 @@ CREATE FUNCTION pg_stat_statements(IN showtext boolean,
>      OUT local_blks_written int8,
>      OUT temp_blks_read int8,
>      OUT temp_blks_written int8,
> -    OUT blk_read_time float8,
> -    OUT blk_write_time float8
> +    OUT shared_blk_read_time float8,
> +    OUT shared_blk_write_time float8
>
> Doing that in an extension upgrade script is incorrect.  These should
> not be touched.
>
> -     Total time the statement spent reading data file blocks, in milliseconds
> +     Total time the statement spent reading shared data file blocks, in milliseconds
>
> Or just shared blocks?  That's what we use elsewhere for
> pg_stat_statements.  "shared data file blocks" sounds a bit confusing
> for relation file blocks read/written from/to shared buffers.
>
> > 0003: Add new local_blk_{read|write}_time variables.
>
>  DATA = pg_stat_statements--1.4.sql \
> +       pg_stat_statements--1.11--1.12.sql \
>         pg_stat_statements--1.10--1.11.sql \
>
> There is no need to bump again pg_stat_statements, as it has already
> been bumped to 1.11 on HEAD per the recent commit 5a3423ad8ee1 from
> Daniel.  So the new changes can just be added to 1.11.

I updated patches based on your comments. v4 is attached.

Regards,
Nazir Bilal Yavuz
Microsoft