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-223: Remove relations and num_relations from pgssSharedState. 

These variables can't be in shared state, as the following problem was
taking place:
1. Process1 call pgss_ExecutorCheckPerms(), acquire lock, update
   relations and num_relations, release lock.
2. Process 2 call pgss_ExecutorCheckPerms(), acquire lock, update
   num_relations = 0;
3. Process 1 read num_relations = 0 in pgss_update_entry, this value is
   wrong as it was updated by Process 2.

Even if we acquire the lock in pgss_update_entry to read num_relations
and relations variable, Process 1 may end up acquiring the lock after
Process 2 has ovewritten the variable values, leading to Process 1
reading of wrong data.

By defining relations and num_relations to be static and global in
pg_stat_monitor.c we take advantage that each individual PostgreSQL
backend will have its own copy of this data, which allows us to remove
the locking in pgss_ExecutorCheckPerms to update these variables,
improving pg_stat_monitor overall performance.
pull/97/head
Diego Fronza 2021-08-27 15:53:11 -04:00
parent f3962509fb
commit a847ed95de
2 changed files with 9 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
pg_stat_monitor.c
View File

@ -66,7 +66,8 @@ static int plan_nested_level = 0;
/* The array to store outer layer query id*/ /* The array to store outer layer query id*/
uint64 *nested_queryids; uint64 *nested_queryids;
FILE *qfile; static char relations[REL_LST][REL_LEN];
static int num_relations; /* Number of relation in the query */
static bool system_init = false; static bool system_init = false;
static struct rusage rusage_start; static struct rusage rusage_start;
static struct rusage rusage_end; static struct rusage rusage_end;
@ -645,7 +646,6 @@ static void
pgss_ExecutorEnd(QueryDesc *queryDesc) pgss_ExecutorEnd(QueryDesc *queryDesc)
{ {
uint64 queryId = queryDesc->plannedstmt->queryId; uint64 queryId = queryDesc->plannedstmt->queryId;
pgssSharedState *pgss = pgsm_get_ss();
SysInfo sys_info; SysInfo sys_info;
PlanInfo plan_info; PlanInfo plan_info;
@ -693,7 +693,7 @@ pgss_ExecutorEnd(QueryDesc *queryDesc)
prev_ExecutorEnd(queryDesc); prev_ExecutorEnd(queryDesc);
else else
standard_ExecutorEnd(queryDesc); standard_ExecutorEnd(queryDesc);
pgss->num_relations = 0; num_relations = 0;
} }
#ifdef BENCHMARK #ifdef BENCHMARK
@ -713,13 +713,11 @@ static bool
pgss_ExecutorCheckPerms(List *rt, bool abort) pgss_ExecutorCheckPerms(List *rt, bool abort)
{ {
ListCell *lr = NULL; ListCell *lr = NULL;
pgssSharedState *pgss = pgsm_get_ss();
int i = 0; int i = 0;
int j = 0; int j = 0;
Oid list_oid[20]; Oid list_oid[20];
LWLockAcquire(pgss->lock, LW_EXCLUSIVE); num_relations = 0;
pgss->num_relations = 0;
foreach(lr, rt) foreach(lr, rt)
{ {
@ -744,14 +742,13 @@ pgss_ExecutorCheckPerms(List *rt, bool abort)
namespace_name = get_namespace_name(get_rel_namespace(rte->relid)); namespace_name = get_namespace_name(get_rel_namespace(rte->relid));
relation_name = get_rel_name(rte->relid); relation_name = get_rel_name(rte->relid);
if (rte->relkind == 'v') if (rte->relkind == 'v')
snprintf(pgss->relations[i++], REL_LEN, "%s.%s*", namespace_name, relation_name); snprintf(relations[i++], REL_LEN, "%s.%s*", namespace_name, relation_name);
else else
snprintf(pgss->relations[i++], REL_LEN, "%s.%s", namespace_name, relation_name); snprintf(relations[i++], REL_LEN, "%s.%s", namespace_name, relation_name);
} }
} }
} }
pgss->num_relations = i; num_relations = i;
LWLockRelease(pgss->lock);
if (prev_ExecutorCheckPerms_hook) if (prev_ExecutorCheckPerms_hook)
return prev_ExecutorCheckPerms_hook(rt, abort); return prev_ExecutorCheckPerms_hook(rt, abort);
@ -1184,7 +1181,6 @@ pgss_update_entry(pgssEntry *entry,
int index; int index;
char application_name[APPLICATIONNAME_LEN]; char application_name[APPLICATIONNAME_LEN];
int application_name_len = pg_get_application_name(application_name); int application_name_len = pg_get_application_name(application_name);
pgssSharedState *pgss = pgsm_get_ss();
double old_mean; double old_mean;
int message_len = error_info ? strlen (error_info->message) : 0; int message_len = error_info ? strlen (error_info->message) : 0;
int comments_len = comments ? strlen (comments) : 0; int comments_len = comments ? strlen (comments) : 0;
@ -1255,8 +1251,8 @@ pgss_update_entry(pgssEntry *entry,
_snprintf(e->counters.planinfo.plan_text, plan_info->plan_text, plan_text_len, PLAN_TEXT_LEN); _snprintf(e->counters.planinfo.plan_text, plan_info->plan_text, plan_text_len, PLAN_TEXT_LEN);
_snprintf(e->counters.info.application_name, application_name, application_name_len, APPLICATIONNAME_LEN); _snprintf(e->counters.info.application_name, application_name, application_name_len, APPLICATIONNAME_LEN);
e->counters.info.num_relations = pgss->num_relations; e->counters.info.num_relations = num_relations;
_snprintf2(e->counters.info.relations, pgss->relations, pgss->num_relations, REL_LEN); _snprintf2(e->counters.info.relations, relations, num_relations, REL_LEN);
e->counters.info.cmd_type = cmd_type; e->counters.info.cmd_type = cmd_type;

2
pg_stat_monitor.h
View File

@ -310,8 +310,6 @@ typedef struct pgssSharedState
uint64 prev_bucket_usec; uint64 prev_bucket_usec;
uint64 bucket_entry[MAX_BUCKETS]; uint64 bucket_entry[MAX_BUCKETS];
int64 query_buf_size_bucket; int64 query_buf_size_bucket;
char relations[REL_LST][REL_LEN];
int num_relations; /* Number of relation in the query */
char bucket_start_time[MAX_BUCKETS][60]; /* start time of the bucket */ char bucket_start_time[MAX_BUCKETS][60]; /* start time of the bucket */
} pgssSharedState; } pgssSharedState;