Jitter kubernetes dial backoff so builders do not retry in lockstep - #3995
Jitter kubernetes dial backoff so builders do not retry in lockstep#39951991santhu wants to merge 2 commits into
Conversation
calculateBackoff was a pure function of the attempt number, so every builder retrying the same condition waited for exactly the same durations. That matters for the condition this backoff exists to handle. Per the comment on tryWithBackoff, it covers the race where Kubernetes marks nodes Ready before their CSRs are approved. Approval lag is a cluster-wide event rather than a per-pod one, so concurrent builds scheduled onto newly-ready nodes hit the transient TLS error at the same moment and then retry at identical instants, concentrating load on the API server while it is still working through the approval backlog. Apply equal jitter: retain half the interval as a floor and randomise the remainder. A retry is never issued immediately, and the result never exceeds maxDelay, so no caller waits longer than before. Adds a test file for the package covering the bounds, the maxDelay cap, and that successive calls vary. Signed-off-by: Santhosh Kumar Somarapu <somarapu.santhosh91@gmail.com>
Equal jitter drew from [d/2, d], so the first retry could fire 250ms after a transient TLS error even though baseDelay is configured as 500ms. Retrying sooner than the configured minimum puts load on the API server during the very window it is working through the CSR approval backlog. Add the jitter instead, drawing from [d, 2d] and bounding the extra by the remaining headroom so the result never exceeds maxDelay. With maxRetries=5 and baseDelay=500ms the delays used are 500ms through 4s, so the cap is never reached in practice and the spread is preserved throughout. Signed-off-by: Santhosh Kumar Somarapu <somarapu.santhosh91@gmail.com>
|
Corrected the direction of the jitter in my own patch. It was equal jitter, I ran the numbers rather than just asserting the problem. 100 builders hitting the same CSR lag: without jitter all 100 dial inside the same 100ms window on every attempt, with it about 28 on the first retry and 7 by the fourth. Simulated arrival times, not something I've measured on a real cluster.
|
The problem
calculateBackoffis a pure function of the attempt number:so every builder retrying the same condition waits for exactly 500ms, 1s, 2s, 4s, 8s.
That matters specifically because of the condition this retry exists to handle. From the comment on
tryWithBackoff:CSR approval lag is a cluster-wide, time-correlated event rather than a per-pod accident. When a node group comes up, every build scheduled onto those nodes hits the transient TLS error at roughly the same moment — and then retries at identical instants. In CI, where parallel builds are the normal case, that concentrates retries on the API server precisely while it is still working through the approval backlog.
The backoff limits how often each builder retries. It does not stop builders retrying together.
The change
Equal jitter — retain half the computed interval as a floor, randomise the remainder:
Half is kept rather than using full jitter so a retry is never issued immediately after a TLS failure. The result never exceeds
maxDelay, so no builder waits longer than it does today — the change only removes the alignment.Testing
go test -count=2 ./driver/kubernetes/...passes,go vetclean.The package had no test file, so this adds one covering: the result stays within
[d/2, d]for attempts 0–5 across 500 samples each; a large attempt count stays capped atmaxDelay; and successive calls vary, which fails if the implementation regresses to deterministic.Note
math/randrather thancrypto/rand— this is load spreading, not a security boundary. Happy to change if the project prefers otherwise. Context for the original retry: #2668.