autopilot: cluster can fail to recover from outages when min_quorum isn't set.
- Dominant language
- Go
- Stars
- 30.1k
- Forks
- 4.6k
- Avg merge
- 1d 18h
- Merged PRs (30d)
- 39
Description
I co-maintain the `consul` package in the NixOS Linux distribution. We have an automated VM-based test [here](https://github.com/NixOS/nixpkgs/blob/823d171956312ba2bdba0359f0448cd6d838cb75/nixos/tests/consul.nix#L101-L141) to check that consul works.
The test starts 3 consul servers, and reboots each of them in a rolling fashion, checking that consensus is re-established when they come back up:
```python
# Test that the cluster can tolearate failures of any single server:
for server in [server1, server2, server3]:
server.crash()
# wait for consul KV operations to work again
server.start()
# ...
```
**This test started failing between consul `1.7.0-beta1` and `1.7.0-beta2`.**
`server1` reboots and Consul works fine then. But as soon as the second server (`server2`) goes down, Consul returns `No cluster leader`, and never recovers from it, even though the other 2 servers are up. It fails reproducibly.
## Analysis
As far as I can tell, the reason is that after its reboot, `server1` is not accepted back as a voter. Then, when `server2` goes down, only 2 servers are left, of which one is not a voter, so they cannot elect a new leader.
With the failing `1.7.0-beta2` and newer, I observe that just after the reboot of `server1`, its `Status` is `left`:
```
# consul members
Node Address Status Type Build Protocol DC Segment
server1 192.168.1.1:8301 left server 1.7.0-beta1 2 dc1
server2 192.168.1.2:8301 alive server 1.7.0-beta1 2 dc1
server3 192.168.1.3:8301 alive server 1.7.0-beta1 2 dc1
```
This was at first surprising to me, because I expected it to be `failed`, given that `.crash()` from the test hard-crashes the VM. Further, the default options for servers (as explained on the [options](https://www.consul.io/docs/agent/options.html) docs) are:
* [`leave_on_terminate`](https://www.consul.io/docs/agent/options.html#leave_on_terminate) - defaults to `false`
* [`skip_leave_on_interrupt`](https://www.consul.io/docs/agent/options.html#skip_leave_on_interrupt) - defaults to `true`
so even if the server shut down cleanly instead of hard-crashing, I'd expect the server to go `failed`, not `left`.
```
# consul operator raft list-peers
Node ID Address State Voter RaftProtocol
server3 420a2da0-4547-d06c-d4a5-8afb1a2fbe8d 192.168.1.3:8300 leader true 3
server2 1cb012d8-4dca-783e-5292-9febf80c08de 192.168.1.2:8300 follower true 3
server1 a071fbe3-b707-139c-0353-f276d248f104 192.168.1.1:8300 follower false 3
```
**This shows that `server1` is `Voter` `false`, and never changes back from it.**
(As soon as `server2` goes down, `consul operator raft list-peers` stops working and also returns `No cluster leader`, and you have to use `consul operator raft list-peers -stale=true` to see the output.)
### Observation: Rolling-rebooting "more slowly" fixes it
When performing the reboots by hand, instead of with the automated test script, I noticed that that works fine:
The `Status` `left` eventually (after around half a minute) turns back into `alive` in `consul members`, and the `Voter` `false` eventually tursn into `true` in `consul operator raft list-peers`.
But of course that is not a solution, because in my production environment, servers may crash and reboot unexpectedly, without "waiting" for other servers to crash. I expect Consul to tolerate arbitrary such crashes and come back without manual intervention, as it did before `1.7.0-beta2`.
### Git bisect
I read the [changelog of v1.7.0-beta2](https://github.com/hashicorp/consul/blob/v1.7.0-beta2/CHANGELOG.md) and the [compare link for `v1.7.0-beta1...v1.7.0-beta2`](https://github.com/hashicorp/consul/compare/v1.7.0-beta1...v1.7.0-beta2).
From that, I identified the listed PR https://github.com/hashicorp/consul/pull/4017 as the only possibly relevant change.
A `git bisect` between the two versions confirmed my guess; the commit that introduced the problem is https://github.com/hashicorp/consul/commit/c47dbffe1c5464d8aa9794e58687d894f1e50df9, part of above PR, with description:
```
autopilot: fix dead server removal condition to use correct failure tolerance (#4017)
```
Click to expand full git bisect log
```
$ git bisect log
git bisect start
# bad: [e01f6913516b4b852da54dad42e8a194c756cfd5] Release v1.7.0-beta2
git bisect bad e01f6913516b4b852da54dad42e8a194c756cfd5
# good: [dcab9358318ce8ea64939d323f8d291fb39178af] Release v1.7.0-beta1
git bisect good dcab9358318ce8ea64939d323f8d291fb39178af
# bad: [77e789a4f4fdb1fc4e6268a074bb7360f10f3605] ui: Add optional name="" attribute so you can name slots with it (#6740)
git bisect bad 77e789a4f4fdb1fc4e6268a074bb7360f10f3605
# good: [1080be2087786fed04f3c4afca0d25539b27a189] Merge pull request #6891 from hashicorp/helm-docs-dec5
git bisect good 1080be2087786fed04f3c4afca0d25539b27a189
# bad: [83c84d4e7e3ca06b2143aa7939b31bcde785cd66] coverage: disable comment and project status, set informational mode (#6954)
git bisect bad 83c84d4e7e3ca06b2143aa7939b31bcde785cd66
# good: [f03153f571feb7cccdbc100fb8438778e449abe3] Merge pull request #6805 from hashicorp/issue-6804-sysctl-path
git bisect good f03153f571feb7cccdbc100fb8438778e449abe3
# good: [4f5d5020b8c37bd8c802164f410d728a16329f4b] dns: fix memoryleak by upgrading outdated miekg/dns (#6748)
git bisect good 4f5d5020b8c37bd8c802164f410d728a16329f4b
# bad: [c47dbffe1c5464d8aa9794e58687d894f1e50df9] autopilot: fix dead server removal condition to use correct failure tolerance (#4017)
git bisect bad c47dbffe1c5464d8aa9794e58687d894f1e50df9
# first bad commit: [c47dbffe1c5464d8aa9794e58687d894f1e50df9] autopilot: fix dead server removal condition to use correct failure tolerance (#4017)
```
### Suspicion: Autopilot
The [Autopilot article](https://learn.hashicorp.com/consul/day-2-operations/autopilot#server-stabilization-time) explains:
> **Server stabilization time**
> When a new server is added to the datacenter, there is a waiting period where it must be healthy and stable for a certain amount of time before being promoted to a full, voting member. This is defined by the `ServerStabilizationTime` autopilot's parameter and by default is 10 seconds.
>
> **Dead server cleanup**
> Autopilot helps prevent these kinds of outages by quickly removing failed servers as soon as a replacement Consul server comes online. When servers are removed by the cleanup process they will enter the "left" state.
"Dead server cleanup" explains why I see `left` instead of `failed` in `consul members`.
"Server stabilization time" seems to explain the main problem, that the rebooted server is `Voter` `false` for a while, and that the script reboots `server2` within less than 10 seconds of `server1` being back, thus not allowing `server1` to be promoted back to `Voter` before that.
## How should this work?
While the autopilot docs _explain_ what happens, I still consider it bugged:
* **Consul <= `1.7.0-beta1` was a HA system** that tolerated arbitrary reboots and would eventually recover back to consensus. **For newer versions, that is no longer the case.**
* The autopilot's logic in "Server stabilization time" makes sense when you replace one (decommissioned) server by another, but it does not seem to make sense to me to apply that when a machine just comes back from reboot, and no replacement server tried to take its place.
* It seems that a it should not require a quorate vote to allow a just-rebooted server to vote again, otherwise any temporary break of quorum will turn into a permanent break of quorum. That also includes just rebooting all servers in paralle.
* Side issues:
* The docs of `leave_on_terminate` and `skip_leave_on_interrupt` should really mention that autopilot's auto-leaving-by-others (which is enable by default) exists, and that thus whatever you configure for these does not last for more than 200 ms (the default value for `LastContactThreshold`).
What can be done about this?
Contributor guide
Research direction
Reproduce the failure with the NixOS test in nixos/tests/consul.nix, focusing on the rolling reboot sequence and the `min_quorum`-unset configuration. Read the Autopilot stabilization behavior and commit c47dbffe1c5464d8aa9794e58687d894f1e50df9, then verify that temporary server outages allow the cluster to regain quorum and promote recovered servers without manual intervention.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- distributed-systems
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100