PG-291: Fix query call count.

The issue with wrong query call count was taking place during transition
to a new bucket, the process is shortly describe bellow:

1. Scan for pending queries in previous bucket.
2. Add pending queries to the new bucket id.
3. Remove pending queries from previous bucket id.

The problem is that when switching to a new bucket, we reset query
statistics for a given entry being processed, so, for example, if the
pending query had a call count of 10 (9 of which were finished, 10th is
the pending one), if we move this query to the new bucket, the entry
will have its stats reseted, clearing the query call count to zero.

To solve the problem, whenever a pending query is detected, if the entry
has a call count > 1, we mark it as finished, and don't remove it from
the previous bucket in order to keep its statistics, then we move just
the pending query (10th in the example) to the new bucket id.

Another issue is that when moving a entry to a new bucket, we missed
copying the query position from the previous entry, which is used to
locate the query text in the query buffer:

hash_entry_dealloc():291
new_entry->query_pos = old_entry->query_pos;
pull/184/head
Diego Fronza 2021-12-22 12:46:58 -03:00 committed by Hamid Akhtar
parent fb4f632027
commit 1881fd737b
1 changed files with 30 additions and 5 deletions

View File

@ -216,8 +216,20 @@ hash_entry_dealloc(int new_bucket_id, int old_bucket_id, unsigned char *query_bu
pgssEntry *bkp_entry = malloc(sizeof(pgssEntry)); pgssEntry *bkp_entry = malloc(sizeof(pgssEntry));
if (!bkp_entry) if (!bkp_entry)
{ {
/* No memory, remove pending query entry from the previous bucket. */ pgsm_log_error("hash_entry_dealloc: out of memory");
elog(ERROR, "hash_entry_dealloc: out of memory"); /*
* No memory, If the entry has calls > 1 then we change the state to finished,
* as the pending query will likely finish execution during the new bucket
* time window. The pending query will vanish in this case, can't list it
* until it completes.
*
* If there is only one call to the query and it's pending, remove the
* entry from the previous bucket and allow it to finish in the new bucket,
* in order to avoid the query living in the old bucket forever.
*/
if (entry->counters.calls.calls > 1)
entry->counters.state = PGSS_FINISHED;
else
entry = hash_search(pgss_hash, &entry->key, HASH_REMOVE, NULL); entry = hash_search(pgss_hash, &entry->key, HASH_REMOVE, NULL);
continue; continue;
} }
@ -231,7 +243,19 @@ hash_entry_dealloc(int new_bucket_id, int old_bucket_id, unsigned char *query_bu
/* Add the entry to a list of nodes to be processed later. */ /* Add the entry to a list of nodes to be processed later. */
pending_entries = lappend(pending_entries, bkp_entry); pending_entries = lappend(pending_entries, bkp_entry);
/* Finally remove the pending query from the expired bucket id. */ /*
* If the entry has calls > 1 then we change the state to finished in
* the previous bucket, as the pending query will likely finish execution
* during the new bucket time window. Can't remove it from the previous bucket
* as it may have many calls and we would lose the query statistics.
*
* If there is only one call to the query and it's pending, remove the entry
* from the previous bucket and allow it to finish in the new bucket,
* in order to avoid the query living in the old bucket forever.
*/
if (entry->counters.calls.calls > 1)
entry->counters.state = PGSS_FINISHED;
else
entry = hash_search(pgss_hash, &entry->key, HASH_REMOVE, NULL); entry = hash_search(pgss_hash, &entry->key, HASH_REMOVE, NULL);
} }
} }
@ -255,6 +279,7 @@ hash_entry_dealloc(int new_bucket_id, int old_bucket_id, unsigned char *query_bu
new_entry->counters = old_entry->counters; new_entry->counters = old_entry->counters;
SpinLockInit(&new_entry->mutex); SpinLockInit(&new_entry->mutex);
new_entry->encoding = old_entry->encoding; new_entry->encoding = old_entry->encoding;
new_entry->query_pos = old_entry->query_pos;
} }
free(old_entry); free(old_entry);