Skip to content

Commit

Permalink
Merge pull request #83 from hashicorp/f-lastcontact
Browse files Browse the repository at this point in the history
Set last contact on leader step down
  • Loading branch information
armon committed Jan 22, 2016
2 parents 8237aa6 + 7e38006 commit 057b893
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 5 deletions.
23 changes: 19 additions & 4 deletions raft.go
Original file line number Diff line number Diff line change
Expand Up @@ -755,6 +755,13 @@ func (r *Raft) runLeader() {

// Cleanup state on step down
defer func() {
// Since we were the leader previously, we update our
// last contact time when we step down, so that we are not
// reporting a last contact time from before we were the
// leader. Otherwise, to a client it would seem our data
// is extremely stale.
r.setLastContact()

// Stop replication
for _, p := range r.leaderState.replState {
close(p.stopCh)
Expand Down Expand Up @@ -792,6 +799,11 @@ func (r *Raft) runLeader() {
select {
case notify <- false:
case <-r.shutdownCh:
// On shutdown, make a best effort but do not block
select {
case notify <- false:
default:
}
}
}
}()
Expand Down Expand Up @@ -1387,9 +1399,7 @@ func (r *Raft) appendEntries(rpc RPC, a *AppendEntriesRequest) {

// Everything went well, set success
resp.Success = true
r.lastContactLock.Lock()
r.lastContact = time.Now()
r.lastContactLock.Unlock()
r.setLastContact()
return
}

Expand Down Expand Up @@ -1574,10 +1584,15 @@ func (r *Raft) installSnapshot(rpc RPC, req *InstallSnapshotRequest) {

r.logger.Printf("[INFO] raft: Installed remote snapshot")
resp.Success = true
r.setLastContact()
return
}

// setLastContact is used to set the last contact time to now
func (r *Raft) setLastContact() {
r.lastContactLock.Lock()
r.lastContact = time.Now()
r.lastContactLock.Unlock()
return
}

type voteResult struct {
Expand Down
7 changes: 6 additions & 1 deletion raft_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1366,6 +1366,11 @@ func TestRaft_LeaderLeaseExpire(t *testing.T) {
t.Fatalf("timeout stepping down as leader")
}

// Ensure the last contact of the leader is non-zero
if leader.LastContact().IsZero() {
t.Fatalf("expected non-zero contact time")
}

// Should be no leaders
if len(c.GetInState(Leader)) != 0 {
t.Fatalf("expected step down")
Expand Down Expand Up @@ -1612,7 +1617,7 @@ func TestRaft_NotifyCh(t *testing.T) {
t.Fatalf("should step down as leader")
}
case <-time.After(conf.HeartbeatTimeout * 6):
t.Fatalf("timeout becoming leader")
t.Fatalf("timeout on step down as leader")
}
}

Expand Down

2 comments on commit 057b893

@hgfischer
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ongardie
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the unit tests sometimes pass and sometimes fail, depending on timing.

Please sign in to comment.