public inbox for [email protected]
help / color / mirror / Atom feedFrom: Ilia Evdokimov <[email protected]>
To: Michael Paquier <[email protected]>
To: Greg Sabino Mullane <[email protected]>
Cc: Andrey M. Borodin <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: Sample rate added to pg_stat_statements
Date: Thu, 21 Nov 2024 12:17:22 +0300
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<[email protected]>
<CAKAnmmLD8C-Y6CVF74Ccup8h9YpzBYzSvWWRG_Qf8+5YEMh4-g@mail.gmail.com>
<[email protected]>
On 20.11.2024 01:07, Michael Paquier wrote:
> On Tue, Nov 19, 2024 at 09:39:21AM -0500, Greg Sabino Mullane wrote:
>> Oh, and a +1 in general to the patch, OP, although it would also be nice to
>> start finding the bottlenecks that cause such performance issues.
> FWIW, I'm not eager to integrate this proposal without looking at this
> exact argument in depth.
>
> One piece of it would be to see how much of such "bottlenecks" we
> would be able to get rid of by integrating pg_stat_statements into
> the central pgstats with the custom APIs, without pushing the module
> into core. This means that we would combine the existing hash of pgss
> to shrink to 8 bytes for objid rather than 13 bytes now as the current
> code relies on (toplevel, userid, queryid) for the entry lookup (entry
> removal is sniped with these three values as well, or dshash seq
> scans). The odds of conflicts still still play in our favor even if
> we have a few million entries, or even ten times that.
>
> This would also get rid of the pgss text file for the queries, which
> is a source of one of the bottlenecks, as we could just store query
> strings upper-bounded based on a postmaster GUC to control the size of
> the entries in the pgstats dshash. More normalization for IN and ANY
> clauses would also help a not here, these are a cause of a lot of
> bloat.
>
> This integration is not something I will be able to work on for the
> PG18 dev cycle as I'm in full review/commit mode for the rest of this
> release, but I got some plans for it in PG19 except if somebody beats
> me to it.
> --
> Michael
I agree. Your proposal can indeed improve performance. Currently, I am
working on these changes and will validate them with benchmarks. Once I
have concrete results, I will open new threads to facilitate further
discussion.
However, in my opinion, the suggested improvements are not enough, and
sampling is essential.
1. I agree with Greg that pgss is widely used. It's quite odd that
sampling exists in 'auto_explain' but not in pgss.
2. If performance issues arise even after these improvements and it
turns out that pgss is the cause, the only painless solution without
restarting the instance is sampling. The current pgss's parameters are
not optimal for achieving this.
BTW, I forgot to include a case of nested statements. Either all will be
tracked or none. I attached new version of patch.
--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC.
Attachments:
[text/x-patch] v4-0001-Allow-setting-sample-ratio-for-pg_stat_statements.patch (3.6K, ../[email protected]/2-v4-0001-Allow-setting-sample-ratio-for-pg_stat_statements.patch)
download | inline diff:
From 68f5451019b261bf03a222f5a05ac57cd0fb9a24 Mon Sep 17 00:00:00 2001
From: Ilia Evdokimov <[email protected]>
Date: Thu, 21 Nov 2024 11:24:03 +0300
Subject: [PATCH] Allow setting sample ratio for pg_stat_statements
New configuration parameter pg_stat_statements.sample_ratio makes it
possible to track just a fraction of the queries meeting the configured
threshold, to reduce the amount of tracking.
---
.../pg_stat_statements/pg_stat_statements.c | 23 ++++++++++++++++++-
doc/src/sgml/pgstatstatements.sgml | 18 +++++++++++++++
2 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 49c657b3e0..d06e0d8a44 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -49,6 +49,7 @@
#include "access/parallel.h"
#include "catalog/pg_authid.h"
+#include "common/pg_prng.h"
#include "common/int.h"
#include "executor/instrument.h"
#include "funcapi.h"
@@ -294,12 +295,16 @@ static bool pgss_track_utility = true; /* whether to track utility commands */
static bool pgss_track_planning = false; /* whether to track planning
* duration */
static bool pgss_save = true; /* whether to save stats across shutdown */
+static double pgss_sample_rate = 1;
+/* Is the current top-level query to be sampled? */
+static bool current_query_sampled = false;
#define pgss_enabled(level) \
(!IsParallelWorker() && \
(pgss_track == PGSS_TRACK_ALL || \
- (pgss_track == PGSS_TRACK_TOP && (level) == 0)))
+ (pgss_track == PGSS_TRACK_TOP && (level) == 0)) && \
+ current_query_sampled)
#define record_gc_qtexts() \
do { \
@@ -414,6 +419,19 @@ _PG_init(void)
NULL,
NULL);
+ DefineCustomRealVariable("pg_stat_statements.sample_rate",
+ "Fraction of queries to process.",
+ NULL,
+ &pgss_sample_rate,
+ 1.0,
+ 0.0,
+ 1.0,
+ PGC_SUSET,
+ 0,
+ NULL,
+ NULL,
+ NULL);
+
DefineCustomEnumVariable("pg_stat_statements.track",
"Selects which statements are tracked by pg_stat_statements.",
NULL,
@@ -835,6 +853,9 @@ pgss_post_parse_analyze(ParseState *pstate, Query *query, JumbleState *jstate)
if (prev_post_parse_analyze_hook)
prev_post_parse_analyze_hook(pstate, query, jstate);
+ if (nesting_level == 0)
+ current_query_sampled = (pg_prng_double(&pg_global_prng_state) < pgss_sample_rate);
+
/* Safety check... */
if (!pgss || !pgss_hash || !pgss_enabled(nesting_level))
return;
diff --git a/doc/src/sgml/pgstatstatements.sgml b/doc/src/sgml/pgstatstatements.sgml
index 501b468e9a..d06349d097 100644
--- a/doc/src/sgml/pgstatstatements.sgml
+++ b/doc/src/sgml/pgstatstatements.sgml
@@ -936,6 +936,24 @@
</listitem>
</varlistentry>
+ <varlistentry>
+ <term>
+ <varname>pg_stat_statements.sample_rate</varname> (<type>real</type>)
+ <indexterm>
+ <primary><varname>pg_stat_statements.sample_rate</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+
+ <listitem>
+ <para>
+ <varname>pg_stat_statements.sample_rate</varname> causes pg_stat_statements to only
+ track a fraction of the statements in each session. The default is 1,
+ meaning track all the queries. In case of nested statements,
+ either all will be tracked or none. Only superusers can change this setting.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term>
<varname>pg_stat_statements.save</varname> (<type>boolean</type>)
--
2.34.1
view thread (58+ messages) latest in thread
reply
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: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: Sample rate added to pg_stat_statements
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox