Hold requests to an upstream that rate limits the proxy

[gilesw]

When an upstream host answers 429, or 503 with Retry-After, the proxy keeps sending it requests. Metadata fetches have no retry or circuit breaker, so a rate-limited upstream keeps getting one request per cache miss for as long as the block lasts. That can extend the block.

This adds a RoundTripper under the auth transport that holds a host after a 429, or a 503 with Retry-After. While the hold lasts, the proxy sends that host nothing. The hold lasts as long as Retry-After asks, capped at an hour. Without Retry-After it starts at a minute and doubles up to fifteen minutes. Because the gate sits in the shared transport, metadata, artifact and passthrough fetches all obey it.

Held requests fail locally:

• Cached metadata is served stale, as before.
• Anything uncached gets a 503 with the remaining Retry-After, so the client also slows down.

Observability:

• /health lists held hosts under upstream_holds.
• proxy_upstream_rate_limit_holds_total counts holds.

Tested with go build ./..., go vet ./... and go test ./..., and by running the proxy against a live server.

[andrew]

Holding the host at the transport is the right place for this. Three things need changing before it can go in, plus one small cleanup.

A late success clears an active hold. RoundTrip calls g.release(host) on any response that isn't limited, so a request that was already in flight when the hold started and comes back 200 afterwards wipes the hold and resets strikes. Rate limiters often let some requests through, so on a busy proxy this is the common case. Since the gate sends nothing while a hold is active, any response arriving during one was sent before it began: have release only delete a hold whose until has passed, and add a test where a 429 is followed by a late 200 and the hold survives.

Held artifact fetches trip the circuit breaker. CircuitBreakerFetcher in git-pkgs/registries counts every error from breaker.Call as a failure, including HeldError, so after five held cache misses the host's breaker opens. From then on artifact misses get the breaker's 502 without Retry-After, proxy_circuit_breaker_trips_total and /health report an outage, and probes during the hold fail and push the breaker's backoff towards five minutes, which can keep the host blocked after the hold ends. Add a Held(url) to RateLimitGate and check it in getOrFetchArtifactFromURL before calling the fetcher, returning the HeldError directly so it never reaches the breaker, with a test that makes more than five held misses and checks the breaker stays closed.

Most handlers still answer a hold with 502. writeUpstreamHeld is wired into the shared Proxy helpers and Maven metadata, but the handlers that fetch metadata themselves (npm, PyPI, Composer, gem, hex, julia, swift, conan, conda, nuget and the container routes, about 20 HTTPClient.Do / fetchOrCacheMetadata call sites) turn the HeldError into a 502. Add the writeUpstreamHeld check to each of those error branches so every uncached request during a hold gets the 503 and Retry-After the README describes.

RecordUpstreamRateLimitHold(host string, _ time.Duration) only takes the duration to match OnHold and never uses it; drop it from OnHold and the function.

[gilesw]

Thanks. All four are addressed in b2955ef.

Late success clears a hold. release now only deletes a hold whose until has passed. TestRateLimitGate_LateSuccessKeepsHold sends a request that is in flight when a 429 starts the hold and gets its 200 afterwards. The test checks that the hold and its deadline survive and that the next request is still held.

Held artifact fetches trip the breaker. Added RateLimitGate.Held(url), nil-safe, and a Proxy.RateLimits field that the server sets in serve and NewMirrorProxy. The check runs in fetchAndCache, fetchAndCacheFromURL and streamFromUpstream, before the fetcher is called. That covers getOrFetchArtifactFromURL, GetOrFetchArtifact and the streaming path, and it keeps held misses out of the fetch_failed upstream error metric. TestMavenHandler_HeldArtifactMissesSkipCircuitBreaker makes eight held misses through a real CircuitBreakerFetcher. It checks that each one gets a 503 with Retry-After, that the upstream is never called again, and that the breaker stays closed. Without the change, the sixth miss gets the breaker's 502.

Handlers still answering 502. Added the writeUpstreamHeld check to every error branch that turns an upstream fetch error into a 502. That covers npm, npm hosted, PyPI, Composer, Composer hosted, gem, hex, NuGet and its cooldown metadata, conda, conan, Swift, cargo, pub, Helm, Arch, Go, the Go checksum database, and the container manifest, tag, referrer and blob routes. Branches for decode, filter, rewrite or digest-mismatch errors still return 502. TestHandlers_UpstreamHeldMetadataReturns503 covers npm, PyPI simple and JSON, cargo, pub, NuGet (service index and search) and conda.

RecordUpstreamRateLimitHold. Dropped the duration from both OnHold and the function.

go vet ./... and go test ./... pass.