From: | Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, Melanie Plageman <melanieplageman(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: pgBufferUsage.blk_{read|write}_time are zero although there are pgBufferUsage.local_blks_{read|written} |
Date: | 2023-10-17 13:44:25 |
Message-ID: | CAN55FZ1==nG6d3WgqzWz_py1iV2ZiZQxDeVrXdJqkpOm7C+hsg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
Thanks for the review!
On Tue, 17 Oct 2023 at 11:40, Michael Paquier <michael(at)paquier(dot)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
Attachment | Content-Type | Size |
---|---|---|
v4-0001-Include-IOOp-IOOP_EXTENDs-while-calculating-block.patch | text/x-diff | 1.0 KB |
v4-0002-Rename-I-O-timing-statistics-columns-to-shared_bl.patch | text/x-diff | 17.1 KB |
v4-0003-Add-local_blk_-read-write-_time-I-O-timing-statis.patch | text/x-diff | 12.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2023-10-17 13:52:36 | Re: run pgindent on a regular basis / scripted manner |
Previous Message | Imseih (AWS), Sami | 2023-10-17 13:43:54 | Re: False "pg_serial": apparent wraparound” in logs |