Re: shared-memory based stats collector - v70

From: Andres Freund <andres(at)anarazel(dot)de>
To: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
Cc: melanieplageman(at)gmail(dot)com, pryzby(at)telsasoft(dot)com, thomas(dot)munro(at)gmail(dot)com, david(dot)g(dot)johnston(at)gmail(dot)com, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: shared-memory based stats collector - v70
Date: 2022-04-07 01:58:52
Message-ID: 20220407015852.7xehapb2z7iebqpt@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 2022-04-07 10:36:30 +0900, Kyotaro Horiguchi wrote:
> At Wed, 6 Apr 2022 09:04:09 -0700, Andres Freund <andres(at)anarazel(dot)de> wrote in
> > > + * Don't define an INVALID value so switch() statements can warn if some
> > > + * cases aren't covered. But define the first member to 1 so that
> > > + * uninitialized values can be detected more easily.
> > >
> > > FWIW, I like this.
> >
> > I think there's no switches left now, so it's not actually providing too much.
>
> (Ouch!)

I think it's great that there's no switches left - means we're pretty close to
pgstat being runtime extensible...

> > > 0010:
> > > (I didn't look this closer. The comments arised while looking other
> > > patches.)
> > >
> > > +pgstat_kind_from_str(char *kind_str)
> > >
> > > I don't think I like "str" so much. Don't we spell it as
> > > "pgstat_kind_from_name"?
> >
> > name makes me think of NameData. What do you dislike about str? We seem to use
> > str in plenty places?
>
> For clarity, I don't dislike it so much. So, I'm fine with the
> current name.
>
> I found that you meant a type by the "str". I thought it as an
> instance (I'm not sure I can express my feeling correctly here..) and
> the following functions were in my mind.
>
> char *get_namespace/rel/collation/func_name(Oid someoid)
> char *pgstat_slru_name(int slru_idx)
>
> Another instance of the same direction is
>
> ForkNumber forkname_to_number(const char *forkName)

It's now pgstat_get_kind_from_str().

It was harder to see earlier (I certainly didn't really see it) - because
there were so many "violations" - but most of pgstat is
pgstat_<verb>_<subject>() or just <verb>_<subject>. I'd already moved most of
the patch series over to that (maybe in v68 or so). Now I also did that with
the internal functions.

There's a few functions breaking that pattern, partially because I added them
:(, but since they're not touched in these patches I've not renamed them. But
it's probably worth doing so tomorrow.

Greetings,

Andres Freund

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2022-04-07 02:36:37 Re: Window Function "Run Conditions"
Previous Message Robert Haas 2022-04-07 01:39:22 Re: Should pg_dumpall dump ALTER SYSTEM settings?