hashicorp / hashicorp/raft

asynchronous heartbeat can cause a stale leader to hold lease indefinitely during disk IO stall

Open
#666 14 comments 0 reactions 1 assignee Claimed by @tgross View on GitHub
bug
Dominant language
Go
Stars
9.1k
Forks
1.1k
Avg merge
3h 27m
Merged PRs (30d)
2

Description

Hi, we found a path where a stale leader can maintain its lease indefinitely through what we call "phantom contacts". We have a reproduction test for this. Posting here to verify our analysis and see if a fix makes sense.

### The inconsistency

In `replicateTo()`, the response term is checked before updating the last contact time:

```go
// replicateTo() — replication.go L238-245
if resp.Term > req.Term {
r.handleStaleTerm(s)
return true
}
s.setLastContact()
```

In `heartbeat()`, this check is missing — `setLastContact()` is called unconditionally when the transport call succeeds:

```go
// heartbeat() — replication.go L423-438
if err != nil {
// ... handle error
} else {
s.setLastContact()
failures = 0
s.notifyAll(resp.Success)
// no resp.Term check
}
```

This means that even when a follower rejects the heartbeat due to a higher term (`resp.Term > req.Term`, `resp.Success == false`), the leader still records a successful contact with that follower.

### What's the problem

Under normal operation, `replicate()` calls `replicateTo()` every `CommitTimeout`, which does check `resp.Term` — so the stale term gets caught quickly. This acts as a safety net.

But `heartbeat()` exists precisely to run **independently** when `replicate()` is blocked:

> ```
> // heartbeat is used to periodically invoke AppendEntries on a peer
> // to ensure they don't time out. This is done async of replicate(),
> // since that routine could potentially be blocked on disk IO.
> ```

When the `replicate()` goroutine blocks on disk IO (e.g. `GetLog()` during `setPreviousLog()`), the safety net is gone. Only `heartbeat()` is sending RPCs, and it keeps calling `setLastContact()` for followers that have already moved on to a higher term. These "phantom contacts" feed into `checkLeaderLease()`, which counts them toward quorum, keeping the stale leader alive.

### Concrete scenario

1. 3-node cluster: Leader L at term T, Followers F1 and F2
2. L's disk IO slows down — `replicate()` goroutines block on `GetLog()`
3. Network partition: L loses contact with F2
4. F1 ends up at term T+N (e.g. participated in a higher-term election during a transient partition), then connectivity between L and F1 is restored
5. L's `heartbeat()` sends `AppendEntries{Term: T}` to F1
6. F1's `appendEntries` handler: `a.Term (T) < currentTerm (T+N)` — rejects, returns `resp.Term = T+N, resp.Success = false`
7. Transport returns `err = nil`
8. L's `heartbeat()` calls `setLastContact()` for F1 — phantom contact
9. L's `checkLeaderLease()`: contacted = {L, F1} = 2 = quorum → **L stays Leader at stale term T**

This continues indefinitely as long as disk IO remains slow.

This requires three conditions to coincide: network partition, node restart, and disk IO stall on the leader. Not an easy combination to hit.

### Reproduction

We wrote a test that reproduces this: https://github.com/specula-org/hashicorp-raft/blob/heartbeat-term-check-bug/heartbeat_term_bug_test.go

### Suggested fix

Add the same `resp.Term` check to `heartbeat()`:

```go
} else {
if resp.Term > req.Term {
r.handleStaleTerm(s)
return
}
s.setLastContact()
failures = 0
s.notifyAll(resp.Success)
}
```

Does this analysis hold up, or is there a mechanism we're not seeing that would prevent this path in production?

Thanks for your time.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.