Skip to content

kv: acquire Replica shared mutex in tryGetOrCreateReplica#74356

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
nvb:nvanbenschoten/assortedPerf18
Jan 3, 2022
Merged

kv: acquire Replica shared mutex in tryGetOrCreateReplica#74356
craig[bot] merged 1 commit intocockroachdb:masterfrom
nvb:nvanbenschoten/assortedPerf18

Conversation

@nvb
Copy link
Copy Markdown
Contributor

@nvb nvb commented Dec 31, 2021

This commit switches the common path in tryGetOrCreateReplica from
grabbing the Replica mutex in an exclusive mode to grabbing it in a
shared mode. The common path in tryGetOrCreateReplica does not mutate
the Replica, so there's no need for the stronger lock.

The locking in setLastReplicaDescriptors showed up in a mutex profile
under a write-heavy workload. It was responsible for 2.06% of mutex
wait time. Grabbing the mutex was probably also slowing down Raft
request processing, as the exclusive lock acquisition had to wait for
other read locks to be dropped.

Screen Shot 2021-12-30 at 9 45 26 PM

@nvb nvb requested a review from erikgrinaker December 31, 2021 03:01
@nvb nvb requested a review from a team as a code owner December 31, 2021 03:01
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

This commit switches the common path in `tryGetOrCreateReplica` from
grabbing the Replica mutex in an exclusive mode to grabbing it in a
shared mode. The common path in `tryGetOrCreateReplica` does not mutate
the Replica, so there's no need for the stronger lock.

The locking in `setLastReplicaDescriptors` showed up in a mutex profile
under a write-heavy workload. It was responsible for **2.06%** of mutex
wait time. Grabbing the mutex was probably also slowing down Raft
request processing, as the exclusive lock acquisition had to wait for
other read locks to be dropped.
@nvb nvb force-pushed the nvanbenschoten/assortedPerf18 branch from f4225f0 to 36e3163 Compare December 31, 2021 07:55
Copy link
Copy Markdown
Member

@tbg tbg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @erikgrinaker)

@nvb
Copy link
Copy Markdown
Contributor Author

nvb commented Jan 3, 2022

TFTR!

bors r=tbg

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Jan 3, 2022

Build succeeded:

@craig craig bot merged commit 5669b9d into cockroachdb:master Jan 3, 2022
@nvb nvb deleted the nvanbenschoten/assortedPerf18 branch January 6, 2022 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants