From: Fujii Masao <masao.fujii@oss.nttdata.com>
To: Julien Rouhaud <rjuju123@gmail.com>
To: Bruce Momjian <bruce@momjian.us>
Cc: Pavel Stehule <pavel.stehule@gmail.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: Magnus Hagander <magnus@hagander.net>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Andres Freund <andres@anarazel.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Stephen Frost <sfrost@snowman.net>
Cc: Christoph Berg <myon@debian.org>
Cc: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: compute_query_id and pg_stat_statements
Date: Thu, 13 May 2021 13:18:05 +0900
Message-ID: <e3d3e3e5-0b33-5165-9121-6e770830e609@oss.nttdata.com> (raw)
In-Reply-To: <20210513040342.fouh7ryglrfgxglu@nol>
References: <20210512.183927.600535883807655721.horikyota.ntt@gmail.com>
<CAFj8pRCfxtRCoT10wyDAGq6cuW3oVfEJ_JUjEGW_jBs=tpPYnA@mail.gmail.com>
<20210512095149.kqbnhe72uijfwi2r@nol>
<20210513003618.GA26043@momjian.us>
<20210513005236.aj7n6b76yjsxythc@nol>
<20210513011325.GA11075@momjian.us>
<20210513015700.fp33q5rfvwl2ahfx@nol>
<20210513030652.GB11075@momjian.us>
<20210513031613.j47baglf6xl3mgbs@nol>
<20210513033332.GF11075@momjian.us>
<20210513040342.fouh7ryglrfgxglu@nol>
On 2021/05/13 13:03, Julien Rouhaud wrote:
> On Wed, May 12, 2021 at 11:33:32PM -0400, Bruce Momjian wrote:
>> On Thu, May 13, 2021 at 11:16:13AM +0800, Julien Rouhaud wrote:
>>> On Wed, May 12, 2021 at 11:06:52PM -0400, Bruce Momjian wrote:
>>>> On Thu, May 13, 2021 at 09:57:00AM +0800, Julien Rouhaud wrote:
>>>>
>>>>> source? What if you have for instance pg_stat_statements, pg_stat_kcache,
>>>>> pg_store_plans and pg_wait_sampling installed? All those extensions need a
>>>>> query_id (or at least benefit from it for pg_wait_sampling), is there any value
>>>>> to give a full list of all the modules that would enable compute_query_id?
>>>>
>>>> Well, we don't have any other cases where the source of the change is
>>>> this indeterminate, so I don't really have a suggestion. I think this
>>>> does highlight another case where 'auto' just isn't a good fit for our
>>>> API.
>>>
>>> It may depends on your next question
>>>
>>>>> I'm assuming that anyone wanting to install any of those extensions (or any
>>>>> similar one) is fully aware that they aggregate metrics based on at least a
>>>>> query_id. If they don't, well they probably never read any documentation since
>>>>> postgres 9.2 which introduced query normalization, and I doubt that they will
>>>>> be interested in having access to the information anyway.
>>>>
>>>> My point is that we are changing a setting from an extension, and if you
>>>> look in pg_setting, it will say "default"?
>>>
>>> No, it will say "on". What the patch I sent implements is:
>>
>> I was asking what pg_settings.source will say:
>>
>> SELECT name, soource FROM pg_settings;
>
> Oh sorry. Here's the full output before and after a dynamic call to
> queryIsWanted():
>
> name | compute_query_id
> setting | auto
> unit | <NULL>
> category | Statistics / Monitoring
> short_desc | Compute query identifiers.
> extra_desc | <NULL>
> context | superuser
> vartype | enum
> source | default
> min_val | <NULL>
> max_val | <NULL>
> enumvals | {auto,on,off}
> boot_val | auto
> reset_val | auto
> sourcefile | <NULL>
> sourceline | <NULL>
> pending_restart | f
>
> =# LOAD 'pg_qualstats'; /* will call queryIsWanted() */
> WARNING: 01000: Without shared_preload_libraries, only current backend stats will be available.
> LOAD
>
> name | compute_query_id
> setting | on
> unit | <NULL>
> category | Statistics / Monitoring
> short_desc | Compute query identifiers.
> extra_desc | <NULL>
> context | superuser
> vartype | enum
> source | default
> min_val | <NULL>
> max_val | <NULL>
> enumvals | {auto,on,off}
> boot_val | auto
> reset_val | auto
> sourcefile | <NULL>
> sourceline | <NULL>
> pending_restart | f
>
> So yes, it says "default" (and it's the same if the change is done during
> postmaster init). It can be fixed if that's the only issue.
I like leaving compute_query_id=auto instead of overwriting it to "on",
even when queryIsWanted() is called, as I told upthread. Then we can decide
that query id computation is necessary when compute_query_id=auto and
the special flag is enabled by queryIsWanted(). Of course as you and Magnus
discussed upthread, the issue in EXEC_BACKEND case should be fixed,
maybe by using save_backend_variables() or something, though.
If we do this, compute_query_id=auto seems to be similar to huge_pages=try.
When huge_pages=try, whether huge pages is actually used is defined depending
outside PostgreSQL (i.e, OS setting in this case). Neither pg_settings.setting nor
souce are not changed in that case.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: masao.fujii@oss.nttdata.com, rjuju123@gmail.com, bruce@momjian.us, pavel.stehule@gmail.com, horikyota.ntt@gmail.com, magnus@hagander.net, michael@paquier.xyz, andres@anarazel.de, tgl@sss.pgh.pa.us, alvherre@alvh.no-ip.org, sfrost@snowman.net, myon@debian.org, peter.eisentraut@enterprisedb.com, pgsql-hackers@lists.postgresql.org
Subject: Re: compute_query_id and pg_stat_statements
In-Reply-To: <e3d3e3e5-0b33-5165-9121-6e770830e609@oss.nttdata.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox