v3-0001-Use-atomic-old_snapshot_threshold.patch

From 7de7eb8a707117a692f3c6489b2b13167bd63516 Mon Sep 17 00:00:00 2001
From: Maxim Orlov <orlovmg@gmail.com>
Date: Wed, 11 Jan 2023 18:07:10 +0300
Subject: [PATCH v3] Use atomic old_snapshot_threshold

Using spinlock to access old_snapshot_threshold lead to the bottleneck on
replica, since GetOldSnapshotThresholdTimestamp is called too often. So, switch
to an atomic value.
---
 src/backend/utils/time/snapmgr.c | 16 ++++++----------
 src/include/utils/old_snapshot.h | 18 +++++++++++++++---
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index 7d11ae3478..e1dc8b0feb 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -230,7 +230,7 @@ SnapMgrInit(void)
 		oldSnapshotControl->latest_xmin = InvalidTransactionId;
 		oldSnapshotControl->next_map_update = 0;
 		SpinLockInit(&oldSnapshotControl->mutex_threshold);
-		oldSnapshotControl->threshold_timestamp = 0;
+		pg_atomic_init_u64(&oldSnapshotControl->threshold_timestamp, 0);
 		oldSnapshotControl->threshold_xid = InvalidTransactionId;
 		oldSnapshotControl->head_offset = 0;
 		oldSnapshotControl->head_timestamp = 0;
@@ -1706,9 +1706,7 @@ GetOldSnapshotThresholdTimestamp(void)
 {
 	TimestampTz threshold_timestamp;
 
-	SpinLockAcquire(&oldSnapshotControl->mutex_threshold);
-	threshold_timestamp = oldSnapshotControl->threshold_timestamp;
-	SpinLockRelease(&oldSnapshotControl->mutex_threshold);
+	threshold_timestamp = pg_atomic_read_u64(&oldSnapshotControl->threshold_timestamp);
 
 	return threshold_timestamp;
 }
@@ -1717,9 +1715,9 @@ void
 SetOldSnapshotThresholdTimestamp(TimestampTz ts, TransactionId xlimit)
 {
 	SpinLockAcquire(&oldSnapshotControl->mutex_threshold);
-	Assert(oldSnapshotControl->threshold_timestamp <= ts);
+	Assert(pg_atomic_read_u64(&oldSnapshotControl->threshold_timestamp) <= ts);
 	Assert(TransactionIdPrecedesOrEquals(oldSnapshotControl->threshold_xid, xlimit));
-	oldSnapshotControl->threshold_timestamp = ts;
+	pg_atomic_write_u64(&oldSnapshotControl->threshold_timestamp, ts);
 	oldSnapshotControl->threshold_xid = xlimit;
 	SpinLockRelease(&oldSnapshotControl->mutex_threshold);
 }
@@ -1739,9 +1737,7 @@ SnapshotTooOldMagicForTest(void)
 
 	ts -= 5 * USECS_PER_SEC;
 
-	SpinLockAcquire(&oldSnapshotControl->mutex_threshold);
-	oldSnapshotControl->threshold_timestamp = ts;
-	SpinLockRelease(&oldSnapshotControl->mutex_threshold);
+	pg_atomic_write_u64(&oldSnapshotControl->threshold_timestamp, ts);
 }
 
 /*
@@ -1846,7 +1842,7 @@ TransactionIdLimitedForOldSnapshots(TransactionId recentXmin,
 
 		/* Check for fast exit without LW locking. */
 		SpinLockAcquire(&oldSnapshotControl->mutex_threshold);
-		threshold_timestamp = oldSnapshotControl->threshold_timestamp;
+		threshold_timestamp = pg_atomic_read_u64(&oldSnapshotControl->threshold_timestamp);
 		threshold_xid = oldSnapshotControl->threshold_xid;
 		SpinLockRelease(&oldSnapshotControl->mutex_threshold);
 
diff --git a/src/include/utils/old_snapshot.h b/src/include/utils/old_snapshot.h
index f1978a28e1..7f26006fdd 100644
--- a/src/include/utils/old_snapshot.h
+++ b/src/include/utils/old_snapshot.h
@@ -16,6 +16,7 @@
 #define OLD_SNAPSHOT_H
 
 #include "datatype/timestamp.h"
+#include "port/atomics.h"
 #include "storage/s_lock.h"
 
 /*
@@ -32,9 +33,20 @@ typedef struct OldSnapshotControlData
 	slock_t		mutex_latest_xmin;	/* protect latest_xmin and next_map_update */
 	TransactionId latest_xmin;	/* latest snapshot xmin */
 	TimestampTz next_map_update;	/* latest snapshot valid up to */
-	slock_t		mutex_threshold;	/* protect threshold fields */
-	TimestampTz threshold_timestamp;	/* earlier snapshot is old */
-	TransactionId threshold_xid;	/* earlier xid may be gone */
+	/*
+	 * Use the spinlock to protect:
+	 *  - the simultaneous access to the threshold_timestamp and the
+	 *	  threshold_xid;
+	 *  - access to the threshold_xid.
+	 *
+	 * NOTE: if threshold fields should be accessed simultaneously, we must use
+	 * spinlock to protect them. If we only read threshold_timestamp, we may
+	 * use atomicity, thus avoid using spinlock. This become critical in some
+	 * workloads.
+	 */
+	slock_t		mutex_threshold;
+	pg_atomic_uint64 threshold_timestamp;	/* earlier snapshot is old */
+	TransactionId threshold_xid;			/* earlier xid may be gone */
 
 	/*
 	 * Keep one xid per minute for old snapshot error handling.
-- 
2.38.1