resetNodes() should respect gossip to dead #106
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
slackpad
reviewed
Feb 8, 2017
state_test.go
Outdated
| @@ -862,13 +862,26 @@ func TestMemberList_ResetNodes(t *testing.T) { | |||
| d := dead{Node: "test2", Incarnation: 1} | |||
| m.deadNode(&d) | |||
|
|
|||
| oldGossipToTheDeadTime := m.config.GossipToTheDeadTime | |||
There was a problem hiding this comment.
No need to save and restore this - the scope here is just this one test.
state_test.go
Outdated
| @@ -862,13 +862,26 @@ func TestMemberList_ResetNodes(t *testing.T) { | |||
| d := dead{Node: "test2", Incarnation: 1} | |||
| m.deadNode(&d) | |||
|
|
|||
| oldGossipToTheDeadTime := m.config.GossipToTheDeadTime | |||
| m.config.GossipToTheDeadTime = 3 * time.Second | |||
There was a problem hiding this comment.
Let's set this to something like 200 * time.Millisecond so the test doesn't take too long.
|
Makes sense. Done. |
slackpad
reviewed
Feb 8, 2017
state_test.go
Outdated
| t.Fatalf("test2 should not be unmapped") | ||
| } | ||
|
|
||
| time.Sleep(5 * time.Second) |
There was a problem hiding this comment.
We can knock this one down, too.
Dieterbe
added a commit
to grafana/metrictank
that referenced
this pull request
Nov 26, 2017
pre v0.1.0 interesting changes: * a bugfix for GossipToTheDeadTime (hashicorp/memberlist#106) post v0.1.0 interesting changes: * Reduce LAN min suspicion multiplier (hashicorp/memberlist#129)
Dieterbe
added a commit
to grafana/metrictank
that referenced
this pull request
Nov 26, 2017
pre v0.1.0 interesting changes: * a bugfix for GossipToTheDeadTime (hashicorp/memberlist#106) post v0.1.0 interesting changes: * Reduce LAN min suspicion multiplier (hashicorp/memberlist#129)
Dieterbe
added a commit
to grafana/metrictank
that referenced
this pull request
Dec 1, 2017
pre v0.1.0 interesting changes: * a bugfix for GossipToTheDeadTime (hashicorp/memberlist#106) post v0.1.0 interesting changes: * Reduce LAN min suspicion multiplier (hashicorp/memberlist#129)
Aergonus
pushed a commit
to bloomberg/metrictank
that referenced
this pull request
Jan 19, 2018
pre v0.1.0 interesting changes: * a bugfix for GossipToTheDeadTime (hashicorp/memberlist#106) post v0.1.0 interesting changes: * Reduce LAN min suspicion multiplier (hashicorp/memberlist#129)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#99 introduced
Config.GossipToTheDeadTime.resetNodes()is called when a node gets to the end of its randomized list of nodes to gossip with. It should not remove dead nodes if they entered the dead state less thanConfig.GossipToTheDeadTimeago.