-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
release-23.2: kvserver: acquire Replica.mu
when returning reproposal error
#117839
release-23.2: kvserver: acquire Replica.mu
when returning reproposal error
#117839
Conversation
Epic: none Release note: None
2290d3a
to
cace08b
Compare
Thanks for opening a backport. Please check the backport criteria before merging:
If your backport adds new functionality, please ensure that the following additional criteria are satisfied:
Also, please add a brief release justification to the body of your PR to justify this |
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems reasonable to backport to me. While you mention there is some chance of this introducing a deadlock, I think it makes sense to rely on our tests here rather than avoiding making a clear correctness change.
Backport 1/1 commits from #117801 on behalf of @erikgrinaker.
/cc @cockroachdb/release
Discovered while working on #117612. Looks like it's been there since #42939.
This is a rare error, and these fields are unlikely to change while we're holding the raft mutex, so seems very unlikely to have caused any problems -- I could be convinced we shouldn't backport this, on the off-chance I've missed a potential deadlock.
Epic: none
Release note: None
Release justification: fixes a rare race condition.