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: synchronize replica destruction on readOnlyCmdMu #10583

Merged
merged 7 commits into from
Nov 10, 2016
Merged

storage: synchronize replica destruction on readOnlyCmdMu #10583

merged 7 commits into from
Nov 10, 2016

Conversation

tamird
Copy link
Contributor

@tamird tamird commented Nov 10, 2016

This change is Reviewable

@tamird
Copy link
Contributor Author

tamird commented Nov 10, 2016

This causes TestReplicaCommandQueueInconsistent to deadlock. I'm investigating.

@tamird tamird changed the title storage: synchronize inconsistent reads on Replica.raftMu storage: synchronize replica destruction on readOnlyCmdMu Nov 10, 2016
@tamird
Copy link
Contributor Author

tamird commented Nov 10, 2016

This ready for a look.

@petermattis
Copy link
Collaborator

:lgtm:


Reviewed 1 of 1 files at r1, 1 of 1 files at r2, 1 of 1 files at r3, 1 of 1 files at r4, 1 of 1 files at r5, 2 of 2 files at r6.
Review status: 2 of 3 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


pkg/storage/store.go, line 2034 at r7 (raw file):

      log.Fatalf(ctx, "unexpectedly overlapped by %v", placeholder)
  }
  delete(s.mu.replicaPlaceholders, rep.RangeID)

Was there a purpose to the movement of this deletion? Seemed more consistent where it was previously (next to the other map deletions).


Comments from Reviewable

@tamird
Copy link
Contributor Author

tamird commented Nov 10, 2016

Review status: 2 of 3 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


pkg/storage/store.go, line 2034 at r7 (raw file):

Previously, petermattis (Peter Mattis) wrote…

Was there a purpose to the movement of this deletion? Seemed more consistent where it was previously (next to the other map deletions).

It was to more closely match the bodies of `addPlaceholderLocked` and `removePlaceholderLocked`.

Comments from Reviewable

@tamird tamird merged commit 3848e3e into cockroachdb:master Nov 10, 2016
@tamird tamird deleted the fix-replica-destroy-race branch November 10, 2016 16:49
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.

2 participants