Merge pull request #399 from codeforall/main

Merging changes back to the main branch after the 2.0.1 release

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": {

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));

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));
+		int rv;
-		plan_info.planid = pgsm_hash_string(plan_info.plan_text, plan_info.plan_len);
+		MemoryContext oldctx;
-		plan_ptr = &plan_info;
+
+		/*
+		 * 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));
 
 	/*

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);

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;

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}