From 795eb11cb0e416501ce2c04e87f8430b280a26ed Mon Sep 17 00:00:00 2001 From: Jonathan Eid Date: Mon, 17 Aug 2026 15:10:34 -0400 Subject: [PATCH 1/4] Retry apiserver 429s instead of failing the mission A single 429 reaching a caller that does not retry ends a whole mission. That is what killed Jenkins stellar-supercluster #1913: one 429 on a `list events` inside WaitForAllReplicasReady, with no retry on that path. Throttling is routine rather than exceptional on a busy cluster -- ssc-eks served 36,784 429s in the week of 2026-08-17, peaking at 63/s -- so every one of the ~46 apiserver calls in this library is exposed, and nothing makes the one that happened to fail special. Retrying in the HttpClient pipeline covers all of them from one place, and sitting below the generated client means a retried 429 never becomes an exception at all. Only 429 is retried: it proves the request was rejected unapplied, so re-issuing is safe even for a write, whereas a 5xx or a timeout leaves that unknown. DELETE is exempt because every delete site already swallows failure, so retrying there buys fewer orphans at the price of multiplying a teardown that removes hundreds of objects in sequence. The 60s budget stays under HttpClientTimeout (100s), which bounds the whole handler chain and otherwise surfaces as a TaskCanceledException that loses the 429. Verified against injected 429s and against real APF throttling on ssc-test: 6 real rejections absorbed mid-mission, unbounded rejection still fails cleanly at the deadline, and 12 rejected DELETEs are not retried. Co-Authored-By: Claude Opus 5 --- src/FSLibrary.Tests/Tests.fs | 58 ++++++++++++++++++++++++++++ src/FSLibrary/ApiRateLimit.fs | 54 ++++++++++++++++++++++++++ src/FSLibrary/StellarSupercluster.fs | 7 +++- 3 files changed, 118 insertions(+), 1 deletion(-) diff --git a/src/FSLibrary.Tests/Tests.fs b/src/FSLibrary.Tests/Tests.fs index de20cf6f..620537c2 100644 --- a/src/FSLibrary.Tests/Tests.fs +++ b/src/FSLibrary.Tests/Tests.fs @@ -161,6 +161,7 @@ let ctx : MissionContext = let netdata = __SOURCE_DIRECTORY__ + "/../../../data/public-network-data-2026-06-03-trimmed-located.json" + let pubkeys = __SOURCE_DIRECTORY__ + "/../../../data/tier1keys.json" let pubnetctx = { ctx with pubnetData = Some netdata; tier1Keys = Some pubkeys } @@ -704,3 +705,60 @@ type Tests(output: ITestOutputHelper) = [] member __.``QuorumIntersectionChecker mission is registered``() = Assert.True(StellarMission.allMissions.ContainsKey "QuorumIntersectionChecker") + +// A stand-in for the apiserver: rejects the first `failures` requests with 429, +// then succeeds, and counts how many times it was actually called. +type private ThrottlingStub(failures: int) = + inherit System.Net.Http.HttpMessageHandler() + let mutable calls = 0 + member __.Calls = calls + + override __.SendAsync(_req, _ct) = + calls <- calls + 1 + + let code = + if calls <= failures then + System.Net.HttpStatusCode.TooManyRequests + else + System.Net.HttpStatusCode.OK + + System.Threading.Tasks.Task.FromResult(new System.Net.Http.HttpResponseMessage(code)) + +let private sendThrough + (handler: ApiRateLimit.ThrottleRetryHandler) + (stub: ThrottlingStub) + (verb: System.Net.Http.HttpMethod) + = + handler.InnerHandler <- stub + use invoker = new System.Net.Http.HttpMessageInvoker(handler) + + let req = + new System.Net.Http.HttpRequestMessage(verb, "http://apiserver.invalid/api/v1/nodes") + + invoker.SendAsync(req, System.Threading.CancellationToken.None).Result + +[] +let ``Throttle retry rides out 429s and returns the eventual success`` () = + let stub = ThrottlingStub(3) + let handler = new ApiRateLimit.ThrottleRetryHandler(System.TimeSpan.FromSeconds 30.0) + let resp = sendThrough handler stub System.Net.Http.HttpMethod.Get + Assert.Equal(System.Net.HttpStatusCode.OK, resp.StatusCode) + // Three rejections plus the attempt that succeeded. + Assert.Equal(4, stub.Calls) + +[] +let ``Throttle retry leaves DELETE alone so teardown stays bounded`` () = + let stub = ThrottlingStub(5) + let handler = new ApiRateLimit.ThrottleRetryHandler(System.TimeSpan.FromSeconds 30.0) + let resp = sendThrough handler stub System.Net.Http.HttpMethod.Delete + Assert.Equal(System.Net.HttpStatusCode.TooManyRequests, resp.StatusCode) + Assert.Equal(1, stub.Calls) + +[] +let ``Throttle retry gives up at the deadline and surfaces the 429`` () = + let stub = ThrottlingStub(1000) + let handler = new ApiRateLimit.ThrottleRetryHandler(System.TimeSpan.Zero) + let resp = sendThrough handler stub System.Net.Http.HttpMethod.Get + // The 429 must reach the caller rather than being swallowed or masked. + Assert.Equal(System.Net.HttpStatusCode.TooManyRequests, resp.StatusCode) + Assert.Equal(1, stub.Calls) diff --git a/src/FSLibrary/ApiRateLimit.fs b/src/FSLibrary/ApiRateLimit.fs index f210db61..473aa80d 100644 --- a/src/FSLibrary/ApiRateLimit.fs +++ b/src/FSLibrary/ApiRateLimit.fs @@ -5,6 +5,10 @@ module ApiRateLimit open Logging +open System.Net +open System.Net.Http +open System.Threading +open System.Threading.Tasks let mutable apiCallStopwatch = System.Diagnostics.Stopwatch.StartNew() let mutable lastApiCallTimeInMs : int64 = int64 (0) @@ -29,3 +33,53 @@ let sleepUntilNextRateLimitedApiCallTime (callsPerSec: int) = System.Threading.Thread.Sleep(toSleep) lastApiCallTimeInMs <- apiCallStopwatch.ElapsedMilliseconds + +// Retries apiserver 429s so a single rejection cannot end a mission. +type ThrottleRetryHandler(deadline: System.TimeSpan) = + inherit DelegatingHandler() + + // F# cannot call `base` from inside a task expression, so the base send needs its own member. + member private this.Send(req: HttpRequestMessage, ct: CancellationToken) = base.SendAsync(req, ct) + + override this.SendAsync(req: HttpRequestMessage, ct: CancellationToken) : Task = + // Deletes are never retried, because every delete site in this library already + // swallows failure, so retrying buys fewer orphans at the price of multiplying a + // teardown that removes hundreds of objects in sequence. + if req.Method = HttpMethod.Delete then + this.Send(req, ct) + else + let sw = System.Diagnostics.Stopwatch.StartNew() + + // Only 429 is retried, because it alone proves the request was rejected unapplied and is safe to re-send. + let rec attempt backoffMs = + task { + let! r = this.Send(req, ct) + + if r.StatusCode <> HttpStatusCode.TooManyRequests || sw.Elapsed >= deadline then + return r + else + // Retry-After is a floor, not a replacement, or a server repeating `Retry-After: 1` pins us at one attempt per second. + let hint = + match r.Headers.RetryAfter with + | ra when not (isNull ra) && ra.Delta.HasValue -> int ra.Delta.Value.TotalMilliseconds + | _ -> 0 + + let waitMs = max backoffMs hint + + LogWarn + "apiserver throttled %s %s (%O elapsed); retrying in %d ms" + req.Method.Method + req.RequestUri.PathAndQuery + sw.Elapsed + waitMs + + r.Dispose() + do! Task.Delay(waitMs, ct) + return! attempt (min (backoffMs * 2) 15000) + } + + task { + // A request can only be sent once unless its body is buffered first. + if not (isNull req.Content) then do! req.Content.LoadIntoBufferAsync() + return! attempt 500 + } diff --git a/src/FSLibrary/StellarSupercluster.fs b/src/FSLibrary/StellarSupercluster.fs index 2c7280fd..f042bd34 100644 --- a/src/FSLibrary/StellarSupercluster.fs +++ b/src/FSLibrary/StellarSupercluster.fs @@ -191,7 +191,12 @@ let ConnectToCluster (cfgFile: string) (nsOpt: string option) : (Kubernetes * st let clientConfig = KubernetesClientConfiguration.BuildConfigFromConfigObject(kCfg) // Disable HTTP2 to avoid intermittent issues with the cluster clientConfig.DisableHttp2 <- true - let kube = new k8s.Kubernetes(clientConfig) + // Rides out apiserver 429s for every call this client makes, and must stay + // well under clientConfig.HttpClientTimeout (100s), which bounds the whole + // handler chain and surfaces as a TaskCanceledException that loses the 429. + let kube = + new k8s.Kubernetes(clientConfig, new ApiRateLimit.ThrottleRetryHandler(System.TimeSpan.FromSeconds 60.0)) + (kube, ns) // Prints the stellar-core StatefulSets and Pods on the provided cluster From 4295a0477d727fd124340df02484118c7c21a3a0 Mon Sep 17 00:00:00 2001 From: Jonathan Eid Date: Mon, 17 Aug 2026 15:28:23 -0400 Subject: [PATCH 2/4] Clamp the retry wait to the remaining deadline The deadline was checked before the wait, so the wait itself was unbudgeted: a 429 arriving at 59s under a 60s budget still waited, and a long Retry-After made the overrun arbitrary. APF sent 16s and 32s hints during testing, which is enough to start an attempt past the deadline and past HttpClientTimeout (100s), where the 429 is replaced by a TaskCanceledException that loses the response body and request path -- the exact diagnosability the 60s budget was chosen to preserve. Clamping to the remaining budget rather than bailing out keeps the whole budget usable: the final attempt still starts, it just starts no later than the deadline. Co-Authored-By: Claude Opus 5 --- src/FSLibrary.Tests/Tests.fs | 14 ++++++++++++++ src/FSLibrary/ApiRateLimit.fs | 7 +++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/FSLibrary.Tests/Tests.fs b/src/FSLibrary.Tests/Tests.fs index 620537c2..96de712f 100644 --- a/src/FSLibrary.Tests/Tests.fs +++ b/src/FSLibrary.Tests/Tests.fs @@ -762,3 +762,17 @@ let ``Throttle retry gives up at the deadline and surfaces the 429`` () = // The 429 must reach the caller rather than being swallowed or masked. Assert.Equal(System.Net.HttpStatusCode.TooManyRequests, resp.StatusCode) Assert.Equal(1, stub.Calls) + +[] +let ``Throttle retry clamps the last wait so it never overruns the deadline`` () = + let stub = ThrottlingStub(1000) + // 750ms budget: 500ms backoff fits, the 1000ms one is clamped to what is left. + let handler = new ApiRateLimit.ThrottleRetryHandler(System.TimeSpan.FromMilliseconds 750.0) + let sw = System.Diagnostics.Stopwatch.StartNew() + let resp = sendThrough handler stub System.Net.Http.HttpMethod.Get + sw.Stop() + Assert.Equal(System.Net.HttpStatusCode.TooManyRequests, resp.StatusCode) + // Two waits (500ms, then 250ms clamped) and three attempts. + Assert.Equal(3, stub.Calls) + // The clamp is the point: without it the second wait would have run to 1500ms. + Assert.True(sw.Elapsed < System.TimeSpan.FromMilliseconds 1400.0, sprintf "took %O" sw.Elapsed) diff --git a/src/FSLibrary/ApiRateLimit.fs b/src/FSLibrary/ApiRateLimit.fs index 473aa80d..5183a69a 100644 --- a/src/FSLibrary/ApiRateLimit.fs +++ b/src/FSLibrary/ApiRateLimit.fs @@ -54,8 +54,10 @@ type ThrottleRetryHandler(deadline: System.TimeSpan) = let rec attempt backoffMs = task { let! r = this.Send(req, ct) + let remaining = deadline - sw.Elapsed - if r.StatusCode <> HttpStatusCode.TooManyRequests || sw.Elapsed >= deadline then + if r.StatusCode <> HttpStatusCode.TooManyRequests + || remaining <= System.TimeSpan.Zero then return r else // Retry-After is a floor, not a replacement, or a server repeating `Retry-After: 1` pins us at one attempt per second. @@ -64,7 +66,8 @@ type ThrottleRetryHandler(deadline: System.TimeSpan) = | ra when not (isNull ra) && ra.Delta.HasValue -> int ra.Delta.Value.TotalMilliseconds | _ -> 0 - let waitMs = max backoffMs hint + // Clamped to what is left, because the wait is otherwise unbudgeted and a long Retry-After would start an attempt past the deadline and past HttpClientTimeout, replacing the 429 with a TaskCanceledException. + let waitMs = min (max backoffMs hint) (int remaining.TotalMilliseconds) LogWarn "apiserver throttled %s %s (%O elapsed); retrying in %d ms" From 45f0f2a8df644e079725a2545765630aa8f309b2 Mon Sep 17 00:00:00 2001 From: Jonathan Eid Date: Mon, 17 Aug 2026 15:34:09 -0400 Subject: [PATCH 3/4] Never start a retry the deadline cannot pay for Replaces clamping the final wait with refusing the attempt outright, per review feedback: the wait now counts against the budget, so a retry is only started when the deadline can cover it. Clamping kept the last attempt inside the deadline but still began an attempt whose own duration was unbounded; refusing outright means the response returned to the caller is always a real 429 with its body and request path, never a TaskCanceledException from HttpClientTimeout. Co-Authored-By: Claude Opus 5 --- src/FSLibrary.Tests/Tests.fs | 12 ++++++------ src/FSLibrary/ApiRateLimit.fs | 21 ++++++++++----------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/FSLibrary.Tests/Tests.fs b/src/FSLibrary.Tests/Tests.fs index 96de712f..418cabcb 100644 --- a/src/FSLibrary.Tests/Tests.fs +++ b/src/FSLibrary.Tests/Tests.fs @@ -764,15 +764,15 @@ let ``Throttle retry gives up at the deadline and surfaces the 429`` () = Assert.Equal(1, stub.Calls) [] -let ``Throttle retry clamps the last wait so it never overruns the deadline`` () = +let ``Throttle retry never starts an attempt the budget cannot pay for`` () = let stub = ThrottlingStub(1000) - // 750ms budget: 500ms backoff fits, the 1000ms one is clamped to what is left. + // 750ms budget: the 500ms backoff fits, the 1000ms one does not, so it stops. let handler = new ApiRateLimit.ThrottleRetryHandler(System.TimeSpan.FromMilliseconds 750.0) let sw = System.Diagnostics.Stopwatch.StartNew() let resp = sendThrough handler stub System.Net.Http.HttpMethod.Get sw.Stop() Assert.Equal(System.Net.HttpStatusCode.TooManyRequests, resp.StatusCode) - // Two waits (500ms, then 250ms clamped) and three attempts. - Assert.Equal(3, stub.Calls) - // The clamp is the point: without it the second wait would have run to 1500ms. - Assert.True(sw.Elapsed < System.TimeSpan.FromMilliseconds 1400.0, sprintf "took %O" sw.Elapsed) + // One wait of 500ms and two attempts; the second wait would have overrun. + Assert.Equal(2, stub.Calls) + // Stopping early is the point: it must not have slept out the full budget. + Assert.True(sw.Elapsed < System.TimeSpan.FromMilliseconds 750.0, sprintf "took %O" sw.Elapsed) diff --git a/src/FSLibrary/ApiRateLimit.fs b/src/FSLibrary/ApiRateLimit.fs index 5183a69a..14bb8983 100644 --- a/src/FSLibrary/ApiRateLimit.fs +++ b/src/FSLibrary/ApiRateLimit.fs @@ -54,21 +54,20 @@ type ThrottleRetryHandler(deadline: System.TimeSpan) = let rec attempt backoffMs = task { let! r = this.Send(req, ct) - let remaining = deadline - sw.Elapsed + // Retry-After is a floor, not a replacement, or a server repeating `Retry-After: 1` pins us at one attempt per second. + let hint = + match r.Headers.RetryAfter with + | ra when not (isNull ra) && ra.Delta.HasValue -> int ra.Delta.Value.TotalMilliseconds + | _ -> 0 + + let waitMs = max backoffMs hint + + // The wait counts against the budget, so an attempt the budget cannot pay for is never started: a long Retry-After would otherwise begin one past the deadline and past HttpClientTimeout, replacing the 429 with a TaskCanceledException. if r.StatusCode <> HttpStatusCode.TooManyRequests - || remaining <= System.TimeSpan.Zero then + || sw.Elapsed + System.TimeSpan.FromMilliseconds(float waitMs) >= deadline then return r else - // Retry-After is a floor, not a replacement, or a server repeating `Retry-After: 1` pins us at one attempt per second. - let hint = - match r.Headers.RetryAfter with - | ra when not (isNull ra) && ra.Delta.HasValue -> int ra.Delta.Value.TotalMilliseconds - | _ -> 0 - - // Clamped to what is left, because the wait is otherwise unbudgeted and a long Retry-After would start an attempt past the deadline and past HttpClientTimeout, replacing the 429 with a TaskCanceledException. - let waitMs = min (max backoffMs hint) (int remaining.TotalMilliseconds) - LogWarn "apiserver throttled %s %s (%O elapsed); retrying in %d ms" req.Method.Method From cdf8c5e6b8d8c70812f35087b9b8b560a6826f7e Mon Sep 17 00:00:00 2001 From: Jonathan Eid Date: Mon, 17 Aug 2026 16:02:49 -0400 Subject: [PATCH 4/4] Construct the test stub with `new` to silence FS0728 ThrottlingStub inherits HttpMessageHandler, which is IDisposable, so F# warns when it is built without `new`. The stub is still disposed by the HttpMessageInvoker's `use` binding, which owns the handler chain. Co-Authored-By: Claude Opus 5 --- src/FSLibrary.Tests/Tests.fs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/FSLibrary.Tests/Tests.fs b/src/FSLibrary.Tests/Tests.fs index 418cabcb..34aa0d08 100644 --- a/src/FSLibrary.Tests/Tests.fs +++ b/src/FSLibrary.Tests/Tests.fs @@ -739,7 +739,7 @@ let private sendThrough [] let ``Throttle retry rides out 429s and returns the eventual success`` () = - let stub = ThrottlingStub(3) + let stub = new ThrottlingStub(3) let handler = new ApiRateLimit.ThrottleRetryHandler(System.TimeSpan.FromSeconds 30.0) let resp = sendThrough handler stub System.Net.Http.HttpMethod.Get Assert.Equal(System.Net.HttpStatusCode.OK, resp.StatusCode) @@ -748,7 +748,7 @@ let ``Throttle retry rides out 429s and returns the eventual success`` () = [] let ``Throttle retry leaves DELETE alone so teardown stays bounded`` () = - let stub = ThrottlingStub(5) + let stub = new ThrottlingStub(5) let handler = new ApiRateLimit.ThrottleRetryHandler(System.TimeSpan.FromSeconds 30.0) let resp = sendThrough handler stub System.Net.Http.HttpMethod.Delete Assert.Equal(System.Net.HttpStatusCode.TooManyRequests, resp.StatusCode) @@ -756,7 +756,7 @@ let ``Throttle retry leaves DELETE alone so teardown stays bounded`` () = [] let ``Throttle retry gives up at the deadline and surfaces the 429`` () = - let stub = ThrottlingStub(1000) + let stub = new ThrottlingStub(1000) let handler = new ApiRateLimit.ThrottleRetryHandler(System.TimeSpan.Zero) let resp = sendThrough handler stub System.Net.Http.HttpMethod.Get // The 429 must reach the caller rather than being swallowed or masked. @@ -765,7 +765,7 @@ let ``Throttle retry gives up at the deadline and surfaces the 429`` () = [] let ``Throttle retry never starts an attempt the budget cannot pay for`` () = - let stub = ThrottlingStub(1000) + let stub = new ThrottlingStub(1000) // 750ms budget: the 500ms backoff fits, the 1000ms one does not, so it stops. let handler = new ApiRateLimit.ThrottleRetryHandler(System.TimeSpan.FromMilliseconds 750.0) let sw = System.Diagnostics.Stopwatch.StartNew()