Issue - (#53): Fix a bug where the query response time histogram does show proper counters.

PG-143 : Total sum of all the response histograms should be equal to total_calls.
pull/55/head
Ibrar Ahmed 2020-10-12 21:49:35 +00:00
parent 166ee0a25b
commit f48a64cc0a
3 changed files with 7 additions and 64 deletions

View File

@ -20,7 +20,6 @@ static HTAB *pgss_object_hash;
static HTAB *pgss_buckethash = NULL; static HTAB *pgss_buckethash = NULL;
static HTAB *pgss_waiteventshash = NULL; static HTAB *pgss_waiteventshash = NULL;
static pgssBucketEntry **pgssBucketEntries = NULL;
static pgssWaitEventEntry **pgssWaitEventEntries = NULL; static pgssWaitEventEntry **pgssWaitEventEntries = NULL;
static HTAB* hash_init(const char *hash_name, int key_size, int entry_size, int hash_size); static HTAB* hash_init(const char *hash_name, int key_size, int entry_size, int hash_size);
@ -81,8 +80,6 @@ pgss_shmem_startup(void)
} }
pgss_hash = hash_init("pg_stat_monitor: Queries hashtable", sizeof(pgssHashKey), sizeof(pgssEntry),PGSM_MAX); pgss_hash = hash_init("pg_stat_monitor: Queries hashtable", sizeof(pgssHashKey), sizeof(pgssEntry),PGSM_MAX);
pgss_buckethash = hash_init("pg_stat_monitor: Bucket hashtable", sizeof(pgssBucketHashKey), sizeof(pgssBucketEntry), PGSM_MAX_BUCKETS);
pgss_waiteventshash = hash_init("pg_stat_monitor: Wait Event hashtable", sizeof(pgssWaitEventKey), sizeof(pgssWaitEventEntry), 100); pgss_waiteventshash = hash_init("pg_stat_monitor: Wait Event hashtable", sizeof(pgssWaitEventKey), sizeof(pgssWaitEventEntry), 100);
pgss_object_hash = hash_init("pg_stat_monitor: Object hashtable", sizeof(pgssObjectHashKey), sizeof(pgssObjectEntry), PGSM_OBJECT_CACHE); pgss_object_hash = hash_init("pg_stat_monitor: Object hashtable", sizeof(pgssObjectHashKey), sizeof(pgssObjectEntry), PGSM_OBJECT_CACHE);
@ -105,24 +102,6 @@ pgss_shmem_startup(void)
} }
} }
pgssBucketEntries = malloc(sizeof (pgssBucketEntry) * PGSM_MAX_BUCKETS);
for (i = 0; i < PGSM_MAX_BUCKETS; i++)
{
pgssBucketHashKey key;
pgssBucketEntry *entry = NULL;
bool found = false;
key.bucket_id = i;
/* Find or create an entry with desired hash code */
entry = (pgssBucketEntry *) hash_search(pgss_buckethash, &key, HASH_ENTER, &found);
if (!found)
{
memset(&entry->counters, 0, sizeof(pgssBucketCounters));
SpinLockInit(&entry->mutex);
pgssBucketEntries[i] = entry;
}
}
LWLockRelease(AddinShmemInitLock); LWLockRelease(AddinShmemInitLock);
/* /*
@ -151,24 +130,12 @@ HTAB* pgsm_get_hash(void)
return pgss_hash; return pgss_hash;
} }
pgssBucketEntry** pgsm_get_bucket_entries(void)
{
Assert(pgssBucketEntries);
return pgssBucketEntries;
}
HTAB* pgsm_get_wait_event_hash(void) HTAB* pgsm_get_wait_event_hash(void)
{ {
Assert(pgss_waiteventshash); Assert(pgss_waiteventshash);
return pgss_waiteventshash; return pgss_waiteventshash;
} }
pgssBucketEntry** pgsm_get_bucket(void)
{
Assert(pgssBucketEntries);
return pgssBucketEntries;
}
pgssWaitEventEntry** pgsm_get_wait_event_entry(void) pgssWaitEventEntry** pgsm_get_wait_event_entry(void)
{ {
Assert(pgssWaitEventEntries); Assert(pgssWaitEventEntries);
@ -291,7 +258,6 @@ hash_entry_reset()
} }
pgss->current_wbucket = 0; pgss->current_wbucket = 0;
free(pgssWaitEventEntries); free(pgssWaitEventEntries);
free(pgssBucketEntries);
LWLockRelease(pgss->lock); LWLockRelease(pgss->lock);
} }

View File

@ -675,7 +675,6 @@ static void pgss_store(const char *query, uint64 queryId,
char tables_name[MAX_REL_LEN] = {0}; char tables_name[MAX_REL_LEN] = {0};
int len; int len;
pgssSharedState *pgss = pgsm_get_ss(); pgssSharedState *pgss = pgsm_get_ss();
pgssBucketEntry **pgssBucketEntries = pgsm_get_bucket();
HTAB *pgss_hash = pgsm_get_hash(); HTAB *pgss_hash = pgsm_get_hash();
Assert(query != NULL); Assert(query != NULL);
@ -831,12 +830,12 @@ static void pgss_store(const char *query, uint64 queryId,
{ {
if (total_time < PGSM_RESPOSE_TIME_LOWER_BOUND + (PGSM_RESPOSE_TIME_STEP * i)) if (total_time < PGSM_RESPOSE_TIME_LOWER_BOUND + (PGSM_RESPOSE_TIME_STEP * i))
{ {
pgssBucketEntries[entry->key.bucket_id]->counters.resp_calls[i]++; e->counters.resp_calls[i]++;
break; break;
} }
} }
if (total_time > PGSM_RESPOSE_TIME_LOWER_BOUND + (PGSM_RESPOSE_TIME_STEP * MAX_RESPONSE_BUCKET)) if (total_time > PGSM_RESPOSE_TIME_LOWER_BOUND + (PGSM_RESPOSE_TIME_STEP * MAX_RESPONSE_BUCKET))
pgssBucketEntries[entry->key.bucket_id]->counters.resp_calls[MAX_RESPONSE_BUCKET - 1]++; e->counters.resp_calls[MAX_RESPONSE_BUCKET - 1]++;
e->counters.calls[kind].rows += rows; e->counters.calls[kind].rows += rows;
e->counters.blocks.shared_blks_hit += bufusage->shared_blks_hit; e->counters.blocks.shared_blks_hit += bufusage->shared_blks_hit;
@ -1012,7 +1011,6 @@ pg_stat_monitor_internal(FunctionCallInfo fcinfo,
char queryid_txt[64]; char queryid_txt[64];
pgssSharedState *pgss = pgsm_get_ss(); pgssSharedState *pgss = pgsm_get_ss();
HTAB *pgss_hash = pgsm_get_hash(); HTAB *pgss_hash = pgsm_get_hash();
pgssBucketEntry **pgssBucketEntries = pgsm_get_bucket_entries();
query_txt = (char*) malloc(PGSM_QUERY_MAX_LEN); query_txt = (char*) malloc(PGSM_QUERY_MAX_LEN);
@ -1112,7 +1110,7 @@ pg_stat_monitor_internal(FunctionCallInfo fcinfo,
nulls[i++] = true; nulls[i++] = true;
} }
values[i++] = TimestampGetDatum(pgssBucketEntries[entry->key.bucket_id]->counters.current_time); values[i++] = TimestampGetDatum(pgss->bucket_start_time[entry->key.bucket_id]);
for (kind = 0; kind < PGSS_NUMKIND; kind++) for (kind = 0; kind < PGSS_NUMKIND; kind++)
{ {
@ -1140,7 +1138,7 @@ pg_stat_monitor_internal(FunctionCallInfo fcinfo,
values[i++] = Int64GetDatumFast(tmp.blocks.temp_blks_written); values[i++] = Int64GetDatumFast(tmp.blocks.temp_blks_written);
values[i++] = Float8GetDatumFast(tmp.blocks.blk_read_time); values[i++] = Float8GetDatumFast(tmp.blocks.blk_read_time);
values[i++] = Float8GetDatumFast(tmp.blocks.blk_write_time); values[i++] = Float8GetDatumFast(tmp.blocks.blk_write_time);
values[i++] = ArrayGetTextDatum(pgssBucketEntries[entry->key.bucket_id]->counters.resp_calls); values[i++] = ArrayGetTextDatum(tmp.resp_calls);
values[i++] = Float8GetDatumFast(tmp.sysinfo.utime); values[i++] = Float8GetDatumFast(tmp.sysinfo.utime);
values[i++] = Float8GetDatumFast(tmp.sysinfo.stime); values[i++] = Float8GetDatumFast(tmp.sysinfo.stime);
if (strlen(tmp.info.tables_name) == 0) if (strlen(tmp.info.tables_name) == 0)
@ -1163,7 +1161,6 @@ get_next_wbucket(pgssSharedState *pgss)
struct timeval tv; struct timeval tv;
uint64 current_usec; uint64 current_usec;
uint64 bucket_id; uint64 bucket_id;
pgssBucketEntry **pgssBucketEntries = pgsm_get_bucket();
gettimeofday(&tv,NULL); gettimeofday(&tv,NULL);
current_usec = tv.tv_sec; current_usec = tv.tv_sec;
@ -1182,7 +1179,7 @@ get_next_wbucket(pgssSharedState *pgss)
memset(buf, 0, sizeof (uint64)); memset(buf, 0, sizeof (uint64));
LWLockRelease(pgss->lock); LWLockRelease(pgss->lock);
pgss->prev_bucket_usec = current_usec; pgss->prev_bucket_usec = current_usec;
pgssBucketEntries[bucket_id]->counters.current_time = GetCurrentTimestamp(); pgss->bucket_start_time[bucket_id] = GetCurrentTimestamp();
return bucket_id; return bucket_id;
} }
return pgss->current_wbucket; return pgss->current_wbucket;

View File

@ -97,26 +97,6 @@ typedef enum AGG_KEY
AGG_KEY_HOST AGG_KEY_HOST
} AGG_KEY; } AGG_KEY;
/* Bucket shared_memory storage */
typedef struct pgssBucketHashKey
{
uint64 bucket_id; /* bucket number */
} pgssBucketHashKey;
typedef struct pgssBucketCounters
{
Timestamp current_time; /* start time of the bucket */
int resp_calls[MAX_RESPONSE_BUCKET]; /* execution time's in msec */
}pgssBucketCounters;
typedef struct pgssBucketEntry
{
pgssBucketHashKey key; /* hash key of entry - MUST BE FIRST */
pgssBucketCounters counters;
slock_t mutex; /* protects the counters only */
}pgssBucketEntry;
/* Objects shared memory storage */
typedef struct pgssObjectHashKey typedef struct pgssObjectHashKey
{ {
uint64 queryid; /* query id */ uint64 queryid; /* query id */
@ -215,6 +195,7 @@ typedef struct Counters
CallTime time[PGSS_NUMKIND]; CallTime time[PGSS_NUMKIND];
Blocks blocks; Blocks blocks;
SysInfo sysinfo; SysInfo sysinfo;
int resp_calls[MAX_RESPONSE_BUCKET]; /* execution time's in msec */
} Counters; } Counters;
/* Some global structure to get the cpu usage, really don't like the idea of global variable */ /* Some global structure to get the cpu usage, really don't like the idea of global variable */
@ -252,6 +233,7 @@ typedef struct pgssSharedState
uint64 bucket_entry[MAX_BUCKETS]; uint64 bucket_entry[MAX_BUCKETS];
QueryFifo query_fifo[MAX_BUCKETS]; QueryFifo query_fifo[MAX_BUCKETS];
int query_buf_size_bucket; int query_buf_size_bucket;
Timestamp bucket_start_time[MAX_BUCKETS]; /* start time of the bucket */
} pgssSharedState; } pgssSharedState;
#define ResetSharedState(x) \ #define ResetSharedState(x) \
@ -318,9 +300,7 @@ void pgss_shmem_shutdown(int code, Datum arg);
shmem_startup_hook_type prev_shmem_startup_hook; shmem_startup_hook_type prev_shmem_startup_hook;
int pgsm_get_bucket_size(void); int pgsm_get_bucket_size(void);
pgssSharedState* pgsm_get_ss(void); pgssSharedState* pgsm_get_ss(void);
pgssBucketEntry** pgsm_get_bucket_entries(void);
HTAB* pgsm_get_wait_event_hash(void); HTAB* pgsm_get_wait_event_hash(void);
pgssBucketEntry** pgsm_get_bucket(void);
HTAB* pgsm_get_hash(void); HTAB* pgsm_get_hash(void);
pgssWaitEventEntry** pgsm_get_wait_event_entry(void); pgssWaitEventEntry** pgsm_get_wait_event_entry(void);
void hash_entry_reset(void); void hash_entry_reset(void);