From eaf9efd4df49e62b991c7c7347613f64c6269336 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Thu, 19 Nov 2020 17:09:51 +1300 Subject: [PATCH v2 2/2] WIP: Provide a lock-free fast path for smgrnblocks(). 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 free 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 --- src/backend/storage/smgr/smgr.c | 58 +++++++++++++++++++++++++++++++-- src/include/storage/smgr.h | 4 +++ 2 files changed, 60 insertions(+), 2 deletions(-) diff --git a/src/backend/storage/smgr/smgr.c b/src/backend/storage/smgr/smgr.c index b5dc370bba..c96fa46785 100644 --- a/src/backend/storage/smgr/smgr.c +++ b/src/backend/storage/smgr/smgr.c @@ -48,6 +48,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 */ @@ -153,6 +154,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 @@ -180,6 +215,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); @@ -337,9 +376,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); /* Remove from the mapping table. */ @@ -447,6 +491,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); @@ -549,6 +595,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); } } } @@ -629,6 +676,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 */ @@ -699,7 +748,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); @@ -999,6 +1048,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 2eea69213b..b2cab478a2 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