kvserver: acquire Replica.mu when returning reproposal error#117801
kvserver: acquire Replica.mu when returning reproposal error#117801craig[bot] merged 1 commit intocockroachdb:masterfrom
Replica.mu when returning reproposal error#117801Conversation
|
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. |
|
This change is |
| // The tracker wants us to forward the request timestamp, but we can't | ||
| // do that without re-evaluating, so give up. The error returned here | ||
| // will go to back to DistSender, so send something it can digest. | ||
| r.mu.RLock() |
There was a problem hiding this comment.
The lock seems legit. At least, the r.mu.proposalBuf access above is assumed without holding the lock (see prpposalBuf comment), and TrackEvaluatingRequest also locks/unlocks the r.mu.RLock().
We're only holding raftMu here (in the weeds of application flow), and it's ok to lock r.mu after it.
Epic: none Release note: None
c5dab33 to
70bd44d
Compare
|
bors r+ |
|
This PR was included in a batch that timed out, it will be automatically retried |
|
Build succeeded: |
|
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from 70bd44d to blathers/backport-release-23.1-117801: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 23.1.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
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