Re: compute_query_id and pg_stat_statements

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Christoph Berg <myon(at)debian(dot)org>, Julien Rouhaud <rjuju123(at)gmail(dot)com>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Magnus Hagander <magnus(at)hagander(dot)net>, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Andres Freund <andres(at)anarazel(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: compute_query_id and pg_stat_statements
Date: 2021-05-13 18:07:06
Message-ID: 20210513180706.GG20766@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Greetings,

* Bruce Momjian (bruce(at)momjian(dot)us) wrote:
> On Thu, May 13, 2021 at 07:39:45PM +0200, Christoph Berg wrote:
> > Re: Bruce Momjian
> > > Well, now that we have clear warnings when it is misconfigured,
> > > especially when querying the pg_stat_statements view, are these
> > > complaints still valid? Also, how is modifying
> > > shared_preload_libraries zero-config, but modifying
> > > shared_preload_libraries and compute_query_id a huge burden?
> >
> > It's zero-config in the sense that if you want to have
> > pg_stat_statements, loading that module via shared_preload_libraries
> > is just natural.

Not sure about natural but it's certainly what folks have at least
become used to. We should be working to eliminate it though.

> > Having to set compute_query_id isn't natural. It's a setting with a
> > completely different name, and the connection of pg_stat_statements to
> > compute_query_id isn't obvious.
> >
> > The reasoning with "we have warnings and stuff" might be ok if
> > pg_stat_statements were a new thing, but it has worked via
> > shared_preload_libraries only for the last decade, and requiring
> > something extra will confuse probably every single user of
> > pg_stat_statements out there.

As we keep seeing, over and over. The ongoing comments claiming that
it's "just" a minor additional configuration tweak fall pretty flat when
you consider the number of times it's already been brought up, and who
it has been brought up by.

> > Perhaps worse, note that these warnings will likely first be seen by
> > the end users of databases, not by the admin performing the initial
> > setup or upgrade, who will not be able to fix the problem themselves.

I don't think this is appreciated anywhere near well enough. This takes
existing perfectly working configurations and actively breaks them in a
manner that isn't obvious and isn't something that an admin would have
any idea about until after they've upgraded and then started trying to
query the view. That's pretty terrible.

> Well, but doing this extra configuration, the query id shows up in a lot
> more places. I can't imagine how this could be done cleanly without
> requiring extra configuration, unless the query_id computation was
> cheaper to compute and we could enable it by default.

There's a ridiculously simple option here which is: drop the idea that
we support an extension redefining the query id and then just make it
on/off with the default to be 'on'. If people actually have a problem
with it being on and they don't want to use pg_stat_statements then they
can turn it off. This won't break any existing configs that are out
there in the field and avoids the complexity of having some kind of
'auto' config. I do agree with the general idea of wanting to be
extensible but I'm not convinced that, in this particular case, it's
worth all of this. I'm somewhat convinced that having a way to disable
the query id is useful in limited cases and if people want a way to do
that, then we can give that to them in a straightfoward way that doens't
break things.

Thanks,

Stephen

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2021-05-13 18:22:16 Re: amvalidate(): cache lookup failed for operator class 123
Previous Message Bruce Momjian 2021-05-13 17:59:47 Re: compute_query_id and pg_stat_statements