-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Drop raft node #4547
Drop raft node #4547
Conversation
What happens if the node being dropped is in communication with the cluster? |
Working with @corylanou here, seeing some funnies on non-dropped nodes after dropping the node. |
@@ -519,7 +535,9 @@ func (s *Store) createLocalNode() error { | |||
} | |||
|
|||
// Set ID locally. | |||
s.mu.Lock() |
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.
Can you add a comment here, outlining why this locking is done?
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.
This was detected as a race by our race detector. Likely only due to test code and would not be a race condition in production, but better safe than sorry.
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.
I meant a comment in the code. :-)
After more system debug, looks like a false alarm. @corylanou doing more system-level testing on systems with a dropped node, even if the node was up and running when dropped. |
65e6805
to
fbe1eaa
Compare
@@ -21,6 +21,7 @@ | |||
- [#4375](https://github.com/influxdb/influxdb/pull/4375): Add Subscriptions so data can be 'forked' out of InfluxDB to another third party. | |||
- [#4506](https://github.com/influxdb/influxdb/pull/4506): Register with Enterprise service and upload stats, if token is available. | |||
- [#4501](https://github.com/influxdb/influxdb/pull/4501): Allow filtering SHOW MEASUREMENTS by regex. | |||
- [#4547](https://github.com/influxdb/influxdb/pull/4547): Drop raft node. |
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.
Just so the guys reading the CHANGELOG know what this about is about can you rephrase it as something like:
DROP SERVER
for all node types (or something).
Some feedback, but this generally all makes sense to me. +1 |
fbe1eaa
to
25a9127
Compare
Currently, if you drop a raft node (even the leader), to bring another raft node to the cluster, you must launch a NEW node. Shutting down an existing node and bringing it back up does not make it a raft by default. This will be fixed in a future PR.