Repository navigation
Share upstream rate-limit holds across instances - #601
Merged
Merged
Conversation
Holds placed by the rate limit gate are written to a new upstream_holds table, and each instance reads that table at most every two seconds and refuses requests to a host any instance holds. Before, every replica had to get its own 429 before it stopped sending. Instances waiting on a fetch lease reread the holds when the fetch fails, so when the holder was rate limited they answer 503 with its Retry-After instead of each sending one more request to the limited host.
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
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.
The rate-limit hold from #483 lives in the
RateLimitGateof each process. With several instances on one database, each had to get its own 429 before it stopped sending, and with the fetch lease from #570 a rate-limited holder made every waiter without a stale copy fetch the same file at once, so one 429 turned into N-1 more requests to the limited host.upstream_holdstable (host, end of hold in Unix ms; migration018_add_upstream_holds(017 is taken by Cache Git LFS objects #595) plus the fresh schemas for SQLite and Postgres) records holds.SharedRateLimitGateininternal/httpclientwraps the registries gate: it writes the gate's holds fromOnHold, before the 429 response returns, and refuses requests to a host any instance holds with the sameHeldError, sowriteUpstreamHeldanswers 503 withRetry-Afteras before.Retry-Afterinstead of fetching. Other failures behave as before.Proxy.RateLimitsis now a small interface (Held), so tests can still pass a plain gate./healthlists shared holds too.Single instance: the shared view only holds what the local gate already holds, with the same end, so requests are refused at the same times. The one difference is that a hold now outlasts a restart.
Not changed: the fallback hold without
Retry-Afterstill doubles per instance, and when a shared hold ends every instance may probe at the same moment. Fixing either would need the gate in registries to accept an external hold (or expose its fallback state), rather than a wrapper.Tests: two proxies on one SQLite file with an httptest upstream answering 429 (
internal/handler/shared_holds_test.go): the second sends nothing for artifacts or metadata after the first is limited, and the hold goes away once it ends. A Postgres test checks that lease waiters answer 503 and the upstream sees one request. Both fail without the change. Unit tests for the refresh interval, reload and expiry, and for the queries on both databases.go test ./... -count=1with and withoutPROXY_DATABASE_URL,-raceon the touched packages, golangci-lint clean.