Alex E

Posted on Sep 30

# The failover test that stopped the wrong node

#dotnet #distributedsystems #csharp #testing

Two weeks ago I shipped Squirix 0.1.0-preview.8. The release notes said RF>=3 clusters survive the loss of a node: the majority elects a new leader, stale leaders get
fenced, clients get rerouted. Then I published the first article of a series about how it works.

On September 23 I found out that the failover part doesn't run in the shipped server. The code exists. It has unit tests, an executable model, E2E tests, and green CI. Nothing in the production host calls it.

I've added an update to part 1. This post is the longer version: how a feature can be written, tested, and documented without ever being switched on, and what I'm changing so it doesn't happen again. The article about elections waits until elections actually run.

An analyzer I had told to be quiet

It started with warnings I had silenced myself.

Squirix builds with Meziantou.Analyzer, and its rule MA0182 flags internal types that nothing references. In the replication code there were seven suppressions of it, all with nearly the same justification:

[SuppressMessage(
    "Usage",
    "MA0182:Internal type is apparently never used",
    Justification = "Test-only activation seam until failover activation wires the gate in a follow-up milestone.")]
internal static class FailoverActivationGate

The seven types were the election timer, the failover activation gate, the leader read barrier, the snapshot catch-up session, the reroute budget, the stale-term classifier, and the election commit rule. That is most of failover.

"Follow-up milestone" sounds like a plan, so I went looking for it. There wasn't one. The milestone folder had preview.8 itself and a backlog with one unrelated item. git log -S showed the suppressions were added inside the preview.8 pull requests, around September 6 and 7. The follow-up only ever existed in the justification string.

The analyzer was right the whole time. It said "nobody uses this", and nobody did.

What was actually missing

Once I stopped trusting the closed issues and read the host wiring instead, the list grew quickly:

AutomaticFailoverEnabled and QuorumReadsEnabled existed on the internal topology options. Nothing in src/ read them. Only tests set them to true.
The replication gRPC contract had no PreVote or RequestVote RPC. Nodes had no way to ask each other for a vote over the network. The vote handling in the follower log was called only from tests.
The repair service was registered and running, but nothing ever queued work for it.
The request path had no reroute or stale-term handling, although the release notes promised "bounded client rerouting on stale terms".

Every issue in the failover part of the milestone was closed. The exit gate checklist at the bottom of the milestone plan, the one that says to enable failover in production after the proof matrix passes, had no boxes ticked. I didn't read it before tagging.

Some of it did ship and run: the replication opt-in with its persistence and mTLS checks, the replica ring and topology fingerprint, the replication contract, durable follower logs, and the majority-commit write path. But lose the owner of a key and the key is unavailable until the owner comes back, same as in preview.7.

The test that stopped the wrong node

This part bothers me most, because it existed to catch exactly this.

The milestone had a hard requirement: stop the leader of an RF=3 group, and the remaining majority serves reads and writes within five seconds. There was a test for it,
and it was green. Here is its shape at the preview.8 tag, trimmed a little:

var key = KeyOwnerHelper.ThreeNode.FindKeyOwnedBy("default", "nodeB", "failover-recover");

await using var client = await LoopbackConnect.ConnectAsync(uriB, uriC, ct);
var cache = await client.GetCacheAsync<string>("default", ct);
await cache.SetAsync(key, "before-loss", cancellationToken: ct);

await cluster.StopNodeAsync("nodeA");

await cache.SetAsync(key, "after-loss", cancellationToken: linked.Token);
Assert.Equal("after-loss", (await cache.GetValueAsync(key, linked.Token)).Value);

The key is owned by nodeB. The test stops nodeA. With three nodes and RF=3, nodeA is just a follower for this key. The owner stays up, nodeB and nodeC still make a majority, the write commits, and the read returns it. No election is needed, none happens, and the test passes well inside five seconds.

The release test suite had a second test with the same bug. Two separate proofs of failover, and both killed a follower.

I can see how it happened. FindKeyOwnedBy("nodeB") and StopNodeAsync("nodeA") sit a few lines apart, each looks reasonable on its own, and the assertion at the end checks the outcome you care about. What the test never checks is that the node it destroyed was the one that mattered. A failure test that doesn't assert what it killed can pass for the wrong reason indefinitely.

The model was fine

The strange part is that the hard thinking had been done. Before writing election code I froze the protocol in an ADR with an executable C# model: a BFS explorer over elections, commits, and quorum reads for RF=2 to RF=5, with crashes around every durable write and deliberately broken variants that must produce counterexamples.

That model says the protocol is safe. It says nothing about whether the server runs the protocol. I had solid evidence about one thing and was reading it as evidence about another.

What I'm changing

The fixes are small, which is probably why they were missing.

Failure tests assert what they kill. The leader-stop tests now pick a key owned by the node they stop. Until failover is wired, they are skipped with an explicit reason: "Automatic failover is not yet wired into production". A skipped test with a reason tells the truth. A green test that exercises nothing doesn't.
"Later" needs an address. A suppression or TODO that defers work to a follow-up has to name a tracked issue. If it can't, the work isn't deferred, it's dropped.
Release notes come from the code, not from the tracker. Each claim should point at a test or a code path in the shipped host.
Closed issues don't make a milestone done. The exit gate gets read, and ticked, before the tag.

Where failover stands

Wiring it up is now tracked as its own piece of work, split into eleven units: storage support for elections, the vote RPCs, the election driver, the follower apply loop, repair, routing and authority checks on the request path, the public switches, and a real E2E proof matrix in which the leader actually dies.

The audit also turned up two problems that had to be fixed first. A leader that restarted with an uncommitted log tail could never commit again. And on a slow disk, a client timeout could cancel a commit after its decision point. Both are fixed on the development branch now.

When the leader-stop tests are un-skipped and pass for real, I'll pick the series back up with elections, fencing, and why RF=2 never promotes. That article is already written. It just has to become true first.

Top comments (1)

Ivan Rossouw • Oct 1

This shows why a failure test needs evidence of the mechanism as well as the final outcome. I’d capture the owner and term before injection, assert that the stopped process was the authoritative owner, then assert a term change, a new leader, and fencing of the old leader after restart. A negative control is useful too: run the same scenario with failover disabled and require it to miss the availability target. That makes the test prove that failover caused recovery rather than merely observing that an unaffected path stayed healthy.

DEV Community