upstream/postgresql
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2026-06-10 09:10:33 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

Use single LWLock for lock statistics in pgstats

Browse source 
Previously, one LWLock was used for each lock type, adding complexity
without an observable performance benefit as data is gathered only for
paths involving lock waits, at least currently.  This commit replaces
the per-type set of LWLocks with a single LWLock protecting the stats
data of all the lock types, like the stats kinds for SLRU or WAL.  A
good chunk of the callbacks get simpler thanks to this change.

The previous approach also had one bug in the flush callback when nowait
was called with "true": a backend iterating over all entries could
successfully flush some entries while skipping others due to contention,
then unconditionally reset the pending data.  This would cause some
stats data loss.

Oversight in 4019f725f5.

Reported-by: Tomas Vondra <tomas@vondra.me>
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/1af63e6d-16d5-4d5b-9b03-11472ef1adf9@vondra.me
This commit is contained in:
Michael Paquier 2026-04-06 14:01:04 +09:00
parent 283c5fb22b
commit 404a17c155
2 changed files with 28 additions and 59 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

80
src/backend/utils/activity/pgstat_lock.c
View file

@ -50,99 +50,71 @@ pgstat_lock_flush(bool nowait)
bool
pgstat_lock_flush_cb(bool nowait)
{
LWLock *lcktype_lock;
PgStat_LockEntry *lck_shstats;
bool lock_not_acquired = false;
LWLock *lckstat_lock;
PgStatShared_Lock *shstats;
if (!have_lockstats)
return false;
shstats = &pgStatLocal.shmem->lock;
lckstat_lock = &shstats->lock;
if (!nowait)
LWLockAcquire(lckstat_lock, LW_EXCLUSIVE);
else if (!LWLockConditionalAcquire(lckstat_lock, LW_EXCLUSIVE))
return true;
for (int i = 0; i <= LOCKTAG_LAST_TYPE; i++)
{
lcktype_lock = &pgStatLocal.shmem->lock.locks[i];
lck_shstats =
&pgStatLocal.shmem->lock.stats.stats[i];
if (!nowait)
LWLockAcquire(lcktype_lock, LW_EXCLUSIVE);
else if (!LWLockConditionalAcquire(lcktype_lock, LW_EXCLUSIVE))
{
lock_not_acquired = true;
continue;
}
#define LOCKSTAT_ACC(fld) \
(lck_shstats->fld += PendingLockStats.stats[i].fld)
(shstats->stats.stats[i].fld += PendingLockStats.stats[i].fld)
LOCKSTAT_ACC(waits);
LOCKSTAT_ACC(wait_time);
LOCKSTAT_ACC(fastpath_exceeded);
#undef LOCKSTAT_ACC
LWLockRelease(lcktype_lock);
}
memset(&PendingLockStats, 0, sizeof(PendingLockStats));
LWLockRelease(lckstat_lock);
memset(&PendingLockStats, 0, sizeof(PendingLockStats));
have_lockstats = false;
return lock_not_acquired;
return false;
}
void
pgstat_lock_init_shmem_cb(void *stats)
{
PgStatShared_Lock *stat_shmem = (PgStatShared_Lock *) stats;
for (int i = 0; i <= LOCKTAG_LAST_TYPE; i++)
LWLockInitialize(&stat_shmem->locks[i], LWTRANCHE_PGSTATS_DATA);
LWLockInitialize(&stat_shmem->lock, LWTRANCHE_PGSTATS_DATA);
}
void
pgstat_lock_reset_all_cb(TimestampTz ts)
{
for (int i = 0; i <= LOCKTAG_LAST_TYPE; i++)
{
LWLock *lcktype_lock = &pgStatLocal.shmem->lock.locks[i];
PgStat_LockEntry *lck_shstats = &pgStatLocal.shmem->lock.stats.stats[i];
LWLock *lckstat_lock = &pgStatLocal.shmem->lock.lock;
LWLockAcquire(lcktype_lock, LW_EXCLUSIVE);
LWLockAcquire(lckstat_lock, LW_EXCLUSIVE);
/*
* Use the lock in the first lock type PgStat_LockEntry to protect the
* reset timestamp as well.
*/
if (i == 0)
pgStatLocal.shmem->lock.stats.stat_reset_timestamp = ts;
pgStatLocal.shmem->lock.stats.stat_reset_timestamp = ts;
memset(lck_shstats, 0, sizeof(*lck_shstats));
LWLockRelease(lcktype_lock);
}
memset(pgStatLocal.shmem->lock.stats.stats, 0,
sizeof(pgStatLocal.shmem->lock.stats.stats));
LWLockRelease(lckstat_lock);
}
void
pgstat_lock_snapshot_cb(void)
{
for (int i = 0; i <= LOCKTAG_LAST_TYPE; i++)
{
LWLock *lcktype_lock = &pgStatLocal.shmem->lock.locks[i];
PgStat_LockEntry *lck_shstats = &pgStatLocal.shmem->lock.stats.stats[i];
PgStat_LockEntry *lck_snap = &pgStatLocal.snapshot.lock.stats[i];
LWLock *lckstat_lock = &pgStatLocal.shmem->lock.lock;
LWLockAcquire(lcktype_lock, LW_SHARED);
LWLockAcquire(lckstat_lock, LW_SHARED);
/*
* Use the lock in the first lock type PgStat_LockEntry to protect the
* reset timestamp as well.
*/
if (i == 0)
pgStatLocal.snapshot.lock.stat_reset_timestamp =
pgStatLocal.shmem->lock.stats.stat_reset_timestamp;
pgStatLocal.snapshot.lock = pgStatLocal.shmem->lock.stats;
/* using struct assignment due to better type safety */
*lck_snap = *lck_shstats;
LWLockRelease(lcktype_lock);
}
LWLockRelease(lckstat_lock);
}
/*

7
src/include/utils/pgstat_internal.h
View file

@ -466,11 +466,8 @@ typedef struct PgStatShared_IO
typedef struct PgStatShared_Lock
{
/*
* locks[i] protects stats.stats[i]. locks[0] also protects
* stats.stat_reset_timestamp.
*/
LWLock locks[LOCKTAG_LAST_TYPE + 1];
/* lock protects ->stats */
LWLock lock;
PgStat_Lock stats;
} PgStatShared_Lock;