Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Remove io prefix from pg_stat_io columns

  1. Remove io prefix from pg_stat_io columns

    Melanie Plageman <melanieplageman@gmail.com> — 2023-04-19T16:26:43Z

    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

  2. Re: Remove io prefix from pg_stat_io columns

    Fabrízio de Royes Mello <fabriziomello@gmail.com> — 2023-04-19T16:54:21Z

    On Wed, Apr 19, 2023 at 1:27 PM Melanie Plageman <melanieplageman@gmail.com>
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

  3. Re: Remove io prefix from pg_stat_io columns

    Michael Paquier <michael@paquier.xyz> — 2023-04-20T00:42:08Z

    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 <melanieplageman@gmail.com> 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

  4. Re: Remove io prefix from pg_stat_io columns

    Melanie Plageman <melanieplageman@gmail.com> — 2023-04-20T00:50:13Z

    On Wed, Apr 19, 2023 at 8:42 PM Michael Paquier <michael@paquier.xyz> 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 <melanieplageman@gmail.com> 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

  5. Re: Remove io prefix from pg_stat_io columns

    Michael Paquier <michael@paquier.xyz> — 2023-04-20T01:13:04Z

    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

  6. Re: Remove io prefix from pg_stat_io columns

    Melanie Plageman <melanieplageman@gmail.com> — 2023-04-20T01:45:32Z

    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

  7. Re: Remove io prefix from pg_stat_io columns

    Michael Paquier <michael@paquier.xyz> — 2023-04-20T02:38:42Z

    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

  8. Re: Remove io prefix from pg_stat_io columns

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2023-04-20T04:24:02Z

    At Thu, 20 Apr 2023 10:13:04 +0900, Michael Paquier <michael@paquier.xyz> 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

  9. Re: Remove io prefix from pg_stat_io columns

    Michael Paquier <michael@paquier.xyz> — 2023-04-20T22:38:01Z

    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

  10. Re: Remove io prefix from pg_stat_io columns

    Andres Freund <andres@anarazel.de> — 2023-04-24T22:15:59Z

    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!