From 073be6d3edf573833f6756903265cbee65b99e38 Mon Sep 17 00:00:00 2001 From: Jon Currey Date: Sat, 28 Jan 2017 00:30:43 -0800 Subject: [PATCH 1/4] resetNodes() should respect GossipToTheDeadTime --- state.go | 4 ++-- state_test.go | 13 +++++++++++++ util.go | 11 ++++++++--- util_test.go | 29 +++++++++++++++++++++++++---- 4 files changed, 48 insertions(+), 9 deletions(-) diff --git a/state.go b/state.go index cc422bc1a..cce013866 100644 --- a/state.go +++ b/state.go @@ -439,8 +439,8 @@ func (m *Memberlist) resetNodes() { m.nodeLock.Lock() defer m.nodeLock.Unlock() - // Move the dead nodes - deadIdx := moveDeadNodes(m.nodes) + // Move dead nodes, but respect gossip to the dead interval + deadIdx := moveDeadNodes(m.nodes, m.config.GossipToTheDeadTime) // Deregister the dead nodes for i := deadIdx; i < len(m.nodes); i++ { diff --git a/state_test.go b/state_test.go index 57b17246e..06355b588 100644 --- a/state_test.go +++ b/state_test.go @@ -862,6 +862,17 @@ 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 + m.resetNodes() + if len(m.nodes) != 3 { + t.Fatalf("Bad length") + } + if _, ok := m.nodeMap["test2"]; !ok { + t.Fatalf("test2 should not be unmapped") + } + + time.Sleep(5*time.Second) m.resetNodes() if len(m.nodes) != 2 { t.Fatalf("Bad length") @@ -869,6 +880,8 @@ func TestMemberList_ResetNodes(t *testing.T) { if _, ok := m.nodeMap["test2"]; ok { t.Fatalf("test2 should be unmapped") } + + m.config.GossipToTheDeadTime = oldGossipToTheDeadTime } func TestMemberList_NextSeq(t *testing.T) { diff --git a/util.go b/util.go index a294c3034..a141f625f 100644 --- a/util.go +++ b/util.go @@ -190,9 +190,9 @@ func pushPullScale(interval time.Duration, n int) time.Duration { return time.Duration(multiplier) * interval } -// moveDeadNodes moves all the nodes in the dead state -// to the end of the slice and returns the index of the first dead node. -func moveDeadNodes(nodes []*nodeState) int { +// moveDeadNodes moves nodes that are dead and beyond the gossip to the dead interval +// to the end of the slice and returns the index of the first moved node. +func moveDeadNodes(nodes []*nodeState, gossipToTheDeadTime time.Duration) int { numDead := 0 n := len(nodes) for i := 0; i < n-numDead; i++ { @@ -200,6 +200,11 @@ func moveDeadNodes(nodes []*nodeState) int { continue } + // Respect the gossip to the dead interval + if time.Since(nodes[i].StateChange) <= gossipToTheDeadTime { + continue + } + // Move this node to the end nodes[i], nodes[n-numDead-1] = nodes[n-numDead-1], nodes[i] numDead++ diff --git a/util_test.go b/util_test.go index bed608821..f67fb1cd3 100644 --- a/util_test.go +++ b/util_test.go @@ -253,28 +253,49 @@ func TestMoveDeadNodes(t *testing.T) { nodes := []*nodeState{ &nodeState{ State: stateDead, + StateChange: time.Now().Add(-20*time.Second), }, &nodeState{ State: stateAlive, + StateChange: time.Now().Add(-20*time.Second), + }, + // This dead node should not be moved, as its state changed + // less than the specified GossipToTheDead time ago + &nodeState{ + State: stateDead, + StateChange: time.Now().Add(-10*time.Second), }, &nodeState{ State: stateAlive, + StateChange: time.Now().Add(-20*time.Second), }, &nodeState{ State: stateDead, + StateChange: time.Now().Add(-20*time.Second), }, &nodeState{ State: stateAlive, + StateChange: time.Now().Add(-20*time.Second), }, } - idx := moveDeadNodes(nodes) - if idx != 3 { + idx := moveDeadNodes(nodes, (15*time.Second)) + if idx != 4 { t.Fatalf("bad index") } for i := 0; i < idx; i++ { - if nodes[i].State != stateAlive { - t.Fatalf("Bad state %d", i) + fmt.Println("index %d, state %d", i, nodes[i].State) + switch i { + case 2: + // Recently dead node remains at index 2, + // since nodes are swapped out to move to end. + if nodes[i].State != stateDead { + t.Fatalf("Bad state %d", i) + } + default: + if nodes[i].State != stateAlive { + t.Fatalf("Bad state %d", i) + } } } for i := idx; i < len(nodes); i++ { From dc6e637c96144c74fd6117928317593797bd11bc Mon Sep 17 00:00:00 2001 From: Jon Currey Date: Sat, 28 Jan 2017 00:33:19 -0800 Subject: [PATCH 2/4] formatting --- state_test.go | 4 ++-- util_test.go | 28 ++++++++++++++-------------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/state_test.go b/state_test.go index 06355b588..5e5517ea4 100644 --- a/state_test.go +++ b/state_test.go @@ -863,7 +863,7 @@ func TestMemberList_ResetNodes(t *testing.T) { m.deadNode(&d) oldGossipToTheDeadTime := m.config.GossipToTheDeadTime - m.config.GossipToTheDeadTime = 3*time.Second + m.config.GossipToTheDeadTime = 3 * time.Second m.resetNodes() if len(m.nodes) != 3 { t.Fatalf("Bad length") @@ -872,7 +872,7 @@ func TestMemberList_ResetNodes(t *testing.T) { t.Fatalf("test2 should not be unmapped") } - time.Sleep(5*time.Second) + time.Sleep(5 * time.Second) m.resetNodes() if len(m.nodes) != 2 { t.Fatalf("Bad length") diff --git a/util_test.go b/util_test.go index f67fb1cd3..4ea7ba893 100644 --- a/util_test.go +++ b/util_test.go @@ -252,34 +252,34 @@ func TestPushPullScale(t *testing.T) { func TestMoveDeadNodes(t *testing.T) { nodes := []*nodeState{ &nodeState{ - State: stateDead, - StateChange: time.Now().Add(-20*time.Second), + State: stateDead, + StateChange: time.Now().Add(-20 * time.Second), }, &nodeState{ - State: stateAlive, - StateChange: time.Now().Add(-20*time.Second), + State: stateAlive, + StateChange: time.Now().Add(-20 * time.Second), }, // This dead node should not be moved, as its state changed // less than the specified GossipToTheDead time ago &nodeState{ - State: stateDead, - StateChange: time.Now().Add(-10*time.Second), + State: stateDead, + StateChange: time.Now().Add(-10 * time.Second), }, &nodeState{ - State: stateAlive, - StateChange: time.Now().Add(-20*time.Second), + State: stateAlive, + StateChange: time.Now().Add(-20 * time.Second), }, &nodeState{ - State: stateDead, - StateChange: time.Now().Add(-20*time.Second), + State: stateDead, + StateChange: time.Now().Add(-20 * time.Second), }, &nodeState{ - State: stateAlive, - StateChange: time.Now().Add(-20*time.Second), + State: stateAlive, + StateChange: time.Now().Add(-20 * time.Second), }, } - idx := moveDeadNodes(nodes, (15*time.Second)) + idx := moveDeadNodes(nodes, (15 * time.Second)) if idx != 4 { t.Fatalf("bad index") } @@ -287,7 +287,7 @@ func TestMoveDeadNodes(t *testing.T) { fmt.Println("index %d, state %d", i, nodes[i].State) switch i { case 2: - // Recently dead node remains at index 2, + // Recently dead node remains at index 2, // since nodes are swapped out to move to end. if nodes[i].State != stateDead { t.Fatalf("Bad state %d", i) From 38c240a9d1c2c95c8a28b1acc87a98a6625c0554 Mon Sep 17 00:00:00 2001 From: Jon Currey Date: Tue, 7 Feb 2017 20:49:00 -0800 Subject: [PATCH 3/4] lower test timeout; don't cache value unnecessarily --- state_test.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/state_test.go b/state_test.go index 5e5517ea4..7eb4be70a 100644 --- a/state_test.go +++ b/state_test.go @@ -862,8 +862,7 @@ 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 + m.config.GossipToTheDeadTime = 200 * time.Millisecond m.resetNodes() if len(m.nodes) != 3 { t.Fatalf("Bad length") @@ -880,8 +879,6 @@ func TestMemberList_ResetNodes(t *testing.T) { if _, ok := m.nodeMap["test2"]; ok { t.Fatalf("test2 should be unmapped") } - - m.config.GossipToTheDeadTime = oldGossipToTheDeadTime } func TestMemberList_NextSeq(t *testing.T) { From 44d5382e50a7965b6c43583eecc3323105ab753d Mon Sep 17 00:00:00 2001 From: James Phillips Date: Wed, 8 Feb 2017 13:14:57 -0800 Subject: [PATCH 4/4] Tweaks timing to make the test run quicker. --- state_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/state_test.go b/state_test.go index 7eb4be70a..093820a7d 100644 --- a/state_test.go +++ b/state_test.go @@ -862,7 +862,7 @@ func TestMemberList_ResetNodes(t *testing.T) { d := dead{Node: "test2", Incarnation: 1} m.deadNode(&d) - m.config.GossipToTheDeadTime = 200 * time.Millisecond + m.config.GossipToTheDeadTime = 100 * time.Millisecond m.resetNodes() if len(m.nodes) != 3 { t.Fatalf("Bad length") @@ -871,7 +871,7 @@ func TestMemberList_ResetNodes(t *testing.T) { t.Fatalf("test2 should not be unmapped") } - time.Sleep(5 * time.Second) + time.Sleep(200 * time.Millisecond) m.resetNodes() if len(m.nodes) != 2 { t.Fatalf("Bad length")