v3-0002-WIP-Provide-a-lock-free-fast-path-for-smgrnblocks.patch

From 2c3c98711bdadd0846c4dfff3aea6cdf84f6df44 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Thu, 19 Nov 2020 17:09:51 +1300
Subject: [PATCH v3 2/2] WIP: Provide a lock-free fast path for smgrnblocks().
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

SMgrRelation objects gain a pointer to the last known SMgrSharedRelation
object.  There are three concurrency hazards to worry about:

1.  The SMgrSharedRelation pointed to could be evicted at any time.  We
record a generation number, and insert memory barriers so that we can
detect that and fall back to a slower path.

2.  The nblocks value is read without any locking, which is atomic
because it is a 32 bit value, and PostgreSQL requires atomic 32 bit
reads generally.

3.  The nblocks value must be fresh enough for scans, extension,
truncatation and dropping buffers, because the those operations all
executed memory barriers when it acquired a snapshot to scan (which
doesn't need to see blocks added after that) or an exclusive heavyweight
lock to extend, truncate or drop.  XXX right?

XXX That's the idea anyway, but this is just a sketch; almost certainly
incomplet and inkorrect.

Discussion: https://postgr.es/m/CAEepm%3D3SSw-Ty1DFcK%3D1rU-K6GSzYzfdD4d%2BZwapdN7dTa6%3DnQ%40mail.gmail.com
Reviewed-by: 陈佳昕(步真) <buzhen.cjx@alibaba-inc.com>
Reviewed-by: Andy Fan <zhihui.fan1213@gmail.com>
---
 src/backend/storage/smgr/smgr.c | 63 +++++++++++++++++++++++++++++++--
 src/include/storage/smgr.h      |  4 +++
 2 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/src/backend/storage/smgr/smgr.c b/src/backend/storage/smgr/smgr.c
index 1da2dfc250..5c17e3e74d 100644
--- a/src/backend/storage/smgr/smgr.c
+++ b/src/backend/storage/smgr/smgr.c
@@ -50,6 +50,7 @@ struct SMgrSharedRelation
 	RelFileNodeBackend rnode;
 	BlockNumber		nblocks[MAX_FORKNUM + 1];
 	pg_atomic_uint32 flags;
+	pg_atomic_uint64 generation;		/* mapping change */
 };
 
 /* For now, we borrow the buffer managers array of locks.  XXX fixme */
@@ -156,6 +157,40 @@ static void smgrshutdown(int code, Datum arg);
 /* GUCs. */
 int smgr_shared_relations = 1000;
 
+/*
+ * Try to get the size of a relation's fork without locking.
+ */
+static BlockNumber
+smgrnblocks_fast(SMgrRelation reln, ForkNumber forknum)
+{
+	SMgrSharedRelation *sr = reln->smgr_shared;
+	BlockNumber result;
+
+	if (sr)
+	{
+		pg_read_barrier();
+
+		/* We can load int-sized values atomically without special measures. */
+		Assert(sizeof(sr->nblocks[forknum]) == sizeof(uint32));
+		result = sr->nblocks[forknum];
+
+		/*
+		 * With a read barrier between the loads, we can check that the object
+		 * still refers to the same rnode before trusting the answer.
+		 */
+		pg_read_barrier();
+		if (pg_atomic_read_u64(&sr->generation) == reln->smgr_shared_generation)
+			return result;
+
+		/*
+		 * The generation doesn't match, the shared relation must have been
+		 * evicted since we got a pointer to it.  We'll need to do more work.
+		 */
+	}
+
+	return InvalidBlockNumber;
+}
+
 /*
  * Try to get the size of a relation's fork by looking it up in the mapping
  * table with a shared lock.  This will succeed if the SMgrRelation already
@@ -183,6 +218,10 @@ smgrnblocks_shared(SMgrRelation reln, ForkNumber forknum)
 	{
 		sr = &sr_pool->objects[mapping->index];
 		result = sr->nblocks[forknum];
+
+		/* We can take the fast path until this SR is eventually evicted. */
+		reln->smgr_shared = sr;
+		reln->smgr_shared_generation = pg_atomic_read_u64(&sr->generation);
 	}
 	LWLockRelease(mapping_lock);
 
@@ -339,9 +378,14 @@ smgr_alloc_sr(void)
 
 	/*
 	 * If we made it this far, there are no dirty forks, so we're now allowed
-	 * to evict the SR from the pool and the mapping table.
+	 * to evict the SR from the pool and the mapping table.  Make sure that
+	 * smgrnblocks_fast() sees that its pointer is now invalid by bumping the
+	 * generation.
 	 */
 	flags &= ~SR_VALID;
+	pg_atomic_write_u64(&sr->generation,
+						pg_atomic_read_u64(&sr->generation) + 1);
+	pg_write_barrier();
 	smgr_unlock_sr(sr, flags);
 
 	/*
@@ -455,6 +499,8 @@ smgrnblocks_update(SMgrRelation reln,
 			mapping->index = sr - sr_pool->objects;
 			smgr_unlock_sr(sr, SR_VALID);
 			sr->rnode = reln->smgr_rnode;
+			pg_atomic_write_u64(&sr->generation,
+								pg_atomic_read_u64(&sr->generation) + 1);
 			for (int i = 0; i <= MAX_FORKNUM; ++i)
 				sr->nblocks[i] = InvalidBlockNumber;
 			LWLockRelease(mapping_lock);
@@ -509,6 +555,11 @@ retry:
 		}
 		ConditionVariableCancelSleep();
 
+		/* Make sure smgrnblocks_fast() knows it's invalidated. */
+		pg_atomic_write_u64(&sr->generation,
+							pg_atomic_read_u64(&sr->generation) + 1);
+		pg_write_barrier();
+
 		/* Mark it invalid and drop the mapping. */
 		smgr_unlock_sr(sr, ~SR_VALID);
 		hash_search_with_hash_value(sr_mapping_table,
@@ -561,6 +612,7 @@ smgr_shmem_init(void)
 		for (uint32 i = 0; i < smgr_shared_relations; ++i)
 		{
 			pg_atomic_init_u32(&sr_pool->objects[i].flags, 0);
+			pg_atomic_init_u64(&sr_pool->objects[i].generation, 0);
 		}
 	}
 }
@@ -640,6 +692,8 @@ smgropen(RelFileNode rnode, BackendId backend)
 		/* hash_search already filled in the lookup key */
 		reln->smgr_owner = NULL;
 		reln->smgr_targblock = InvalidBlockNumber;
+		reln->smgr_shared = NULL;
+		reln->smgr_shared_generation = 0;
 		reln->smgr_which = 0;	/* we only have md.c at present */
 
 		/* implementation-specific initialization */
@@ -710,7 +764,7 @@ smgrclearowner(SMgrRelation *owner, SMgrRelation reln)
 bool
 smgrexists(SMgrRelation reln, ForkNumber forknum)
 {
-	if (smgrnblocks_shared(reln, forknum) != InvalidBlockNumber)
+	if (smgrnblocks_fast(reln, forknum) != InvalidBlockNumber)
 		return true;
 
 	return smgrsw[reln->smgr_which].smgr_exists(reln, forknum);
@@ -1010,6 +1064,11 @@ smgrnblocks(SMgrRelation reln, ForkNumber forknum)
 {
 	BlockNumber result;
 
+	/* Can we get the answer from shared memory without locking? */
+	result = smgrnblocks_fast(reln, forknum);
+	if (result != InvalidBlockNumber)
+		return result;
+
 	/* Can we get the answer from shared memory with only a share lock? */
 	result = smgrnblocks_shared(reln, forknum);
 	if (result != InvalidBlockNumber)
diff --git a/src/include/storage/smgr.h b/src/include/storage/smgr.h
index 7031c70196..fec13cb7bc 100644
--- a/src/include/storage/smgr.h
+++ b/src/include/storage/smgr.h
@@ -51,6 +51,10 @@ typedef struct SMgrRelationData
 	/* pointer to owning pointer, or NULL if none */
 	struct SMgrRelationData **smgr_owner;
 
+	/* pointer to shared object, valid if non-NULL and generation matches */
+	SMgrSharedRelation *smgr_shared;
+	uint64		smgr_shared_generation;
+
 	BlockNumber smgr_targblock; /* current insertion target block */
 
 	/* additional public fields may someday exist here */
-- 
2.20.1