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 via email to