cockroachdb / cockroachdb/cockroach
kvserver: do not block concurrent shared lock acquisitions when releasing a shared lock
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
**Describe the problem**
Shared locks are released using a `ResolveIntent` request, which acquires a write latch over the key that was locked:
https://github.com/cockroachdb/cockroach/blob/c83c57d354741ac36740894f7387c014eb6c09fe/pkg/kv/kvserver/batcheval/cmd_resolve_intent.go#L43
This means that any concurrent shared lock acquisition will conflict on latches with the release:
https://github.com/cockroachdb/cockroach/blob/f342e06c94b54885d6ee0c894ff73841e1cc27d2/pkg/kv/kvserver/batcheval/declare.go#L105-L126
Contrast this to the acquisition path, where we allow concurrent shared lock acquisitions from different transactions.
----
The reason for this latch conflict is that `ResolveIntent{,Range}Request` doesn't know the lock strength of the lock it is trying to resolve. It therefore needs to pessimistically assume that there may be an `Intent` at the key, and if that were the case, it needs to acquire a write latch to resolve it. However, if we were to thread in the expected lock strength of the lock we're resolving (this is already tracked on the client), we could optimize the latches required for releasing shared locks as well. They'd look very similar to the latches we grab on the lock acquisition path, which then allows other transactions trying to acquire shared locks to run concurrently.
This is particularly helpful for workloads using read committed with a particularly hot foreign key row. Read committed transactions use shared locks to uphold FK constraints, which means that a hot FK row is going to have lots of shared lock reads/releases. Moreover, they acquire replicated locks, which increases the duration for which latches will be held during acquisition and release of locks.
cc @nvanbenschoten, we spoke about this last week.
Jira issue: CRDB-49692
Contributor guide
Assessment
This issue has not been assessed yet.