Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kvserver: use replicasByKey addition func in snapshot path #96870

Merged
merged 4 commits into from
Feb 9, 2023

Conversation

pav-kv
Copy link
Collaborator

@pav-kv pav-kv commented Feb 9, 2023

This commit makes one step towards better code sharing between Replica initialization paths: split trigger and snapshot application. It makes both to use the same method to check and insert the initialized Replica to replicasByKey map.

Touches #94912

@pav-kv pav-kv requested review from tbg and a team February 9, 2023 14:12
@pav-kv pav-kv requested a review from a team as a code owner February 9, 2023 14:12
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@pav-kv
Copy link
Collaborator Author

pav-kv commented Feb 9, 2023

@tbg A few small commits to review one by one.

@pav-kv pav-kv force-pushed the reuse_add_to_replicas_by_key branch from 73ae8e7 to ec91985 Compare February 9, 2023 14:17
pkg/kv/kvserver/store_create_replica.go Outdated Show resolved Hide resolved
pkg/kv/kvserver/store_split.go Show resolved Hide resolved
@pav-kv
Copy link
Collaborator Author

pav-kv commented Feb 9, 2023

Some tests timed out. Possibly there is a deadlock.

The reason probably is: updateRangeInfo locks r.mu somewhere down the stack, so we should call it without holding the mutex.

@pav-kv pav-kv force-pushed the reuse_add_to_replicas_by_key branch 3 times, most recently from eff5ec4 to 00fbcbd Compare February 9, 2023 17:58
@pav-kv
Copy link
Collaborator Author

pav-kv commented Feb 9, 2023

bors r=tbg

This commit makes one step towards better code sharing between Replica
initialization paths: split trigger and snapshot application. It makes both to
use the same method to check and insert the initialized Replica to
replicasByKey map.

Release note: none
Epic: none
@pav-kv pav-kv force-pushed the reuse_add_to_replicas_by_key branch from 00fbcbd to f5bbd77 Compare February 9, 2023 18:37
@craig
Copy link
Contributor

craig bot commented Feb 9, 2023

Canceled.

@pav-kv
Copy link
Collaborator Author

pav-kv commented Feb 9, 2023

bors cancel

@pav-kv
Copy link
Collaborator Author

pav-kv commented Feb 9, 2023

bors r=tbg

@craig
Copy link
Contributor

craig bot commented Feb 9, 2023

Build succeeded:

@craig craig bot merged commit 732c3a7 into cockroachdb:master Feb 9, 2023
@pav-kv pav-kv deleted the reuse_add_to_replicas_by_key branch February 9, 2023 19:59
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