Pace Docker Hub against its rolling window - #280
Merged
Conversation
x-ratelimit-reset is not a window boundary. It is recomputed on every response as roughly now+58s, an estimate of when the oldest request ages out. A 150-request probe held remaining flat at 573 for 85s rather than sawtoothing back to 600, which is a rolling minute, not a fixed one. apply_rate/2 gated on `reset == state.window`, so after the first response of a window that comparison stopped matching and every later reading of remaining was discarded. What was left was a fixed-window model: burst the whole limit, park until the anchor elapsed, burst again. Against a rolling counter the second burst lands while the first is still inside the window, which is how remaining reached -534 in production. The gate now keeps its own send times and admits only while fewer than limit of them fall in the trailing window, the same model the server applies. Slots free one at a time as sends age out instead of a whole budget at once. The budget is metered per account, not per source IP: a node deleting at 27 requests/minute took 56 429s in 92 requests while another node burst from a different IP. Reading remaining on every response is what carries that across nodes, so one backs off when another spends, with no shared state. It is read against the requests still in flight, which the server has not counted, and dropped once it is older than the window. A 429 now parks the gate. It previously fed through the same reset mismatch and did nothing, so paced_request re-acquired immediately from a limiter that still believed it had budget, up to 20 times per request. Slots are taken per HTTP attempt rather than per call, so the retries Bob.HTTP makes for a transport error or a 5xx are counted too, and every granted caller is monitored so one that dies without reporting cannot hold its slot for good.
ericmj
marked this pull request as ready for review
July 30, 2026 22:57
Four defects, each with a case that reproduces it. The unmeasured path admitted on `in_flight == 0` alone, ignoring both `limit` and the trailing sends. A window's worth of responses carrying no usable rate headers, which is what a run of transport errors or 5xx looks like, left the gate with nothing bounding it: measured at 7 sends inside one window against a limit of 2. A slot was held from acquire until the caller reported, with nothing bounding the wait. `receive_timeout` applies per receive, so a response that trickles holds one indefinitely, and the gate is a singleton: one wedged caller stopped every Docker Hub request on the node. Slots older than max_hold_ms are now taken back. A waiter killed while queued was still handed a slot, spending a unit of window for a request never made. Bob.Runner kills up to 25 cleanup tasks at once on a job timeout, any of which may be in acquire. `acquire` took a slot that had just opened even with callers already queued, so a latecomer could barge past a waiter. It now queues and admits from the front. Pacing needed the same reading to work harder. A reading was treated as true for a whole window and then discarded, but the budget it describes recovers continuously, so the gate spent whole windows admitting a single request. It now carries the reading forward: sends since it was taken are subtracted, sends that have left the window are added back. Against a rolling-window model of the account at concurrency 25 that is 591/min of the 600/min ceiling, against 360/min before, with the hub peaking at 607 in a window. Grants clear headroom the size of the in-flight set, because crediting runs slightly ahead of when the server drops a request from its own window. A 429 without parseable headers is read as a spent budget rather than leaving the previous reading in place, which would have released a burst as soon as the park cleared. The retry cap drops to 5, since Bob.HTTP.retry multiplies its own attempts on top of it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
x-ratelimit-resetis not a window boundary. It is recomputed on every response as roughly now+58s, an estimate of when the oldest request ages out. Probing the delete endpoint at one request every 2.2s heldremainingflat at 573 for 85 seconds rather than sawtoothing back to 600, which is a rolling minute.apply_rate/2gated onreset == state.window, so once the first response of a window had landed that comparison stopped matching and every later reading ofremainingwas discarded. What was left was a fixed-window model: burst the whole limit, park until the anchor elapsed, burst again. Against a rolling counter the second burst lands while the first is still inside the window, which is howremainingreached -534 in production.The gate now keeps its own send times and admits while fewer than
limitof them fall in the trailing window, the model the server applies, so slots free one at a time as sends age out.The budget is metered per account, not per source IP. A node deleting at 27 requests/minute took 56 429s in 92 requests while another node burst from a different IP, both reporting distinct
x-ratelimit-ipvalues. Readingremainingon every response is what carries that between nodes, so one backs off when another spends, with no shared state. It is read against the requests still in flight, which the server has not counted, and dropped once older than the window it was taken in.A 429 now parks the gate. It previously fed through the same reset mismatch and did nothing, so
paced_requestre-acquired immediately from a limiter that still believed it had budget, up to 20 times per request.Slots are taken per HTTP attempt rather than per call, so the retries
Bob.HTTPmakes for a transport error or a 5xx count against the budget too. Every granted caller is monitored so one that dies without reporting gives its slot back.This paces the Reconcile pager as well as the tag cleanup.