PG-475: Inconsistent behaviour of PGSM

Reverting the bucket locking mechanism to previous behavior. This has
a lot of room for improvement that needs to be part of a major refactoring
in the 2.x release.
pull/295/head
Hamid Akhtar 2022-08-22 20:35:49 +05:00
parent 3b9e180837
commit ca771b9ad6
1 changed files with 45 additions and 40 deletions

View File

@ -2151,7 +2151,7 @@ get_next_wbucket(pgssSharedState *pgss)
uint64 new_bucket_id; uint64 new_bucket_id;
uint64 prev_bucket_id; uint64 prev_bucket_id;
struct tm *lt; struct tm *lt;
char file_name[1024]; bool update_bucket = false;
gettimeofday(&tv, NULL); gettimeofday(&tv, NULL);
current_sec = (TimestampTz) tv.tv_sec - ((POSTGRES_EPOCH_JDATE - UNIX_EPOCH_JDATE) * SECS_PER_DAY); current_sec = (TimestampTz) tv.tv_sec - ((POSTGRES_EPOCH_JDATE - UNIX_EPOCH_JDATE) * SECS_PER_DAY);
@ -2172,28 +2172,27 @@ get_next_wbucket(pgssSharedState *pgss)
* definitely make the while condition to fail, we can stop the loop as * definitely make the while condition to fail, we can stop the loop as
* another thread has already updated prev_bucket_sec. * another thread has already updated prev_bucket_sec.
*/ */
if ((current_sec - current_bucket_sec) < (uint64)PGSM_BUCKET_TIME) while ((current_sec - current_bucket_sec) > ((uint64)PGSM_BUCKET_TIME))
{ {
return pg_atomic_read_u64(&pgss->current_wbucket); if (pg_atomic_compare_exchange_u64(&pgss->prev_bucket_sec, &current_bucket_sec, current_sec))
{
update_bucket = true;
break;
} }
current_bucket_sec = pg_atomic_read_u64(&pgss->prev_bucket_sec);
}
if (update_bucket)
{
char file_name[1024];
new_bucket_id = (tv.tv_sec / PGSM_BUCKET_TIME) % PGSM_MAX_BUCKETS; new_bucket_id = (tv.tv_sec / PGSM_BUCKET_TIME) % PGSM_MAX_BUCKETS;
/* Update bucket id and retrieve the previous one. */ /* Update bucket id and retrieve the previous one. */
prev_bucket_id = pg_atomic_exchange_u64(&pgss->current_wbucket, new_bucket_id); prev_bucket_id = pg_atomic_exchange_u64(&pgss->current_wbucket, new_bucket_id);
tv.tv_sec = (tv.tv_sec) - (tv.tv_sec % PGSM_BUCKET_TIME);
lt = localtime(&tv.tv_sec);
LWLockAcquire(pgss->lock, LW_EXCLUSIVE); LWLockAcquire(pgss->lock, LW_EXCLUSIVE);
/* Reconfirm that no other backend has created the bucket while we waited */
if (new_bucket_id == prev_bucket_id)
{
LWLockRelease(pgss->lock);
return new_bucket_id;
}
hash_entry_dealloc(new_bucket_id, prev_bucket_id, pgss_qbuf); hash_entry_dealloc(new_bucket_id, prev_bucket_id, pgss_qbuf);
if (pgss->overflow) if (pgss->overflow)
@ -2202,9 +2201,9 @@ get_next_wbucket(pgssSharedState *pgss)
if (pgss->n_bucket_cycles >= PGSM_MAX_BUCKETS) if (pgss->n_bucket_cycles >= PGSM_MAX_BUCKETS)
{ {
/* /*
* A full rotation of PGSM_MAX_BUCKETS buckets happened since we * A full rotation of PGSM_MAX_BUCKETS buckets happened since
* detected a query buffer overflow. Reset overflow state and * we detected a query buffer overflow.
* remove the dump file. * Reset overflow state and remove the dump file.
*/ */
pgss->overflow = false; pgss->overflow = false;
pgss->n_bucket_cycles = 0; pgss->n_bucket_cycles = 0;
@ -2213,12 +2212,18 @@ get_next_wbucket(pgssSharedState *pgss)
} }
} }
LWLockRelease(pgss->lock);
tv.tv_sec = (tv.tv_sec) - (tv.tv_sec % PGSM_BUCKET_TIME);
lt = localtime(&tv.tv_sec);
snprintf(pgss->bucket_start_time[new_bucket_id], sizeof(pgss->bucket_start_time[new_bucket_id]), snprintf(pgss->bucket_start_time[new_bucket_id], sizeof(pgss->bucket_start_time[new_bucket_id]),
"%04d-%02d-%02d %02d:%02d:%02d", lt->tm_year + 1900, lt->tm_mon + 1, lt->tm_mday, lt->tm_hour, lt->tm_min, lt->tm_sec); "%04d-%02d-%02d %02d:%02d:%02d", lt->tm_year + 1900, lt->tm_mon + 1, lt->tm_mday, lt->tm_hour, lt->tm_min, lt->tm_sec);
LWLockRelease(pgss->lock);
return new_bucket_id; return new_bucket_id;
}
return pg_atomic_read_u64(&pgss->current_wbucket);
} }
#if PG_VERSION_NUM < 140000 #if PG_VERSION_NUM < 140000