The jitter that never jittered: how a passing test hid a real bug

Our retry backoff had jitter. It was configured, it was switched on, and it ran on every message we published and every cached read in the bidder. It also returned exactly the same number, every single time it was called. The test we had written for it passed.

This is what we found, how we fixed it, and the thing we actually changed afterwards — which was not the code.

First, what jitter is for

When a service you depend on stops answering, you retry. If you retry immediately and forever, you make things worse, so you wait a bit longer after each attempt. That is backoff.

But backoff alone has a trap. If a hundred machines all fail at the same moment and all wait the same two seconds, they all come back at the same moment too. The dependency gets hit by one wall of traffic, falls over again, and the wall re-forms. That is a thundering herd, and it is the pattern in the banner above.

Jitter is the fix: give every waiter a slightly different wait. Same number of retries, spread over time. It only works if the waits are actually different from each other.

One line, two bugs

Stripped to its essentials, ours looked like this:

// Add jitter (±25%)
jitter := delay * 0.25 * (2*float64(time.Now().UnixNano()%2) - 1)
delay += jitter

You can read the intent straight off it, which is exactly why it passed review. It looks like an ordinary way to nudge a delay up or down by a quarter. It is wrong in two separate ways, and the second one is the interesting one.

Here is the same function before and after the fix, measured the same way:

Before 1 distinct value

100k0

0 ms50 ms100 ms

After 99,951 distinct

100k0

0 ms50 ms100 ms

The test is the real story

A bug that returns one constant value on every call should be easy to catch. We had a test for this exact function. Here it is:

delays := make(map[time.Duration]bool)
for i := 0; i < 10; i++ {
    delays[calculateDelay(cfg, 0)] = true
}

// Should have some variation (may not always be true due to randomness)
// Just check it doesn't panic and returns reasonable values
for delay := range delays {
    if delay < 0 {
        t.Errorf("delay should not be negative, got %v", delay)
    }
}

Read what it does. It collects ten delays into a set — the exact structure that would expose the bug — and then never checks how big that set is. It only checks that the numbers are not negative. We ran it against the broken code to be certain. It passes, comfortably.

The comment is the confession. "may not always be true due to randomness" is someone noticing that the obvious check would be flaky, and settling that by deleting the check instead of making it reliable. That trade gets made all the time, usually under deadline, and it is almost always the wrong one.

Picking the fix is an architecture decision

There is a standard set of choices here, best known from Marc Brooker's writing on backoff at AWS. Full jitter picks a wait anywhere between zero and the delay. Equal jitter keeps half the delay as a floor and randomises the other half. Decorrelated jitter works out each wait from the previous one. They are not interchangeable, and the right one depends on what sits underneath the retry.

Full jitter spreads the widest, which is what you want. The cost is that a retry can fire almost straight away, so backoff no longer guarantees any minimum gap. Whether you can afford that is not really a question about jitter — it is a question about what else is protecting the dependency.

We chose full jitter because every place we call it already had that protection: the database and cache paths sit behind a circuit breaker, and the message-broker path has a per-attempt write timeout inside a bounded overall budget. Something else was already responsible for refusing to hammer a dead service. Without those, equal jitter's floor would have been the right trade instead.

delay = rand.Float64() * delay  // full jitter: anywhere in [0, delay)

The replacement scales the delay down instead of offsetting it, which quietly fixes a third thing. The old ±25% could push the wait a quarter past the ceiling we had configured. Multiplying by a number below 1 makes the maximum a real maximum. It also can't produce a negative number, which let us delete a defensive if delay < 0 branch that had been covering for the symptom.

Proving that a random fix actually works

Testing randomness has a specific failure mode: it is easy to write a test that passes everything, which is how we got here in the first place. So the new test checks three things — every value falls inside [0, base), at least half of 1,000 draws are different from each other, and the average sits within 5% of where it should.

Then we checked that those checks can actually fail, by running them against four deliberately broken versions:

The highlighted row is the one that earns its keep. It has a perfectly good random number and still fails — which means the test is checking the shape of the spread, not the health of the random number generator. That is what makes the result portable: we could not reproduce the clock behaviour on our build machines, and thanks to that fake broken version we did not need to.

On flakiness, the thing that started all this: the average check sits about five and a half standard deviations away from its limit at a thousand samples. That is tight enough to catch a collapsed spread and loose enough that it should effectively never fire by accident. We ran it 500 times to confirm. Tests on random things do not have to be flaky. They have to be sized.

Where this hides in an ad platform

Real-time bidding is an unusually good home for this kind of bug. Many identical stateless pods, a few shared dependencies, a hard auction deadline measured in milliseconds, and traffic that genuinely arrives in lockstep at the top of every hour. Deliberate randomness does real work in more places than most systems — and in every one of them, a collapsed spread fails silently. The code runs. The numbers look plausible. The tests stay green.

The rule underneath all of them is short enough to keep in your head: anywhere randomness is used to spread things out, a collapsed spread turns it into a synchroniser. The mechanism doesn't weaken. It inverts.

What we took away

• Treat randomness as infrastructure. A source of randomness deserves the same care as a clock or an ID generator. Building one out of a timestamp is a smell worth recognising — timestamps only go up, and they move in steps, which are the two properties you least want.
• Check the spread, not the absence of a crash. "It returned a reasonable value" is satisfied by a constant. Counting distinct values, checking bounds, and a properly sized average are cheap and they actually tell you something.
• Break your own test on purpose. If you can't show your new test failing against a plausible wrong version, you haven't yet shown that it tests anything.
• Never widen a tolerance to fix a flaky test. Make it deterministic, or size the statistics properly. A loosened limit is a bug admitted on purpose — and the comment explaining why will still be there when it matters.
• Choose the jitter scheme by what's underneath it. Full jitter is the right default when a circuit breaker or timeout already caps the damage. Without one, buy the floor.

If you want the wider picture of how the bidder is put together, that's in our architecture post. And if this is the kind of engineering you like doing, we occasionally have roles open in Dhaka.