Skip to content

Commit b472c79

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
1 parent 91b163d commit b472c79

3 files changed

Lines changed: 35 additions & 41 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: 26 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,13 @@ shm_mq *pgws_collector_mq = NULL;
6060
uint64 *pgws_proc_queryids = NULL;
6161
CollectorShmqHeader *pgws_collector_hdr = NULL;
6262

63-
/* Receiver (backend) local shm_mq pointers and lock */
63+
/* Receiver (backend) local shm_mq pointers */
6464
static shm_mq *recv_mq = NULL;
6565
static shm_mq_handle *recv_mqh = NULL;
66-
static LOCKTAG queueTag;
66+
67+
/* LWLock pointers */
68+
static LWLock *queue_lock;
69+
LWLock *collector_lock;
6770

6871
/* Hook functions */
6972
#if PG_VERSION_NUM >= 150000
@@ -239,6 +242,10 @@ pgws_shmem_request(void)
239242
prev_shmem_request_hook();
240243

241244
RequestAddinShmemSpace(pgws_shmem_size());
245+
246+
/* We request two different LWLock Tranches for ease of use */
247+
RequestNamedLWLockTranche(PGWS_QUEUE_LOCK_NAME, 1);
248+
RequestNamedLWLockTranche(PGWS_COLLECTOR_LOCK_NAME, 1);
242249
}
243250
#endif
244251

@@ -258,6 +265,9 @@ pgws_shmem_startup(void)
258265
if (!found)
259266
{
260267
/* Create shared objects */
268+
queue_lock = &(GetNamedLWLockTranche(PGWS_QUEUE_LOCK_NAME))->lock;
269+
collector_lock = &(GetNamedLWLockTranche(PGWS_COLLECTOR_LOCK_NAME))->lock;
270+
261271
toc = shm_toc_create(PG_WAIT_SAMPLING_MAGIC, pgws, segsize);
262272

263273
pgws_collector_hdr = shm_toc_allocate(toc, sizeof(CollectorShmqHeader));
@@ -302,7 +312,7 @@ pgws_cleanup_callback(int code, Datum arg)
302312
{
303313
elog(DEBUG3, "pg_wait_sampling cleanup: detaching shm_mq and releasing queue lock");
304314
shm_mq_detach(recv_mqh);
305-
LockRelease(&queueTag, ExclusiveLock, false);
315+
LWLockRelease(queue_lock);
306316
}
307317

308318
/*
@@ -322,9 +332,13 @@ _PG_init(void)
322332
* resources in pgws_shmem_startup().
323333
*
324334
* If you change code here, don't forget to also report the modifications
325-
* in pgsp_shmem_request() for pg15 and later.
335+
* in pgws_shmem_request() for pg15 and later.
326336
*/
327337
RequestAddinShmemSpace(pgws_shmem_size());
338+
339+
/* We request two different LWLock Tranches for ease of use */
340+
RequestNamedLWLockTranche(PGWS_QUEUE_LOCK_NAME, 1);
341+
RequestNamedLWLockTranche(PGWS_COLLECTOR_LOCK_NAME, 1);
328342
#endif
329343

330344
pgws_register_wait_collector();
@@ -619,22 +633,10 @@ typedef struct
619633
ProfileItem *items;
620634
} Profile;
621635

622-
void
623-
pgws_init_lock_tag(LOCKTAG *tag, uint32 lock)
624-
{
625-
tag->locktag_field1 = PG_WAIT_SAMPLING_MAGIC;
626-
tag->locktag_field2 = lock;
627-
tag->locktag_field3 = 0;
628-
tag->locktag_field4 = 0;
629-
tag->locktag_type = LOCKTAG_USERLOCK;
630-
tag->locktag_lockmethodid = USER_LOCKMETHOD;
631-
}
632-
633636
/* Get array (history or profile data) from shared memory */
634637
static void *
635638
receive_array(SHMRequest request, Size item_size, Size *count)
636639
{
637-
LOCKTAG collectorTag;
638640
shm_mq_result res;
639641
Size len,
640642
i;
@@ -644,14 +646,11 @@ receive_array(SHMRequest request, Size item_size, Size *count)
644646
MemoryContext oldctx;
645647

646648
/* Ensure nobody else trying to send request to queue */
647-
pgws_init_lock_tag(&queueTag, PGWS_QUEUE_LOCK);
648-
LockAcquire(&queueTag, ExclusiveLock, false, false);
649-
650-
pgws_init_lock_tag(&collectorTag, PGWS_COLLECTOR_LOCK);
651-
LockAcquire(&collectorTag, ExclusiveLock, false, false);
649+
LWLockAcquire(queue_lock, LW_EXCLUSIVE);
650+
LWLockAcquire(collector_lock, LW_EXCLUSIVE);
652651
recv_mq = shm_mq_create(pgws_collector_mq, COLLECTOR_QUEUE_SIZE);
653652
pgws_collector_hdr->request = request;
654-
LockRelease(&collectorTag, ExclusiveLock, false);
653+
LWLockRelease(collector_lock);
655654

656655
/*
657656
* Check that the collector was started to avoid NULL
@@ -711,7 +710,7 @@ receive_array(SHMRequest request, Size item_size, Size *count)
711710

712711
/* We still have to detach and release lock during normal operation. */
713712
shm_mq_detach(recv_mqh);
714-
LockRelease(&queueTag, ExclusiveLock, false);
713+
LWLockRelease(queue_lock);
715714

716715
return result;
717716
}
@@ -814,18 +813,12 @@ PG_FUNCTION_INFO_V1(pg_wait_sampling_reset_profile);
814813
Datum
815814
pg_wait_sampling_reset_profile(PG_FUNCTION_ARGS)
816815
{
817-
LOCKTAG collectorTag;
818-
819816
check_shmem();
820817

821-
pgws_init_lock_tag(&queueTag, PGWS_QUEUE_LOCK);
822-
823-
LockAcquire(&queueTag, ExclusiveLock, false, false);
824-
825-
pgws_init_lock_tag(&collectorTag, PGWS_COLLECTOR_LOCK);
826-
LockAcquire(&collectorTag, ExclusiveLock, false, false);
818+
LWLockAcquire(queue_lock, LW_EXCLUSIVE);
819+
LWLockAcquire(collector_lock, LW_EXCLUSIVE);
827820
pgws_collector_hdr->request = PROFILE_RESET;
828-
LockRelease(&collectorTag, ExclusiveLock, false);
821+
LWLockRelease(collector_lock);
829822

830823
/*
831824
* Check that the collector was started to avoid NULL
@@ -837,7 +830,7 @@ pg_wait_sampling_reset_profile(PG_FUNCTION_ARGS)
837830

838831
SetLatch(pgws_collector_hdr->latch);
839832

840-
LockRelease(&queueTag, ExclusiveLock, false);
833+
LWLockRelease(queue_lock);
841834

842835
PG_RETURN_VOID();
843836
}

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)