Re: Planning counters in pg_stat_statements (using pgss_store) - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Planning counters in pg_stat_statements (using pgss_store)
Date
Msg-id 42557.1627229005@sss.pgh.pa.us
Whole thread Raw
In response to Re: Planning counters in pg_stat_statements (using pgss_store)  (Fujii Masao <masao.fujii@oss.nttdata.com>)
Responses Re: Planning counters in pg_stat_statements (using pgss_store)  (Julien Rouhaud <rjuju123@gmail.com>)
Re: Planning counters in pg_stat_statements (using pgss_store)  (Andrew Dunstan <andrew@dunslane.net>)
List pgsql-hackers
[ blast from the past department ]

Fujii Masao <masao.fujii@oss.nttdata.com> writes:
> Finally I pushed the patch!
> Many thanks for all involved in this patch!

It turns out that the regression test outputs from this patch are
unstable under debug_discard_caches (nee CLOBBER_CACHE_ALWAYS).
You can easily check this in HEAD or v14, with something along
the lines of

$ cd ~/pgsql/contrib/pg_stat_statements
$ echo "debug_discard_caches = 1" >/tmp/temp_config
$ TEMP_CONFIG=/tmp/temp_config make check

and what you will get is a diff like this:

 SELECT query, plans, calls, rows FROM pg_stat_statements ORDER BY query COLLATE "C";
                        query                         | plans | calls | rows
...
- PREPARE prep1 AS SELECT COUNT(*) FROM test          |     2 |     4 |    4
+ PREPARE prep1 AS SELECT COUNT(*) FROM test          |     4 |     4 |    4

The reason we didn't detect this long since is that the buildfarm
client script fails to run "make check" for contrib modules that
are marked NO_INSTALLCHECK, so that pg_stat_statements (among
others) has received precisely zero buildfarm testing.  Buildfarm
member sifaka is running an unreleased version of the script that
fixes that oversight, and when I experimented with turning on
debug_discard_caches, I got this failure, as shown at [1].

The cause of the failure of course is that cache clobbering includes
plan cache clobbering, so that the prepared statement's plan is
remade each time it's used, not only twice as the test expects.
However, remembering that cache flushes can happen for other reasons,
it's my guess that this test case would prove unstable in the buildfarm
even without considering the CLOBBER_CACHE_ALWAYS members.  For example,
a background autovacuum hitting the "test" table at just the right time
would result in extra planning.  We haven't seen that because the
buildfarm's not running this test, but that's about to change.

So AFAICS this test is inherently unstable and there is no code bug
to be fixed.  We could drop the "plans" column from this query, or
print something approximate like "plans > 0 AND plans <= calls".
Thoughts?

            regards, tom lane

[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=sifaka&dt=2021-07-24%2023%3A53%3A52



pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Rename of triggers for partitioned tables
Next
From: Yura Sokolov
Date:
Subject: Re: [PoC] Improve dead tuple storage for lazy vacuum