kv: fix lock promotion state mismatch in tryFreeLockOnReplicatedAcquire #122009
Conversation
arulajmani
added
the
backport-24.1.x
|
This change is |
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @arulajmani)
pkg/kv/kvserver/concurrency/testdata/lock_table/lock_promotion line 50 at r1 (raw file):
active: false req: 2 promoting: true, strength: Intent, txn: 00000000-0000-0000-0000-000000000001 # Replicated lock acquisition will cause us to forget the unreplicated lock.
nit: update this comment?
pkg/kv/kvserver/concurrency/testdata/lock_table/lock_promotion line 58 at r1 (raw file):
active: false req: 2 promoting: true, strength: Intent, txn: 00000000-0000-0000-0000-000000000001 dequeue r=req2
nit: should we keep the (no-op) dequeue to make this behave like it will in practice?
Previously, when a lock was free-ed on replicated re-acquisition, we woudln't release any locking requests from the lock holder's transaction. Now, if the re-acquisition corresponded to a lock promotion, we would leave the lock's wait queue in mismatched state -- the request that acquired the replicated lock would think it's promoting, but we just released the lock. This mismatched state was short lived (and thus benign), as the lock acquisition was quickly followed by a call to `Dequeue` that would do the right thing. However, this would have prevented us from making verification claims around lock promoters (see cockroachdb#120909), so it's worth addressing. Informs cockroachdb#120909 Release note: None
There was a problem hiding this comment.
TFTR!
bors r=nvanbenschoten
Reviewable status:
pkg/kv/kvserver/concurrency/testdata/lock_table/lock_promotion line 50 at r1 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
nit: update this comment?
I assume you were hinting towards clarification about what happens to the inactive waiter in the lock's wait queue? If so, done.
pkg/kv/kvserver/concurrency/testdata/lock_table/lock_promotion line 58 at r1 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
nit: should we keep the (no-op)
dequeueto make this behave like it will in practice?
Done.
Previously, when a lock was free-ed on replicated re-acquisition, we woudln't release any locking requests from the lock holder's transaction. Now, if the re-acquisition corresponded to a lock promotion, we would leave the lock's wait queue in mismatched state -- the request that acquired the replicated lock would think it's promoting, but we just released the lock.
This mismatched state was short lived (and thus benign), as the lock acquisition was quickly followed by a call to
Dequeuethat would do the right thing. However, this would have prevented us from making verification claims around lock promoters (see #120909), so it's worth addressing.Informs #120909
Release note: None