Skip to content

Commit

Permalink
concurrency: enable joint claims on the request scan path
Browse files Browse the repository at this point in the history
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 cockroachdb#102275

Release note: None
  • Loading branch information
arulajmani committed Aug 2, 2023
1 parent 6805624 commit 110ca56
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 27 deletions.
36 changes: 19 additions & 17 deletions pkg/kv/kvserver/concurrency/lock_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 accessStrength {
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", accessStrength))
}
}

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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.
Expand Down
32 changes: 22 additions & 10 deletions pkg/kv/kvserver/concurrency/testdata/lock_table/shared_locks
Original file line number Diff line number Diff line change
Expand Up @@ -38,31 +38,41 @@ 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
lock: "a"
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.
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

Expand All @@ -73,20 +83,21 @@ 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
# waiters with conflicting lock strengths (even though the lock itself is
# 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

Expand All @@ -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

0 comments on commit 110ca56

Please sign in to comment.