diff --git a/META.json b/META.json index 673fb3a..0475907 100644 --- a/META.json +++ b/META.json @@ -2,7 +2,7 @@ "name": "pg_stat_monitor", "abstract": "PostgreSQL Query Performance Monitoring Tool", "description": "pg_stat_monitor is a PostgreSQL Query Performance Monitoring tool, based on PostgreSQL's contrib module pg_stat_statements. PostgreSQL’s pg_stat_statements provides the basic statistics, which is sometimes not enough. The major shortcoming in pg_stat_statements is that it accumulates all the queries and their statistics and does not provide aggregated statistics nor histogram information. In this case, a user would need to calculate the aggregates, which is quite an expensive operation.", - "version": "2.0.0", + "version": "2.0.1", "maintainer": [ "ibrar.ahmed@percona.com" ], @@ -10,9 +10,9 @@ "provides": { "pg_stat_monitor": { "abstract": "PostgreSQL Query Performance Monitoring Tool", - "file": "pg_stat_monitor--1.0.sql", + "file": "pg_stat_monitor--2.0.sql", "docfile": "README.md", - "version": "2.0.0" + "version": "2.0.1" } }, "prereqs": { diff --git a/hash_query.c b/hash_query.c index e25a486..c18e20f 100644 --- a/hash_query.c +++ b/hash_query.c @@ -23,6 +23,9 @@ static PGSM_HASH_TABLE_HANDLE pgsm_create_bucket_hash(pgsmSharedState * pgsm, ds static Size pgsm_get_shared_area_size(void); static void InitializeSharedState(pgsmSharedState * pgsm); +#define PGSM_BUCKET_INFO_SIZE (sizeof(TimestampTz) * pgsm_max_buckets) +#define PGSM_SHARED_STATE_SIZE (sizeof(pgsmSharedState) + PGSM_BUCKET_INFO_SIZE) + #if USE_DYNAMIC_HASH /* parameter for the shared hash */ static dshash_parameters dsh_params = { @@ -56,7 +59,7 @@ pgsm_query_area_size(void) Size pgsm_ShmemSize(void) { - Size sz = MAXALIGN(sizeof(pgsmSharedState)); + Size sz = MAXALIGN(PGSM_SHARED_STATE_SIZE); sz = add_size(sz, MAX_QUERY_BUF); #if USE_DYNAMIC_HASH @@ -114,7 +117,7 @@ pgsm_startup(void) SpinLockInit(&pgsm->mutex); InitializeSharedState(pgsm); /* the allocation of pgsmSharedState itself */ - p += MAXALIGN(sizeof(pgsmSharedState)); + p += MAXALIGN(PGSM_SHARED_STATE_SIZE); pgsm->raw_dsa_area = p; dsa = dsa_create_in_place(pgsm->raw_dsa_area, pgsm_query_area_size(), @@ -138,6 +141,10 @@ pgsm_startup(void) * references. */ dsa_detach(dsa); + + pgsmStateLocal.pgsm_mem_cxt = AllocSetContextCreate(TopMemoryContext, + "pg_stat_monitor local store", + ALLOCSET_DEFAULT_SIZES); } #ifdef BENCHMARK @@ -158,10 +165,6 @@ InitializeSharedState(pgsmSharedState * pgsm) { pg_atomic_init_u64(&pgsm->current_wbucket, 0); pg_atomic_init_u64(&pgsm->prev_bucket_sec, 0); - memset(&pgsm->bucket_entry, 0, MAX_BUCKETS * sizeof(uint64)); - pgsm->pgsm_mem_cxt = AllocSetContextCreate(TopMemoryContext, - "pg_stat_monitor local store", - ALLOCSET_DEFAULT_SIZES); } @@ -235,6 +238,11 @@ pgsm_attach_shmem(void) MemoryContextSwitchTo(oldcontext); } +MemoryContext GetPgsmMemoryContext(void) +{ + return pgsmStateLocal.pgsm_mem_cxt; +} + dsa_area * get_dsa_area_for_query_text(void) { @@ -290,7 +298,6 @@ hash_entry_alloc(pgsmSharedState * pgsm, pgsmHashKey * key, int encoding) elog(DEBUG1, "[pg_stat_monitor] hash_entry_alloc: OUT OF MEMORY."); else if (!found) { - pgsm->bucket_entry[pg_atomic_read_u64(&pgsm->current_wbucket)]++; /* New entry, initialize it */ /* reset the statistics */ memset(&entry->counters, 0, sizeof(Counters)); diff --git a/pg_stat_monitor.c b/pg_stat_monitor.c index a76a6a7..3f1c693 100644 --- a/pg_stat_monitor.c +++ b/pg_stat_monitor.c @@ -36,7 +36,7 @@ typedef enum pgsmVersion PG_MODULE_MAGIC; -#define BUILD_VERSION "2.0.0" +#define BUILD_VERSION "2.0.1" /* Number of output arguments (columns) for various API versions */ #define PG_STAT_MONITOR_COLS_V1_0 52 @@ -692,9 +692,30 @@ pgsm_ExecutorEnd(QueryDesc *queryDesc) /* Extract the plan information in case of SELECT statement */ if (queryDesc->operation == CMD_SELECT && pgsm_enable_query_plan) { - plan_info.plan_len = snprintf(plan_info.plan_text, PLAN_TEXT_LEN, "%s", pgsm_explain(queryDesc)); - plan_info.planid = pgsm_hash_string(plan_info.plan_text, plan_info.plan_len); - plan_ptr = &plan_info; + int rv; + MemoryContext oldctx; + + /* + * Making sure it is a per query context so that there's no memory + * leak when executor ends. + */ + oldctx = MemoryContextSwitchTo(queryDesc->estate->es_query_cxt); + + rv = snprintf(plan_info.plan_text, PLAN_TEXT_LEN, "%s", pgsm_explain(queryDesc)); + + /* + * If snprint didn't write anything or there was an error, let's keep + * planinfo as NULL. + */ + if (rv > 0) + { + plan_info.plan_len = (rv < PLAN_TEXT_LEN) ? rv : PLAN_TEXT_LEN - 1; + plan_info.planid = pgsm_hash_string(plan_info.plan_text, plan_info.plan_len); + plan_ptr = &plan_info; + } + + /* Switch back to old context */ + MemoryContextSwitchTo(oldctx); } if (queryId != UINT64CONST(0) && queryDesc->totaltime && pgsm_enabled(exec_nested_level)) @@ -1569,7 +1590,7 @@ static void pgsm_add_to_list(pgsmEntry * entry, char *query_text, int query_len) { /* Switch to pgsm memory context */ - MemoryContext oldctx = MemoryContextSwitchTo(pgsm_get_ss()->pgsm_mem_cxt); + MemoryContext oldctx = MemoryContextSwitchTo(GetPgsmMemoryContext()); entry->query_text.query_pointer = pnstrdup(query_text, query_len); lentries = lappend(lentries, entry); @@ -1624,7 +1645,8 @@ static void pgsm_cleanup_callback(void *arg) { /* Reset the memory context holding the list */ - MemoryContextReset(pgsm_get_ss()->pgsm_mem_cxt); + MemoryContextReset(GetPgsmMemoryContext()); + lentries = NIL; callback_setup = false; } @@ -1645,7 +1667,7 @@ pgsm_create_hash_entry(uint64 bucket_id, uint64 queryid, PlanInfo * plan_info) char *username = NULL; /* Create an entry in the pgsm memory context */ - oldctx = MemoryContextSwitchTo(pgsm_get_ss()->pgsm_mem_cxt); + oldctx = MemoryContextSwitchTo(GetPgsmMemoryContext()); entry = palloc0(sizeof(pgsmEntry)); /* diff --git a/pg_stat_monitor.h b/pg_stat_monitor.h index 0495a3b..339f647 100644 --- a/pg_stat_monitor.h +++ b/pg_stat_monitor.h @@ -77,7 +77,6 @@ #define HISTOGRAM_MAX_TIME 50000000 #define MAX_RESPONSE_BUCKET 50 #define INVALID_BUCKET_ID -1 -#define MAX_BUCKETS 10 #define TEXT_LEN 255 #define ERROR_MESSAGE_LEN 100 #define REL_TYPENAME_LEN 64 @@ -388,8 +387,6 @@ typedef struct pgsmSharedState slock_t mutex; /* protects following fields only: */ pg_atomic_uint64 current_wbucket; pg_atomic_uint64 prev_bucket_sec; - uint64 bucket_entry[MAX_BUCKETS]; - TimestampTz bucket_start_time[MAX_BUCKETS]; /* start time of the bucket */ int hash_tranche_id; void *raw_dsa_area; /* DSA area pointer to store query texts. * dshash also lives in this memory when @@ -400,16 +397,9 @@ typedef struct pgsmSharedState * hash table handle. can be either classic shared memory hash or dshash * (if we are using USE_DYNAMIC_HASH) */ - MemoryContext pgsm_mem_cxt; - /* - * context to store stats in local memory until they are pushed to shared - * hash - */ - int resp_calls[MAX_RESPONSE_BUCKET + 2]; /* execution time's in - * msec; including 2 - * outlier buckets */ bool pgsm_oom; + TimestampTz bucket_start_time[]; /* start time of the bucket */ } pgsmSharedState; typedef struct pgsmLocalState @@ -418,6 +408,8 @@ typedef struct pgsmLocalState dsa_area *dsa; /* local dsa area for backend attached to the * dsa area created by postmaster at startup. */ PGSM_HASH_TABLE *shared_hash; + MemoryContext pgsm_mem_cxt; + } pgsmLocalState; #if PG_VERSION_NUM < 140000 @@ -479,6 +471,7 @@ void pgsm_startup(void); /* hash_query.c */ void pgsm_startup(void); +MemoryContext GetPgsmMemoryContext(void); /* guc.c */ void init_guc(void); diff --git a/regression/expected/version.out b/regression/expected/version.out index fe02474..60b6014 100644 --- a/regression/expected/version.out +++ b/regression/expected/version.out @@ -2,7 +2,7 @@ CREATE EXTENSION pg_stat_monitor; SELECT pg_stat_monitor_version(); pg_stat_monitor_version ------------------------- - 2.0.0 + 2.0.1 (1 row) DROP EXTENSION pg_stat_monitor; diff --git a/rpm/pg-stat-monitor.spec b/rpm/pg-stat-monitor.spec index 8be12cf..c9a2e58 100644 --- a/rpm/pg-stat-monitor.spec +++ b/rpm/pg-stat-monitor.spec @@ -33,7 +33,7 @@ It provides all the features of pg_stat_statment plus its own feature set. %build -sed -i 's:PG_CONFIG = pg_config:PG_CONFIG = /usr/pgsql-%{pgrel}/bin/pg_config:' Makefile +sed -i 's:PG_CONFIG ?= pg_config:PG_CONFIG = /usr/pgsql-%{pgrel}/bin/pg_config:' Makefile %{__make} USE_PGXS=1 %{?_smp_mflags}