Re: Remove io prefix from pg_stat_io columns
On 2023-04-21 07:38:01 +0900, Michael Paquier wrote: > On Thu, Apr 20, 2023 at 11:38:42AM +0900, Michael Paquier wrote: > > No worries, committers should take care of that. > > Done as of 0ecb87e, as I can keep an eye on the buildfarm today, with > a catversion bump. Thanks!
Re: Remove io prefix from pg_stat_io columns
On Thu, Apr 20, 2023 at 11:38:42AM +0900, Michael Paquier wrote: > No worries, committers should take care of that. Done as of 0ecb87e, as I can keep an eye on the buildfarm today, with a catversion bump. -- Michael signature.asc Description: PGP signature
Re: Remove io prefix from pg_stat_io columns
At Thu, 20 Apr 2023 10:13:04 +0900, Michael Paquier wrote in > On Wed, Apr 19, 2023 at 08:50:13PM -0400, Melanie Plageman wrote: > > I thought about changing parameter and local variable names to remove > > the prefix, but in the original discussion folks seemed to think it made > > sense to leave the "C level" references with an "io" prefix. I think we > > could change many of them, but some of them may be required for clarity. > > I agree with the feeling of not touching the internal variables. It > makes them easier to grep, and it seems that these are mostly on lines > where there is little context about what they refer to.. I find the names for local loop variables are a bit annoying, but I don't feel strongly about removing the prifix there. I'm also not in favor of removing the prefix in other cases, bacause it helps with grep'ability. > if (backend_io->times[io_object][io_context][io_op] != 0 && > backend_io->counts[io_object][io_context][io_op] <= 0) > Perhaps others have comments or objections, so let's wait a bit, but > I'd be OK to apply this one myself, with a catversion bump. (Happy to > help.) So, I don't have any issues with the patch overall. From what I can tell, there are no remaining instances of io_foobar that need to be rewritten. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
Re: Remove io prefix from pg_stat_io columns
On Wed, Apr 19, 2023 at 09:45:32PM -0400, Melanie Plageman wrote: > Great, thanks! Once you feel an appropriate amount of time has passed, > it would be great if you could apply it. Sure. Probably on tomorrow morning, or Monday in the worst-case scenario, I think.. > I forgot to add a note about the catalog version bump. oops! No worries, committers should take care of that. -- Michael signature.asc Description: PGP signature
Re: Remove io prefix from pg_stat_io columns
On Thu, Apr 20, 2023 at 10:13:04AM +0900, Michael Paquier wrote: > On Wed, Apr 19, 2023 at 08:50:13PM -0400, Melanie Plageman wrote: > > I thought about changing parameter and local variable names to remove > > the prefix, but in the original discussion folks seemed to think it made > > sense to leave the "C level" references with an "io" prefix. I think we > > could change many of them, but some of them may be required for clarity. > > I agree with the feeling of not touching the internal variables. It > makes them easier to grep, and it seems that these are mostly on lines > where there is little context about what they refer to.. > > Perhaps others have comments or objections, so let's wait a bit, but > I'd be OK to apply this one myself, with a catversion bump. (Happy to > help.) Great, thanks! Once you feel an appropriate amount of time has passed, it would be great if you could apply it. I forgot to add a note about the catalog version bump. oops! - Melanie
Re: Remove io prefix from pg_stat_io columns
On Wed, Apr 19, 2023 at 08:50:13PM -0400, Melanie Plageman wrote: > I thought about changing parameter and local variable names to remove > the prefix, but in the original discussion folks seemed to think it made > sense to leave the "C level" references with an "io" prefix. I think we > could change many of them, but some of them may be required for clarity. I agree with the feeling of not touching the internal variables. It makes them easier to grep, and it seems that these are mostly on lines where there is little context about what they refer to.. Perhaps others have comments or objections, so let's wait a bit, but I'd be OK to apply this one myself, with a catversion bump. (Happy to help.) -- Michael signature.asc Description: PGP signature
Re: Remove io prefix from pg_stat_io columns
On Wed, Apr 19, 2023 at 8:42 PM Michael Paquier wrote: > > On Wed, Apr 19, 2023 at 01:54:21PM -0300, Fabrízio de Royes Mello wrote: > > On Wed, Apr 19, 2023 at 1:27 PM Melanie Plageman > > wrote: > >> Over in [1], we discussed removing the "io" prefix from the columns > >> "io_context" and "io_object" in pg_stat_io since they seem redundant > >> given the view name > > > > LGTM. All tests passed and were built without warnings. > > There are a lot of internal references to both of them mainly around > the buffer manager and the pgstat code, still I agree that the view > feels redundant as currently written, so agreed. It does not seem > like you have missed any references here, from what I can see. I thought about changing parameter and local variable names to remove the prefix, but in the original discussion folks seemed to think it made sense to leave the "C level" references with an "io" prefix. I think we could change many of them, but some of them may be required for clarity. - Melanie
Re: Remove io prefix from pg_stat_io columns
On Wed, Apr 19, 2023 at 01:54:21PM -0300, Fabrízio de Royes Mello wrote: > On Wed, Apr 19, 2023 at 1:27 PM Melanie Plageman > wrote: >> Over in [1], we discussed removing the "io" prefix from the columns >> "io_context" and "io_object" in pg_stat_io since they seem redundant >> given the view name > > LGTM. All tests passed and were built without warnings. There are a lot of internal references to both of them mainly around the buffer manager and the pgstat code, still I agree that the view feels redundant as currently written, so agreed. It does not seem like you have missed any references here, from what I can see. -- Michael signature.asc Description: PGP signature
Re: Remove io prefix from pg_stat_io columns
On Wed, Apr 19, 2023 at 1:27 PM Melanie Plageman wrote: > > Hi, > > Over in [1], we discussed removing the "io" prefix from the columns > "io_context" and "io_object" in pg_stat_io since they seem redundant > given the view name > LGTM. All tests passed and were built without warnings. Regards -- Fabrízio de Royes Mello
Remove io prefix from pg_stat_io columns
Hi, Over in [1], we discussed removing the "io" prefix from the columns "io_context" and "io_object" in pg_stat_io since they seem redundant given the view name Attached patch does that. - Melanie [1] https://www.postgresql.org/message-id/20230215.164021.227543675435826022.horikyota.ntt%40gmail.com From 632102f3a9ad5f0246d544c0e29c85915ccde495 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Wed, 19 Apr 2023 11:43:13 -0400 Subject: [PATCH v1] Remove io prefix from pg_stat_io columns a9c70b46 added the statistics view pg_stat_io which contained columns "io_context" and "io_object". Given that the columns are in the pg_stat_io view, the "io" prefix is somewhat redundant. Remove this prefix. Discussion: https://postgr.es/m/20230215.164021.227543675435826022.horikyota.ntt%40gmail.com --- contrib/amcheck/expected/check_heap.out | 4 +-- contrib/amcheck/sql/check_heap.sql | 4 +-- doc/src/sgml/monitoring.sgml| 22 src/backend/catalog/system_views.sql| 4 +-- src/backend/utils/adt/pgstatfuncs.c | 8 +++--- src/include/catalog/pg_proc.dat | 2 +- src/test/regress/expected/rules.out | 6 ++--- src/test/regress/expected/stats.out | 34 - src/test/regress/sql/stats.sql | 34 - 9 files changed, 59 insertions(+), 59 deletions(-) diff --git a/contrib/amcheck/expected/check_heap.out b/contrib/amcheck/expected/check_heap.out index e4785141a6..8f1beb4681 100644 --- a/contrib/amcheck/expected/check_heap.out +++ b/contrib/amcheck/expected/check_heap.out @@ -80,7 +80,7 @@ INSERT INTO heaptest (a, b) SET allow_in_place_tablespaces = true; CREATE TABLESPACE regress_test_stats_tblspc LOCATION ''; SELECT sum(reads) AS stats_bulkreads_before - FROM pg_stat_io WHERE io_context = 'bulkread' \gset + FROM pg_stat_io WHERE context = 'bulkread' \gset ALTER TABLE heaptest SET TABLESPACE regress_test_stats_tblspc; -- Check that valid options are not rejected nor corruption reported -- for a non-empty table @@ -114,7 +114,7 @@ SELECT pg_stat_force_next_flush(); (1 row) SELECT sum(reads) AS stats_bulkreads_after - FROM pg_stat_io WHERE io_context = 'bulkread' \gset + FROM pg_stat_io WHERE context = 'bulkread' \gset SELECT :stats_bulkreads_after > :stats_bulkreads_before; ?column? -- diff --git a/contrib/amcheck/sql/check_heap.sql b/contrib/amcheck/sql/check_heap.sql index 6794ca4eb0..cf5ce4d0c0 100644 --- a/contrib/amcheck/sql/check_heap.sql +++ b/contrib/amcheck/sql/check_heap.sql @@ -40,7 +40,7 @@ INSERT INTO heaptest (a, b) SET allow_in_place_tablespaces = true; CREATE TABLESPACE regress_test_stats_tblspc LOCATION ''; SELECT sum(reads) AS stats_bulkreads_before - FROM pg_stat_io WHERE io_context = 'bulkread' \gset + FROM pg_stat_io WHERE context = 'bulkread' \gset ALTER TABLE heaptest SET TABLESPACE regress_test_stats_tblspc; -- Check that valid options are not rejected nor corruption reported @@ -55,7 +55,7 @@ SELECT * FROM verify_heapam(relation := 'heaptest', startblock := 0, endblock := -- causing an additional bulkread, which should be reflected in pg_stat_io. SELECT pg_stat_force_next_flush(); SELECT sum(reads) AS stats_bulkreads_after - FROM pg_stat_io WHERE io_context = 'bulkread' \gset + FROM pg_stat_io WHERE context = 'bulkread' \gset SELECT :stats_bulkreads_after > :stats_bulkreads_before; CREATE ROLE regress_heaptest_role; diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 2903b67170..99f7f95c39 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -3754,7 +3754,7 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i -io_object text +object text Target object of an I/O operation. Possible values are: @@ -3777,7 +3777,7 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i -io_context text +context text The context of an I/O operation. Possible values are: @@ -3786,10 +3786,10 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i normal: The default or standard - io_context for a type of I/O operation. For + context for a type of I/O operation. For example, by default, relation data is read into and written out from shared buffers. Thus, reads and writes of relation data to and from - shared buffers are tracked in io_context + shared buffers are tracked in context normal. @@ -3798,7 +3798,7 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i vacuum: I/O operations performed outside of shared buffers while vacuuming and analyzing permanen
