From 29a9db3e0711078df07aa622e5156db30a84b3ab Mon Sep 17 00:00:00 2001 From: Arul Ajmani Date: Mon, 31 Jul 2023 11:51:18 -0400 Subject: [PATCH] concurrency: enable joint claims on the request scan path This patch addresses a TODO that was blocking joint claims on a request's scan path and adds a test to show joint claims work. In particular, previously the lock mode of a queued request was hardcoded to use locking strength `lock.Intent`. We now use the correct lock mode by snapshotting an immutable copy of the request's lock strength on to the queuedGuard when inserting a it into a lock's wait queue. Informs #102275 Release note: None --- pkg/kv/kvserver/concurrency/lock_table.go | 36 ++++++++++--------- .../testdata/lock_table/shared_locks | 32 +++++++++++------ 2 files changed, 41 insertions(+), 27 deletions(-) diff --git a/pkg/kv/kvserver/concurrency/lock_table.go b/pkg/kv/kvserver/concurrency/lock_table.go index 4d4ee57be37a..f20ab052549d 100644 --- a/pkg/kv/kvserver/concurrency/lock_table.go +++ b/pkg/kv/kvserver/concurrency/lock_table.go @@ -764,23 +764,28 @@ func (g *lockTableGuardImpl) curStrength() lock.Strength { // request. The value returned by this method are mutable as the request's scan // of the lock table progresses from lock to lock. func (g *lockTableGuardImpl) curLockMode() lock.Mode { - switch g.curStrength() { + return makeLockMode(g.curStrength(), g.txn, g.ts) +} + +// makeLockMode constructs and returns a lock mode. +func makeLockMode(str lock.Strength, txn *roachpb.Transaction, ts hlc.Timestamp) lock.Mode { + switch str { case lock.None: iso := isolation.Serializable - if g.txn != nil { - iso = g.txn.IsoLevel + if txn != nil { + iso = txn.IsoLevel } - return lock.MakeModeNone(g.ts, iso) + return lock.MakeModeNone(ts, iso) case lock.Shared: - assert(g.txn != nil, "only transactional requests can acquire shared locks") + assert(txn != nil, "only transactional requests can acquire shared locks") return lock.MakeModeShared() case lock.Exclusive: - assert(g.txn != nil, "only transactional requests can acquire exclusive locks") - return lock.MakeModeExclusive(g.ts, g.txn.IsoLevel) + assert(txn != nil, "only transactional requests can acquire exclusive locks") + return lock.MakeModeExclusive(ts, txn.IsoLevel) case lock.Intent: - return lock.MakeModeIntent(g.ts) + return lock.MakeModeIntent(ts) default: - panic(fmt.Sprintf("unhandled request strength: %s", g.curStrength())) + panic(fmt.Sprintf("unhandled request strength: %s", str)) } } @@ -919,7 +924,8 @@ func (g *lockTableGuardImpl) resumeScan(notify bool) { // and non-transactional requests. type queuedGuard struct { guard *lockTableGuardImpl - active bool // protected by lockState.mu + mode lock.Mode // protected by lockState.mu + active bool // protected by lockState.mu } // Information about a lock holder for unreplicated locks. @@ -2201,6 +2207,7 @@ func (l *lockState) enqueueLockingRequest(g *lockTableGuardImpl) (maxQueueLength } qg := &queuedGuard{ guard: g, + mode: g.curLockMode(), active: true, } // The request isn't in the queue. Add it in the correct position, based on @@ -2281,13 +2288,7 @@ func (l *lockState) shouldRequestActivelyWait(g *lockTableGuardImpl) bool { // conflicting waiters; no need to actively wait here. return false } - // TODO(arul): Inactive waiters will need to capture the strength at which - // they're trying to acquire a lock in their queuedGuard. We can't simply - // use the guard's curStrength (or curLockMode) -- inactive waiters may have - // mutated these values as they scan. For now, we can just use the intent - // lock mode as that's the only lock strength supported by the lock table. - waiterLockMode := lock.MakeModeIntent(qqg.guard.ts) - if lock.Conflicts(waiterLockMode, g.curLockMode(), &g.lt.settings.SV) { + if lock.Conflicts(qqg.mode, g.curLockMode(), &g.lt.settings.SV) { return true } } @@ -2691,6 +2692,7 @@ func (l *lockState) discoveredLock( // Put self in queue as inactive waiter. qg := &queuedGuard{ guard: g, + mode: makeLockMode(accessStrength, g.txn, g.ts), active: false, } // g is not necessarily first in the queue in the (rare) case (a) above. diff --git a/pkg/kv/kvserver/concurrency/testdata/lock_table/shared_locks b/pkg/kv/kvserver/concurrency/testdata/lock_table/shared_locks index ef8d29d908d0..daeafde70dc5 100644 --- a/pkg/kv/kvserver/concurrency/testdata/lock_table/shared_locks +++ b/pkg/kv/kvserver/concurrency/testdata/lock_table/shared_locks @@ -38,6 +38,15 @@ scan r=req3 ---- start-waiting: false +# Another shared locking request should be able to acquire a joint claim as +# well. +new-request r=req4 txn=txn2 ts=10 spans=shared@a +---- + +scan r=req4 +---- +start-waiting: false + print ---- num=1 @@ -45,6 +54,7 @@ num=1 holder: txn: 00000000-0000-0000-0000-000000000001 epoch: 0, iso: Serializable, ts: 10.000000000,0, info: unrepl [(str: Shared seq: 0)] queued writers: active: false req: 2, txn: 00000000-0000-0000-0000-000000000002 + active: false req: 3, txn: 00000000-0000-0000-0000-000000000002 # TODO(arul): This is currently a limitation of the lock table, as it doesn't # allow multiple locks on a single key from different transactions. @@ -52,17 +62,17 @@ acquire r=req3 k=a durability=u strength=shared ---- existing lock cannot be acquired by different transaction -new-request r=req4 txn=txn2 ts=10 spans=exclusive@a +new-request r=req5 txn=txn2 ts=10 spans=exclusive@a ---- -scan r=req4 +scan r=req5 ---- start-waiting: true -new-request r=req5 txn=txn2 ts=10 spans=intent@a +new-request r=req6 txn=txn2 ts=10 spans=intent@a ---- -scan r=req5 +scan r=req6 ---- start-waiting: true @@ -73,9 +83,10 @@ num=1 holder: txn: 00000000-0000-0000-0000-000000000001 epoch: 0, iso: Serializable, ts: 10.000000000,0, info: unrepl [(str: Shared seq: 0)] queued writers: active: false req: 2, txn: 00000000-0000-0000-0000-000000000002 - active: true req: 3, txn: 00000000-0000-0000-0000-000000000002 + active: false req: 3, txn: 00000000-0000-0000-0000-000000000002 active: true req: 4, txn: 00000000-0000-0000-0000-000000000002 - distinguished req: 3 + active: true req: 5, txn: 00000000-0000-0000-0000-000000000002 + distinguished req: 4 # ------------------------------------------------------------------------------ # Ensure requests with locking strength shared actively wait if there are active @@ -83,10 +94,10 @@ num=1 # compatible with the shared lock request). # ------------------------------------------------------------------------------ -new-request r=req6 txn=txn2 ts=10 spans=shared@a +new-request r=req7 txn=txn2 ts=10 spans=shared@a ---- -scan r=req6 +scan r=req7 ---- start-waiting: true @@ -97,7 +108,8 @@ num=1 holder: txn: 00000000-0000-0000-0000-000000000001 epoch: 0, iso: Serializable, ts: 10.000000000,0, info: unrepl [(str: Shared seq: 0)] queued writers: active: false req: 2, txn: 00000000-0000-0000-0000-000000000002 - active: true req: 3, txn: 00000000-0000-0000-0000-000000000002 + active: false req: 3, txn: 00000000-0000-0000-0000-000000000002 active: true req: 4, txn: 00000000-0000-0000-0000-000000000002 active: true req: 5, txn: 00000000-0000-0000-0000-000000000002 - distinguished req: 3 + active: true req: 6, txn: 00000000-0000-0000-0000-000000000002 + distinguished req: 4