| From: | Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> |
|---|---|
| To: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: Improve pg_stat_statements scalability |
| Date: | 2026-05-31 17:26:40 |
| Message-ID: | CAN4CZFN=6+2ntiDx=p4fq5X_kZmXtK=Eyd0G+kZdobdzAK=9JQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hello!
I noticed that the patch has some issues with minmax_only, and these
differences seem to be unintended to me:
1. minmax only resets updates mean/sum_var_time (but then when it
calculates them again, it also uses the data before the minmax reset)
+ /*
+ * Reset only min/max timing values and update minmax_stats_since.
+ * Iterate matching entries in the dshash, reset the corresponding
+ * pgstat counters' min/max fields.
+ */
...
+ shared->counters.mean_time[kind] = 0;
+ shared->counters.sum_var_time[kind] = 0;
2. min_time doesn't recover after a minmax reset:
+ if (pending->min_time[kind] < shared->min_time[kind])
+ shared->min_time[kind] = pending->min_time[kind];
As it is only updated if it's smaller, but it is 0 after it, and
because the call count remains > 0 it doesn't get initialized with the
current value.
+ if ((int) pg_atomic_read_u64(&pgss_shared->nentries) <= pgss_max *
(100 - USAGE_DEALLOC_PERCENT) / 100)
+ break;
isn't there a possible overflow here? The similar deleted check had
explicit uint64 casts for pgss_max multiplication.
- /*
- * Don't proceed if file does not exceed 512 bytes per possible entry.
- */
- if ((uint64) extent < (uint64) 512 * pgss_max)
- return false;
| From | Date | Subject | |
|---|---|---|---|
| Previous Message | Kirk Wolak | 2026-05-31 16:53:09 | Re: Add wait events for server logging destination writes |