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

storage: Deflake TestSplitSnapshotRace_SnapshotWins #16893

Merged
merged 1 commit into from
Jul 6, 2017

Conversation

bdarnell
Copy link
Contributor

@bdarnell bdarnell commented Jul 6, 2017

multiTestContext was not properly filling in the destination of its
request, leading to a fallback search in Stores.Send which could send
a request to a stale replica when combined with outdated information
in the DistSender's cache.

Specifically, a request to the post-split RHS was being sent to a
pre-split replica, which did not have a quorum because the LHS stores
were stopped, so it spun endlessly trying to get a lease.

Fixes #16251

multiTestContext was not properly filling in the destination of its
request, leading to a fallback search in Stores.Send which could send
a request to a stale replica when combined with outdated information
in the DistSender's cache.

Specifically, a request to the post-split RHS was being sent to a
pre-split replica, which did not have a quorum because the LHS stores
were stopped, so it spun endlessly trying to get a lease.

Fixes cockroachdb#16251
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@petermattis
Copy link
Collaborator

:lgtm:

leading to a fallback search in Stores.Send

I really don't like that fallback search in Stores.Send. I wonder how many tests rely on it.


Review status: 0 of 2 files reviewed at latest revision, all discussions resolved, some commit checks pending.


Comments from Reviewable

@bdarnell
Copy link
Contributor Author

bdarnell commented Jul 6, 2017

Yeah, I've wanted to get rid of it for a long time too. But a lot of tests in the storage package rely on it. (This is issue #9014)


Review status: 0 of 2 files reviewed at latest revision, all discussions resolved, some commit checks pending.


Comments from Reviewable

@bdarnell bdarnell merged commit 54365a7 into cockroachdb:master Jul 6, 2017
@bdarnell bdarnell deleted the deflake-split-snapshot branch July 6, 2017 18:52
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