Re: Planning counters in pg_stat_statements (using pgss_store)

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
Cc: Julien Rouhaud <rjuju123(at)gmail(dot)com>, Sergei Kornilov <sk(at)zsrv(dot)org>, "imai(dot)yoshikazu(at)fujitsu(dot)com" <imai(dot)yoshikazu(at)fujitsu(dot)com>, legrand legrand <legrand_legrand(at)hotmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Planning counters in pg_stat_statements (using pgss_store)
Date: 2021-07-25 16:03:25
Message-ID: 42557.1627229005@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

[ blast from the past department ]

Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Yura Sokolov 2021-07-25 16:07:18 Re: [PoC] Improve dead tuple storage for lazy vacuum
Previous Message Tom Lane 2021-07-25 14:49:43 Re: Rename of triggers for partitioned tables