diff --git a/guc.c b/guc.c index 6bab2d5..6acbe0e 100644 --- a/guc.c +++ b/guc.c @@ -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++]); diff --git a/pg_stat_monitor--2.0.sql b/pg_stat_monitor--2.0.sql index 3ee7935..7bc2836 100644 --- a/pg_stat_monitor--2.0.sql +++ b/pg_stat_monitor--2.0.sql @@ -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; - diff --git a/pg_stat_monitor.c b/pg_stat_monitor.c index 24f0333..26e4da6 100644 --- a/pg_stat_monitor.c +++ b/pg_stat_monitor.c @@ -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; - } + if (q_time >= b_start && q_time <= b_end) + return index; } - return 0; + + /* + * 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); } diff --git a/pg_stat_monitor.h b/pg_stat_monitor.h index 9d2dfc2..0336643 100644 --- a/pg_stat_monitor.h +++ b/pg_stat_monitor.h @@ -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 diff --git a/t/009_settings_pgsm_histogram_max.pl b/t/009_settings_pgsm_histogram_max.pl index dcccf9c..884a860 100644 --- a/t/009_settings_pgsm_histogram_max.pl +++ b/t/009_settings_pgsm_histogram_max.pl @@ -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']); diff --git a/t/010_settings_pgsm_histogram_min.pl b/t/010_settings_pgsm_histogram_min.pl index 09e26e8..01d7d9f 100644 --- a/t/010_settings_pgsm_histogram_min.pl +++ b/t/010_settings_pgsm_histogram_min.pl @@ -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']); diff --git a/t/expected/009_settings_pgsm_histogram_max.out b/t/expected/009_settings_pgsm_histogram_max.out index a6539dd..17c9caa 100644 --- a/t/expected/009_settings_pgsm_histogram_max.out +++ b/t/expected/009_settings_pgsm_histogram_max.out @@ -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(); diff --git a/t/expected/010_settings_pgsm_histogram_min.out b/t/expected/010_settings_pgsm_histogram_min.out index cae3d5b..dfb7f09 100644 --- a/t/expected/010_settings_pgsm_histogram_min.out +++ b/t/expected/010_settings_pgsm_histogram_min.out @@ -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;