Pagination and Rate-Limit Handling In Docker Registry Namespace API Calls #4557
Conversation
…try list images calls
kashifkhan0771
left a comment
kashifkhan0771
left a comment
There was a problem hiding this comment.
My suggestion is to keep most of the existing code as-is and simply introduce a fixed page size (e.g., 30) for all registry APIs. Then we can wrap the current logic for all registries in a loop that continues until next_page is empty, and use golang.org/x/time/rate to generically control the number of requests per second.
Even though it’s unlikely we’ll hit rate limits, since how many namespaces will have hundreds of images and this gives us a consistent safeguard so we don’t accidentally overload their APIs in any scenario.
…some unnecessary logic in Quay.ListImages()
…retry-in-docker-registries
…retry-in-docker-registries
…imit-retry-in-docker-registries' into update/source/add-pagination-and-rate-limit-retry-in-docker-registries
…retry-in-docker-registries
camgunz
left a comment
camgunz
left a comment
There was a problem hiding this comment.
Thanks for picking this up! I think we should use the Retryable HTTP client from common: we should get the retry stuff for free, and we've been adding super useful metrics to it too.
kashifkhan0771
left a comment
kashifkhan0771
left a comment
There was a problem hiding this comment.
Good Work! Overall looks fine to me. Just make sure to test it properly locally with popular organization images.
pkg/sources/docker/registries.go
Outdated
| // registryRateLimiter limits how quickly we make registry API calls across all registries. | ||
| // We allow roughly 5 requests every ~7.5 seconds (one token every 1.5s) as a simple | ||
| // safeguard against overloading upstream APIs. | ||
| var registryRateLimiter = rate.NewLimiter(rate.Every(1500*time.Millisecond), 1) |
There was a problem hiding this comment.
// 1 event every 1.5s, burst of 2
var registryRateLimiter = rate.NewLimiter(rate.Limit(2.0/3.0), 2)
pkg/sources/docker/registries.go
Outdated
| return nil, err | ||
| } | ||
| query := baseURL.Query() | ||
| query.Set("page_size", "100") // fetch images in batches of 100 per page |
There was a problem hiding this comment.
Let's make the page size a constant and I would suggest to keep it maybe 30-40 per page but I'll leave that up to you.
There was a problem hiding this comment.
I'm inclined to keep it maximum, while it's unlikely to have that many images in a page, it still shouldn't hurt just in case
5 participants
Description:
This PR adds pagination and rate-limit prevention for Dockerhub, Quay, and GHCR namespace discovery. This will ensure we fetch all images under a namespace while avoiding
429 Too Many Requestsresponses without missing results that span across pages.Updates:
Implemented pagination for DockerHub (using
nextkey), Quay (next_page), and GHCR (Linkheader).Added
golang.org/x/time/rateto prevent429responses by limiting a single request per 1.5 seconds.Checklist:
make test-community)?make lintthis requires golangci-lint)?