external
/
pg_stat_monitor
mirror of https://github.com/percona/pg_stat_monitor.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

PG-562: Histogram Ranges/Buckets are not correct. 

Added buckets for queries that take less than minimum histogram time
and one for the ones taking more than the max value specified.

Also, in case the buckets end up overlapping, on server start, an
error will be thrown informing the user of this issue and requesting
a rectification.

Refactored the code to consolidate the calculations in a single
function.
pull/360/head
Hamid Akhtar 2023-01-19 05:39:37 +05:00 committed by Muhammad Usama
parent 1286427445
commit 209f370cef
8 changed files with 109 additions and 92 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
guc.c
View File

@ -150,7 +150,7 @@ init_guc(void)
.guc_max = MAX_RESPONSE_BUCKET,
.guc_restart = true,
.guc_unit = 0,
.guc_value = &PGSM_HISTOGRAM_BUCKETS
.guc_value = &PGSM_HISTOGRAM_BUCKETS_USER
};
DefineIntGUC(&conf[i++]);

3
pg_stat_monitor--2.0.sql
View File

@ -63,7 +63,7 @@ SELECT
$$
LANGUAGE SQL PARALLEL SAFE;
CREATE FUNCTION histogram(_bucket int, _quryid text)
CREATE FUNCTION histogram(_bucket int, _quryid int8)
RETURNS SETOF RECORD AS $$
DECLARE
rec record;
@ -463,4 +463,3 @@ REVOKE ALL ON FUNCTION pgsm_create_14_view FROM PUBLIC;
REVOKE ALL ON FUNCTION pgsm_create_15_view FROM PUBLIC;
GRANT SELECT ON pg_stat_monitor TO PUBLIC;

129
pg_stat_monitor.c
View File

@ -93,6 +93,7 @@ static struct rusage rusage_end;
static char *pgss_explain(QueryDesc *queryDesc);
static void extract_query_comments(const char *query, char *comments, size_t max_len);
static void histogram_bucket_timings(int index, int64 *b_start, int64 *b_end);
static int get_histogram_bucket(double q_time);
static bool IsSystemInitialized(void);
static double time_diff(struct timeval end, struct timeval start);
@ -254,6 +255,24 @@ _PG_init(void)
/* Inilize the GUC variables */
init_guc();
/* Validate histogram values and in case of failure, revert to defaults */
{
int64 b1_start;
int64 b2_start;
int64 b1_end;
int64 b2_end;
if (PGSM_HISTOGRAM_BUCKETS_USER >= 2)
{
histogram_bucket_timings(1, &b1_start, &b1_end);
histogram_bucket_timings(2, &b2_start, &b2_end);
/* Checking if histograms buckets overlap */
if (b2_start < b1_end || b2_start == b2_end)
elog(ERROR, "pg_stat_monitor: Histogram buckets are overlapping, please fix histogram configuration.");
}
}
#if PG_VERSION_NUM >= 140000
/*
@ -3238,65 +3257,99 @@ unpack_sql_state(int sql_state)
return buf;
}
/*
* Given an index, return the histogram start and end times.
*/
static void
histogram_bucket_timings(int index, int64 *b_start, int64 *b_end)
{
double q_min = PGSM_HISTOGRAM_MIN;
double q_max = PGSM_HISTOGRAM_MAX;
int b_count = PGSM_HISTOGRAM_BUCKETS;
int b_count_user = PGSM_HISTOGRAM_BUCKETS_USER;
double bucket_size;
/*
* We must not skip any queries that fall outside the user defined
* histogram buckets. So capturing min/max outliers.
*/
if (index == 0 && q_min > 0)
{
*b_start = 0;
*b_end = q_min;
return;
}
else if (index == (b_count - 1))
{
*b_start = q_max;
*b_end = -1;
return;
}
/*
* Equisized logrithmic values will yield exponential values as required.
* For calculating logrithmic value, we MUST use the number of bucket provided
* by the user.
*/
bucket_size = log(q_max - q_min) / (double) b_count_user;
/* Can't do exp(0) as that returns 1. So handling the case of first entry specifically */
*b_start = q_min + ((index == 0 || (index == 1 && q_min > 0)) ? 0 : exp(bucket_size * (index - 1)));
*b_end = q_min + exp(bucket_size * index);
}
/*
* Get the histogram bucket index for a given query time.
*/
static int
get_histogram_bucket(double q_time)
{
double q_min = PGSM_HISTOGRAM_MIN;
double q_max = PGSM_HISTOGRAM_MAX;
int64 b_start;
int64 b_end;
int b_count = PGSM_HISTOGRAM_BUCKETS;
int index = 0;
double b_max;
double b_min;
double bucket_size;
q_time -= q_min;
b_max = log(q_max - q_min);
b_min = 0;
bucket_size = (b_max - b_min) / (double) b_count;
for (index = 1; index <= b_count; index++)
for (index = 0; index < b_count; index++)
{
int64 b_start = (index == 1) ? 0 : exp(bucket_size * (index - 1));
int64 b_end = exp(bucket_size * index);
histogram_bucket_timings(index, &b_start, &b_end);
if ((index == 1 && q_time < b_start)
|| (q_time >= b_start && q_time <= b_end)
|| (index == b_count && q_time > b_end))
{
return index - 1;
}
}
return 0;
if (q_time >= b_start && q_time <= b_end)
return index;
}
/*
* So haven't found a histogram bucket for this query. That's only possible for the
* last bucket as its end time is less than 0.
*/
return (b_count - 1);
}
/*
* Get the timings of the histogram as a single string. The last bucket
* has ellipses as the end value indication infinity.
*/
Datum
get_histogram_timings(PG_FUNCTION_ARGS)
{
double q_min = PGSM_HISTOGRAM_MIN;
double q_max = PGSM_HISTOGRAM_MAX;
int64 b_start;
int64 b_end;
int b_count = PGSM_HISTOGRAM_BUCKETS;
int index = 0;
double b_max;
double b_min;
double bucket_size;
bool first = true;
char *tmp_str = palloc0(MAX_STRING_LEN);
char *text_str = palloc0(MAX_STRING_LEN);
b_max = log(q_max - q_min);
b_min = 0;
bucket_size = (b_max - b_min) / (double) b_count;
for (index = 1; index <= b_count; index++)
for (index = 0; index < b_count; index++)
{
int64 b_start = (index == 1) ? 0 : exp(bucket_size * (index - 1));
int64 b_end = exp(bucket_size * index);
histogram_bucket_timings(index, &b_start, &b_end);
if (first)
if (index == 0)
{
snprintf(text_str, MAX_STRING_LEN, "(%ld - %ld)}", b_start, b_end);
first = false;
}
else if (index == (b_count - 1))
{
snprintf(tmp_str, MAX_STRING_LEN, "%s, (%ld - ...)}", text_str, b_start);
snprintf(text_str, MAX_STRING_LEN, "%s", tmp_str);
}
else
{
@ -3304,7 +3357,9 @@ get_histogram_timings(PG_FUNCTION_ARGS)
snprintf(text_str, MAX_STRING_LEN, "%s", tmp_str);
}
}
pfree(tmp_str);
return CStringGetTextDatum(text_str);
}

10
pg_stat_monitor.h
View File

@ -485,7 +485,15 @@ static const struct config_enum_entry track_options[] =
#define PGSM_BUCKET_TIME get_conf(5)->guc_variable
#define PGSM_HISTOGRAM_MIN get_conf(6)->guc_variable
#define PGSM_HISTOGRAM_MAX get_conf(7)->guc_variable
#define PGSM_HISTOGRAM_BUCKETS get_conf(8)->guc_variable
/*
* Important that we keep user bucket values separate but must add 1
* for max outlier queries. However, for min, bucket should only be added
* if the minimum value provided by user is greater than 0
*/
#define PGSM_HISTOGRAM_BUCKETS_USER get_conf(8)->guc_variable
#define PGSM_HISTOGRAM_BUCKETS (PGSM_HISTOGRAM_BUCKETS_USER + 1 + (int)(PGSM_HISTOGRAM_MIN > 0))
#define PGSM_QUERY_SHARED_BUFFER get_conf(9)->guc_variable
#define PGSM_OVERFLOW_TARGET get_conf(10)->guc_variable
#define PGSM_QUERY_PLAN get_conf(11)->guc_variable

14
t/009_settings_pgsm_histogram_max.pl
View File

@ -41,18 +41,8 @@ PGSM::append_to_file($stdout);
ok($cmdret == 0, "Print PGSM EXTENSION Settings");
PGSM::append_to_file($stdout);
$node->append_conf('postgresql.conf', "pg_stat_monitor.pgsm_histogram_max = 10\n");
$node->restart();
($cmdret, $stdout, $stderr) = $node->psql('postgres', 'SELECT pg_stat_monitor_reset();', extra_params => ['-a', '-Pformat=aligned','-Ptuples_only=off']);
ok($cmdret == 0, "Reset PGSM EXTENSION");
PGSM::append_to_file($stdout);
($cmdret, $stdout, $stderr) = $node->psql('postgres', "SELECT name, setting, unit, context, vartype, source, min_val, max_val, enumvals, boot_val, reset_val, pending_restart FROM pg_settings WHERE name='pg_stat_monitor.pgsm_histogram_max';", extra_params => ['-a', '-Pformat=aligned','-Ptuples_only=off']);
ok($cmdret == 0, "Print PGSM EXTENSION Settings");
PGSM::append_to_file($stdout);
$node->append_conf('postgresql.conf', "pg_stat_monitor.pgsm_histogram_max = 100\n");
$node->append_conf('postgresql.conf', "pg_stat_monitor.pgsm_histogram_max = 50\n");
$node->append_conf('postgresql.conf', "pg_stat_monitor.pgsm_histogram_buckets = 3\n");
$node->restart();
($cmdret, $stdout, $stderr) = $node->psql('postgres', 'SELECT pg_stat_monitor_reset();', extra_params => ['-a', '-Pformat=aligned','-Ptuples_only=off']);

15
t/010_settings_pgsm_histogram_min.pl
View File

@ -41,7 +41,7 @@ PGSM::append_to_file($stdout);
ok($cmdret == 0, "Print PGSM EXTENSION Settings");
PGSM::append_to_file($stdout);
$node->append_conf('postgresql.conf', "pg_stat_monitor.pgsm_histogram_min = 1000\n");
$node->append_conf('postgresql.conf', "pg_stat_monitor.pgsm_histogram_min = 20\n");
$node->restart();
($cmdret, $stdout, $stderr) = $node->psql('postgres', 'SELECT pg_stat_monitor_reset();', extra_params => ['-a', '-Pformat=aligned','-Ptuples_only=off']);
@ -52,18 +52,7 @@ PGSM::append_to_file($stdout);
ok($cmdret == 0, "Print PGSM EXTENSION Settings");
PGSM::append_to_file($stdout);
$node->append_conf('postgresql.conf', "pg_stat_monitor.pgsm_histogram_min = 10000\n");
$node->restart();
($cmdret, $stdout, $stderr) = $node->psql('postgres', 'SELECT pg_stat_monitor_reset();', extra_params => ['-a', '-Pformat=aligned','-Ptuples_only=off']);
ok($cmdret == 0, "Reset PGSM EXTENSION");
PGSM::append_to_file($stdout);
($cmdret, $stdout, $stderr) = $node->psql('postgres', "SELECT name, setting, unit, context, vartype, source, min_val, max_val, enumvals, boot_val, reset_val, pending_restart FROM pg_settings WHERE name='pg_stat_monitor.pgsm_histogram_min';", extra_params => ['-a', '-Pformat=aligned','-Ptuples_only=off']);
ok($cmdret == 0, "Print PGSM EXTENSION Settings");
PGSM::append_to_file($stdout);
$node->append_conf('postgresql.conf', "pg_stat_monitor.pgsm_histogram_min = 99999\n");
$node->append_conf('postgresql.conf', "pg_stat_monitor.pgsm_histogram_min = 100\n");
$node->restart();
($cmdret, $stdout, $stderr) = $node->psql('postgres', 'SELECT pg_stat_monitor_reset();', extra_params => ['-a', '-Pformat=aligned','-Ptuples_only=off']);

14
t/expected/009_settings_pgsm_histogram_max.out
View File

@ -20,19 +20,7 @@ SELECT pg_stat_monitor_reset();
SELECT name, setting, unit, context, vartype, source, min_val, max_val, enumvals, boot_val, reset_val, pending_restart FROM pg_settings WHERE name='pg_stat_monitor.pgsm_histogram_max';
name | setting | unit | context | vartype | source | min_val | max_val | enumvals | boot_val | reset_val | pending_restart
------------------------------------+---------+------+------------+---------+--------------------+---------+------------+----------+----------+-----------+-----------------
pg_stat_monitor.pgsm_histogram_max | 10 | | postmaster | integer | configuration file | 10 | 2147483647 | | 100000 | 10 | f
(1 row)
SELECT pg_stat_monitor_reset();
pg_stat_monitor_reset
-----------------------
(1 row)
SELECT name, setting, unit, context, vartype, source, min_val, max_val, enumvals, boot_val, reset_val, pending_restart FROM pg_settings WHERE name='pg_stat_monitor.pgsm_histogram_max';
name | setting | unit | context | vartype | source | min_val | max_val | enumvals | boot_val | reset_val | pending_restart
------------------------------------+---------+------+------------+---------+--------------------+---------+------------+----------+----------+-----------+-----------------
pg_stat_monitor.pgsm_histogram_max | 100 | | postmaster | integer | configuration file | 10 | 2147483647 | | 100000 | 100 | f
pg_stat_monitor.pgsm_histogram_max | 50 | | postmaster | integer | configuration file | 10 | 2147483647 | | 100000 | 50 | f
(1 row)
SELECT pg_stat_monitor_reset();

16
t/expected/010_settings_pgsm_histogram_min.out
View File

@ -20,7 +20,7 @@ SELECT pg_stat_monitor_reset();
SELECT name, setting, unit, context, vartype, source, min_val, max_val, enumvals, boot_val, reset_val, pending_restart FROM pg_settings WHERE name='pg_stat_monitor.pgsm_histogram_min';
name | setting | unit | context | vartype | source | min_val | max_val | enumvals | boot_val | reset_val | pending_restart
------------------------------------+---------+------+------------+---------+--------------------+---------+------------+----------+----------+-----------+-----------------
pg_stat_monitor.pgsm_histogram_min | 1000 | | postmaster | integer | configuration file | 0 | 2147483647 | | 1 | 1000 | f
pg_stat_monitor.pgsm_histogram_min | 20 | | postmaster | integer | configuration file | 0 | 2147483647 | | 1 | 20 | f
(1 row)
SELECT pg_stat_monitor_reset();
@ -32,19 +32,7 @@ SELECT pg_stat_monitor_reset();
SELECT name, setting, unit, context, vartype, source, min_val, max_val, enumvals, boot_val, reset_val, pending_restart FROM pg_settings WHERE name='pg_stat_monitor.pgsm_histogram_min';
name | setting | unit | context | vartype | source | min_val | max_val | enumvals | boot_val | reset_val | pending_restart
------------------------------------+---------+------+------------+---------+--------------------+---------+------------+----------+----------+-----------+-----------------
pg_stat_monitor.pgsm_histogram_min | 10000 | | postmaster | integer | configuration file | 0 | 2147483647 | | 1 | 10000 | f
(1 row)
SELECT pg_stat_monitor_reset();
pg_stat_monitor_reset
-----------------------
(1 row)
SELECT name, setting, unit, context, vartype, source, min_val, max_val, enumvals, boot_val, reset_val, pending_restart FROM pg_settings WHERE name='pg_stat_monitor.pgsm_histogram_min';
name | setting | unit | context | vartype | source | min_val | max_val | enumvals | boot_val | reset_val | pending_restart
------------------------------------+---------+------+------------+---------+--------------------+---------+------------+----------+----------+-----------+-----------------
pg_stat_monitor.pgsm_histogram_min | 99999 | | postmaster | integer | configuration file | 0 | 2147483647 | | 1 | 99999 | f
pg_stat_monitor.pgsm_histogram_min | 100 | | postmaster | integer | configuration file | 0 | 2147483647 | | 1 | 100 | f
(1 row)
DROP EXTENSION pg_stat_monitor;