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

raft: NPE in TestRaftAfterRemoveRange #1911

Closed
tamird opened this issue Aug 3, 2015 · 6 comments
Closed

raft: NPE in TestRaftAfterRemoveRange #1911

tamird opened this issue Aug 3, 2015 · 6 comments
Labels
C-test-failure Broken test (automatically or manually discovered). E-intermediate Intermediate complexity, needs a contributor with 3-6 months of past contribution experience. help wanted Help is requested / needed by the one who filed the issue to fix it.

Comments

@tamird
Copy link
Contributor

tamird commented Aug 3, 2015

@tamird tamird added C-test-failure Broken test (automatically or manually discovered). multi-cpu labels Aug 3, 2015
@bdarnell
Copy link
Contributor

bdarnell commented Aug 3, 2015

The problem is that in the Store.processRaft thread, while processing the EndTransaction, we update the range descriptor and then tell raft to apply the configuration change. The test thread calls RemoveReplica as soon as the descriptor is updated. If this happens before the raft ApplyConfChange, it will cause this panic.

We need to make RemoveReplica atomic with respect to commands being processed in Store.processRaft, either by adding a mutex that is held while processing each command, or adding a new channel to the processRaft select loop so that the meat of RemoveReplica can be done in that thread.

@tbg tbg added the PTAL label Aug 3, 2015
@bdarnell bdarnell removed their assignment Aug 3, 2015
@tamird tamird added help wanted Help is requested / needed by the one who filed the issue to fix it. E-easy Easy issue to tackle, requires little or no CockroachDB experience and removed PTAL labels Aug 3, 2015
@iankronquist
Copy link

Hey, I'm interested in investigating this issue.

@tamird
Copy link
Contributor Author

tamird commented Aug 4, 2015

Great, go ahead!

@iankronquist
Copy link

@tamird Your IRC channel looks pretty empty. If I want to get a hold of you is there somewhere else I can ask you questions other than here?

@tamird
Copy link
Contributor Author

tamird commented Aug 4, 2015

Heh, I didn't even know about the IRC channel. I'm in there now.

@tamird
Copy link
Contributor Author

tamird commented Aug 21, 2015

@tamird tamird added E-intermediate Intermediate complexity, needs a contributor with 3-6 months of past contribution experience. and removed E-easy Easy issue to tackle, requires little or no CockroachDB experience labels Aug 21, 2015
bdarnell added a commit to bdarnell/cockroach that referenced this issue Sep 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-test-failure Broken test (automatically or manually discovered). E-intermediate Intermediate complexity, needs a contributor with 3-6 months of past contribution experience. help wanted Help is requested / needed by the one who filed the issue to fix it.
Projects
None yet
Development

No branches or pull requests

4 participants