asynchronous heartbeat can cause a stale leader to hold lease indefinitely during disk IO stall
- 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
Assessment
This issue has not been assessed yet.