Subject: Re: compute_query_id and pg_stat_statements
To: Julien Rouhaud <rjuju123@gmail.com>, Bruce Momjian <bruce@momjian.us>
Cc: Pavel Stehule <pavel.stehule@gmail.com>,
 Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
 Magnus Hagander <magnus@hagander.net>, Michael Paquier
 <michael@paquier.xyz>, Andres Freund <andres@anarazel.de>,
 Tom Lane <tgl@sss.pgh.pa.us>, Alvaro Herrera <alvherre@alvh.no-ip.org>,
 Stephen Frost <sfrost@snowman.net>, Christoph Berg <myon@debian.org>,
 Peter Eisentraut <peter.eisentraut@enterprisedb.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
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>
From: Fujii Masao <masao.fujii@oss.nttdata.com>
Message-ID: <e3d3e3e5-0b33-5165-9121-6e770830e609@oss.nttdata.com>
Date: Thu, 13 May 2021 13:18:05 +0900
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:78.0)
 Gecko/20100101 Thunderbird/78.10.1
MIME-Version: 1.0
In-Reply-To: <20210513040342.fouh7ryglrfgxglu@nol>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/e3d3e3e5-0b33-5165-9121-6e770830e609%40oss.nttdata.com>
Precedence: bulk


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