Skip to content

Share upstream rate-limit holds across instances - #601

Merged
andrew merged 1 commit into
git-pkgs:mainfrom
simonchrz:perf/shared-rate-limit-holds
Oct 25, 2026
Merged

andrew merged 1 commit into
git-pkgs:mainfrom
simonchrz:perf/shared-rate-limit-holds

Conversation

@simonchrz

Copy link
Copy Markdown
Contributor

The rate-limit hold from #483 lives in the RateLimitGate of 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.

  • A new upstream_holds table (host, end of hold in Unix ms; migration 018_add_upstream_holds (017 is taken by Cache Git LFS objects #595) plus the fresh schemas for SQLite and Postgres) records holds. SharedRateLimitGate in internal/httpclient wraps the registries gate: it writes the gate's holds from OnHold, before the 429 response returns, and refuses requests to a host any instance holds with the same HeldError, so writeUpstreamHeld answers 503 with Retry-After as before.
  • Each instance keeps a copy of the table and rereads it at most every 2 s, only when it checks a host. Another instance can therefore still send for up to 2 s after a hold starts. A failed read keeps the last copy. Rows that have ended are deleted on the next read that sees them.
  • When a fetch lease ends with a failed fetch, the waiters reread the holds at once. If the holder was rate limited, the waiters without a stale copy now answer 503 with the holder's Retry-After instead of fetching. Other failures behave as before.
  • Proxy.RateLimits is now a small interface (Held), so tests can still pass a plain gate. /health lists 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-After still 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=1 with and without PROXY_DATABASE_URL, -race on the touched packages, golangci-lint clean.

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.
@andrew
andrew merged commit 3c2449e into git-pkgs:main Oct 25, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants