Skip to content

Commit 365ac1a

Browse files
committed
Replace LOCKTAGs with LWLocks
The only advantage that LOCKTAGs (heavy locks) could've had against LWLocks is deadlock prevention and, after some considiration, we were unable to find scenarios where aforementioned deadlock prevention is used. So we have no arguments to keep LOCKTAGs Also add a forgotten LWLockAcquire(AddinShmemInitLock, LW_EXCLUSIVE) Also limit history size with MaxAllocSize / sizeof(HistoryItem), since if we allow history size of INT_MAX we can get easily get invalid memory alloc request size
2 parents ac12329 + e3f40d8 commit 365ac1a

3 files changed

Lines changed: 41 additions & 42 deletions

File tree

collector.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -426,12 +426,10 @@ pgws_collector_main(Datum main_arg)
426426
/* Handle request if any */
427427
if (pgws_collector_hdr->request != NO_REQUEST)
428428
{
429-
LOCKTAG tag;
430429
SHMRequest request;
431430

432-
pgws_init_lock_tag(&tag, PGWS_COLLECTOR_LOCK);
431+
LWLockAcquire(collector_lock, LW_EXCLUSIVE);
433432

434-
LockAcquire(&tag, ExclusiveLock, false, false);
435433
request = pgws_collector_hdr->request;
436434
pgws_collector_hdr->request = NO_REQUEST;
437435

@@ -475,7 +473,7 @@ pgws_collector_main(Datum main_arg)
475473
hash_destroy(profile_hash);
476474
profile_hash = make_profile_hash();
477475
}
478-
LockRelease(&tag, ExclusiveLock, false);
476+
LWLockRelease(collector_lock);
479477
}
480478
}
481479

pg_wait_sampling.c

Lines changed: 32 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,13 @@ shm_mq *pgws_collector_mq = NULL;
6363
uint64 *pgws_proc_queryids = NULL;
6464
CollectorShmqHeader *pgws_collector_hdr = NULL;
6565

66-
/* Receiver (backend) local shm_mq pointers and lock */
66+
/* Receiver (backend) local shm_mq pointers */
6767
static shm_mq *recv_mq = NULL;
6868
static shm_mq_handle *recv_mqh = NULL;
69-
static LOCKTAG queueTag;
69+
70+
/* LWLock pointers */
71+
static LWLock *queue_lock;
72+
LWLock *collector_lock;
7073

7174
/* Hook functions */
7275
#if PG_VERSION_NUM >= 150000
@@ -242,6 +245,10 @@ pgws_shmem_request(void)
242245
prev_shmem_request_hook();
243246

244247
RequestAddinShmemSpace(pgws_shmem_size());
248+
249+
/* We request two different LWLock Tranches for ease of use */
250+
RequestNamedLWLockTranche(PGWS_QUEUE_LOCK_NAME, 1);
251+
RequestNamedLWLockTranche(PGWS_COLLECTOR_LOCK_NAME, 1);
245252
}
246253
#endif
247254

@@ -256,11 +263,16 @@ pgws_shmem_startup(void)
256263
void *pgws;
257264
shm_toc *toc;
258265

266+
LWLockAcquire(AddinShmemInitLock, LW_EXCLUSIVE);
267+
259268
pgws = ShmemInitStruct("pg_wait_sampling", segsize, &found);
260269

261270
if (!found)
262271
{
263272
/* Create shared objects */
273+
queue_lock = &(GetNamedLWLockTranche(PGWS_QUEUE_LOCK_NAME))->lock;
274+
collector_lock = &(GetNamedLWLockTranche(PGWS_COLLECTOR_LOCK_NAME))->lock;
275+
264276
toc = shm_toc_create(PG_WAIT_SAMPLING_MAGIC, pgws, segsize);
265277

266278
pgws_collector_hdr = shm_toc_allocate(toc, sizeof(CollectorShmqHeader));
@@ -283,6 +295,8 @@ pgws_shmem_startup(void)
283295

284296
shmem_initialized = true;
285297

298+
LWLockRelease(AddinShmemInitLock);
299+
286300
if (prev_shmem_startup_hook)
287301
prev_shmem_startup_hook();
288302
}
@@ -305,7 +319,7 @@ pgws_cleanup_callback(int code, Datum arg)
305319
{
306320
elog(DEBUG3, "pg_wait_sampling cleanup: detaching shm_mq and releasing queue lock");
307321
shm_mq_detach(recv_mqh);
308-
LockRelease(&queueTag, ExclusiveLock, false);
322+
LWLockRelease(queue_lock);
309323
}
310324

311325
/*
@@ -325,9 +339,13 @@ _PG_init(void)
325339
* resources in pgws_shmem_startup().
326340
*
327341
* If you change code here, don't forget to also report the modifications
328-
* in pgsp_shmem_request() for pg15 and later.
342+
* in pgws_shmem_request() for pg15 and later.
329343
*/
330344
RequestAddinShmemSpace(pgws_shmem_size());
345+
346+
/* We request two different LWLock Tranches for ease of use */
347+
RequestNamedLWLockTranche(PGWS_QUEUE_LOCK_NAME, 1);
348+
RequestNamedLWLockTranche(PGWS_COLLECTOR_LOCK_NAME, 1);
331349
#endif
332350

333351
pgws_register_wait_collector();
@@ -361,7 +379,8 @@ _PG_init(void)
361379
&pgws_historySize,
362380
5000,
363381
100,
364-
INT_MAX,
382+
/* to avoid error in collector.c:alloc_history */
383+
MaxAllocSize / sizeof(HistoryItem),
365384
PGC_SIGHUP,
366385
0,
367386
NULL,
@@ -624,22 +643,10 @@ typedef struct
624643
ProfileItem *items;
625644
} Profile;
626645

627-
void
628-
pgws_init_lock_tag(LOCKTAG *tag, uint32 lock)
629-
{
630-
tag->locktag_field1 = PG_WAIT_SAMPLING_MAGIC;
631-
tag->locktag_field2 = lock;
632-
tag->locktag_field3 = 0;
633-
tag->locktag_field4 = 0;
634-
tag->locktag_type = LOCKTAG_USERLOCK;
635-
tag->locktag_lockmethodid = USER_LOCKMETHOD;
636-
}
637-
638646
/* Get array (history or profile data) from shared memory */
639647
static void *
640648
receive_array(SHMRequest request, Size item_size, Size *count)
641649
{
642-
LOCKTAG collectorTag;
643650
shm_mq_result res;
644651
Size len,
645652
i;
@@ -649,14 +656,11 @@ receive_array(SHMRequest request, Size item_size, Size *count)
649656
MemoryContext oldctx;
650657

651658
/* Ensure nobody else trying to send request to queue */
652-
pgws_init_lock_tag(&queueTag, PGWS_QUEUE_LOCK);
653-
LockAcquire(&queueTag, ExclusiveLock, false, false);
654-
655-
pgws_init_lock_tag(&collectorTag, PGWS_COLLECTOR_LOCK);
656-
LockAcquire(&collectorTag, ExclusiveLock, false, false);
659+
LWLockAcquire(queue_lock, LW_EXCLUSIVE);
660+
LWLockAcquire(collector_lock, LW_EXCLUSIVE);
657661
recv_mq = shm_mq_create(pgws_collector_mq, COLLECTOR_QUEUE_SIZE);
658662
pgws_collector_hdr->request = request;
659-
LockRelease(&collectorTag, ExclusiveLock, false);
663+
LWLockRelease(collector_lock);
660664

661665
/*
662666
* Check that the collector was started to avoid NULL
@@ -716,7 +720,7 @@ receive_array(SHMRequest request, Size item_size, Size *count)
716720

717721
/* We still have to detach and release lock during normal operation. */
718722
shm_mq_detach(recv_mqh);
719-
LockRelease(&queueTag, ExclusiveLock, false);
723+
LWLockRelease(queue_lock);
720724

721725
return result;
722726
}
@@ -822,18 +826,12 @@ PG_FUNCTION_INFO_V1(pg_wait_sampling_reset_profile);
822826
Datum
823827
pg_wait_sampling_reset_profile(PG_FUNCTION_ARGS)
824828
{
825-
LOCKTAG collectorTag;
826-
827829
check_shmem();
828830

829-
pgws_init_lock_tag(&queueTag, PGWS_QUEUE_LOCK);
830-
831-
LockAcquire(&queueTag, ExclusiveLock, false, false);
832-
833-
pgws_init_lock_tag(&collectorTag, PGWS_COLLECTOR_LOCK);
834-
LockAcquire(&collectorTag, ExclusiveLock, false, false);
831+
LWLockAcquire(queue_lock, LW_EXCLUSIVE);
832+
LWLockAcquire(collector_lock, LW_EXCLUSIVE);
835833
pgws_collector_hdr->request = PROFILE_RESET;
836-
LockRelease(&collectorTag, ExclusiveLock, false);
834+
LWLockRelease(collector_lock);
837835

838836
/*
839837
* Check that the collector was started to avoid NULL
@@ -845,7 +843,7 @@ pg_wait_sampling_reset_profile(PG_FUNCTION_ARGS)
845843

846844
SetLatch(pgws_collector_hdr->latch);
847845

848-
LockRelease(&queueTag, ExclusiveLock, false);
846+
LWLockRelease(queue_lock);
849847

850848
PG_RETURN_VOID();
851849
}

pg_wait_sampling.h

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,14 @@
1313
#include "datatype/timestamp.h"
1414
#include "storage/latch.h"
1515
#include "storage/lock.h"
16+
#include "storage/lwlock.h"
1617
#include "storage/shm_mq.h"
1718

1819
#define PG_WAIT_SAMPLING_MAGIC 0xCA94B107
1920
#define COLLECTOR_QUEUE_SIZE (16 * 1024)
2021
#define HISTORY_TIME_MULTIPLIER 10
21-
#define PGWS_QUEUE_LOCK 0
22-
#define PGWS_COLLECTOR_LOCK 1
22+
#define PGWS_QUEUE_LOCK_NAME "pgws_queue_lock"
23+
#define PGWS_COLLECTOR_LOCK_NAME "pgws_collector_lock"
2324

2425
typedef struct
2526
{
@@ -68,10 +69,12 @@ extern int pgws_profileQueries;
6869
extern bool pgws_sampleCpu;
6970

7071
/* pg_wait_sampling.c */
71-
extern CollectorShmqHeader *pgws_collector_hdr;
7272
extern shm_mq *pgws_collector_mq;
7373
extern uint64 *pgws_proc_queryids;
74-
extern void pgws_init_lock_tag(LOCKTAG *tag, uint32 lock);
74+
extern CollectorShmqHeader *pgws_collector_hdr;
75+
76+
extern LWLock *collector_lock;
77+
7578
extern bool pgws_should_sample_proc(PGPROC *proc, int *pid_p, uint32 *wait_event_info_p);
7679

7780
/* collector.c */

0 commit comments

Comments
 (0)