feat: background syncer for github_rpm remotes (#108)
## Why Builds on #107 (merged), which derives `github_rpm` RPM metadata lazily on the client request path, single-flighted per replica. Two problems remain: the derive still happens per replica, so across a multi-replica deployment the same releases are scanned and re-derived N times, multiplying GitHub queries; and a cold cache blocks the first request on a full derive. GitHub's rate limits are low (~60/hr unauthenticated, ~5000/hr authenticated), so this needs a single coordinated syncer with a shared rate limit and conditional requests. ## How - Add a single per-process background syncer (started at boot, cleanly stopped on shutdown) that owns a deduped/coalescing work queue, a worker pool, and one global token-bucket rate limiter (`golang.org/x/time/rate`) bound onto the github provider so every GitHub call (releases list + each ranged asset GET) acquires a token first. - Re-check each `github_rpm` remote for new/changed releases on its existing `mutable_ttl` cadence; derive only new/changed assets incrementally and prune assets that disappear upstream. Repodata is served from primed DB rows. - Prime metadata in the background on remote creation; the create call returns immediately. - Send the stored releases-list `ETag` as `If-None-Match`; a `304` derives nothing and is not counted against GitHub's rate limit, so an unchanged repo is nearly free. - Coordinate replicas through a `github_rpm_sync_state` row (`last_synced_at`, `etag`, `sync_lease_owner`, `sync_lease_expires`): a periodic scan runs only for the replica that atomically claims the lease, bounding total GitHub load to ~once per `mutable_ttl` regardless of replica count; the ETag is shared through the same row. - Keep the request path fast: serve current cache, enqueue a prime on an empty cache, and return a bounded wait then a retryable `503` rather than blocking on a cold derive. - Add `GITHUB_SYNC_RATE` / `GITHUB_SYNC_BURST` / `GITHUB_SYNC_WORKERS` / `GITHUB_SYNC_POLL_INTERVAL` config with conservative defaults (1 req/s, burst 5, 3 workers, 60s tick) and document the syncer in the README. ## Tests - Unit (httptest, Range/ETag-aware fixture): `304` releases response derives nothing; incremental derive fetches only the newly added asset; the shared limiter caps request rate; work-queue enqueues coalesce to one job; prime enqueues a job; a held lease stops a second replica from scanning; cold-start serves `503` while warm cache serves `200`. - DB integration (testcontainers postgres): the real lease SQL — one holder at a time, recency gate blocks a too-soon periodic re-claim, prime (freshness 0) bypasses recency but respects a live lease. - Docker e2e re-run: `dnf install dotvault` works; prime-on-create derives in the background at ~1 req/s (global limiter); `dnf makecache` served fast from the priming cache (no cold block); clean shutdown mid-scan, no panics. ## Notes - Reuses `mutable_ttl` as the check interval (no new per-remote field), per brief. Reviewed-on: #108 Co-authored-by: Ben Vincent <ben@unkin.net> Co-committed-by: Ben Vincent <ben@unkin.net>
This commit was merged in pull request #108.
This commit is contained in:
+148
-15
@@ -18,13 +18,19 @@ import (
|
||||
"time"
|
||||
|
||||
rpmlib "github.com/cavaliergopher/rpm"
|
||||
"golang.org/x/time/rate"
|
||||
|
||||
"git.unkin.net/unkin/artifactapi/internal/provider"
|
||||
"git.unkin.net/unkin/artifactapi/pkg/models"
|
||||
)
|
||||
|
||||
// gitHubProvider is the process-wide singleton. The background Syncer binds its
|
||||
// shared rate limiter and work queue onto this instance so the request path and
|
||||
// the syncer drive the same derive machinery.
|
||||
var gitHubProvider = newGitHubProvider()
|
||||
|
||||
func init() {
|
||||
provider.Register(newGitHubProvider())
|
||||
provider.Register(gitHubProvider)
|
||||
}
|
||||
|
||||
// Tuning knobs for the no-precache header fetch. Fields (not consts) so tests
|
||||
@@ -39,6 +45,12 @@ const (
|
||||
defaultScanTimeout = 10 * time.Minute
|
||||
// defaultServeTimeout bounds a repodata DB read served on a detached context.
|
||||
defaultServeTimeout = 30 * time.Second
|
||||
|
||||
// defaultColdWait bounds how long a repodata request blocks waiting for a
|
||||
// just-enqueued prime to populate an empty cache before returning a
|
||||
// retryable 503. Kept short so a client never hangs on a rate-limited derive
|
||||
// of a large repo; small repos usually prime within this window.
|
||||
defaultColdWait = 8 * time.Second
|
||||
)
|
||||
|
||||
// GitHubProvider is a metadata-only remote: it scans a GitHub repo's releases
|
||||
@@ -54,6 +66,15 @@ type GitHubProvider struct {
|
||||
pageCap int
|
||||
scanTimeout time.Duration
|
||||
serveTimeout time.Duration
|
||||
coldWait time.Duration
|
||||
|
||||
// limiter, when set by the Syncer, gates every GitHub HTTP call (releases
|
||||
// list + each ranged asset fetch) through a single process-wide token bucket.
|
||||
// nil means unlimited (direct provider use / unit tests).
|
||||
limiter *rate.Limiter
|
||||
// syncer, when set, routes freshness refresh and cold-start priming through
|
||||
// the shared background work queue instead of an inline per-replica scan.
|
||||
syncer *Syncer
|
||||
|
||||
mu sync.Mutex
|
||||
scanning map[string]bool
|
||||
@@ -68,11 +89,21 @@ func newGitHubProvider() *GitHubProvider {
|
||||
pageCap: defaultReleasePageCap,
|
||||
scanTimeout: defaultScanTimeout,
|
||||
serveTimeout: defaultServeTimeout,
|
||||
coldWait: defaultColdWait,
|
||||
scanning: map[string]bool{},
|
||||
lastScan: map[string]time.Time{},
|
||||
}
|
||||
}
|
||||
|
||||
// limiterWait blocks until the shared rate limiter grants a token, or returns
|
||||
// the context error if it is canceled first. A nil limiter is a no-op.
|
||||
func (p *GitHubProvider) limiterWait(ctx context.Context) error {
|
||||
if p.limiter == nil {
|
||||
return nil
|
||||
}
|
||||
return p.limiter.Wait(ctx)
|
||||
}
|
||||
|
||||
func (p *GitHubProvider) Type() models.PackageType { return models.PackageGitHubRPM }
|
||||
|
||||
// Classify/ContentType/UpstreamURL/RewriteResponse/AuthHeaders satisfy the
|
||||
@@ -114,7 +145,7 @@ func (p *GitHubProvider) AuthHeaders(_ context.Context, remote models.Remote) (h
|
||||
// 302-redirects .rpm downloads to the backend releases_remote. Returns false
|
||||
// only for paths it does not own, letting the normal proxy path take over.
|
||||
func (p *GitHubProvider) ServeRemote(w http.ResponseWriter, r *http.Request, remote models.Remote, path, proxyBaseURL string, store provider.RemoteMetadataStore) bool {
|
||||
p.refresh(remote, store)
|
||||
p.onRequest(remote, store)
|
||||
|
||||
if strings.HasPrefix(path, "repodata/") {
|
||||
// Serve repodata on a context detached from the inbound request: a
|
||||
@@ -124,6 +155,15 @@ func (p *GitHubProvider) ServeRemote(w http.ResponseWriter, r *http.Request, rem
|
||||
defer cancel()
|
||||
sr := r.WithContext(sctx)
|
||||
|
||||
// Cold start: with the syncer wired, an empty cache means the prime has
|
||||
// not landed yet. Enqueue it and wait briefly rather than serving empty
|
||||
// repodata; if it still has not primed, return a retryable 503.
|
||||
if p.syncer != nil && !p.ensurePrimed(sctx, remote, store) {
|
||||
w.Header().Set("Retry-After", "5")
|
||||
http.Error(w, "metadata is being prepared, retry shortly", http.StatusServiceUnavailable)
|
||||
return true
|
||||
}
|
||||
|
||||
tail := strings.TrimPrefix(path, "repodata/")
|
||||
lp := &Provider{}
|
||||
switch {
|
||||
@@ -154,6 +194,56 @@ func (p *GitHubProvider) ServeRemote(w http.ResponseWriter, r *http.Request, rem
|
||||
return false
|
||||
}
|
||||
|
||||
// onRequest keeps a remote's derived metadata fresh off the request path. With
|
||||
// the background syncer wired it enqueues a deduped, rate-limited, lease-gated
|
||||
// refresh and returns immediately; the request always serves the current cache.
|
||||
// Without a syncer (direct provider use / unit tests) it falls back to the
|
||||
// legacy inline single-flight scan.
|
||||
func (p *GitHubProvider) onRequest(remote models.Remote, store provider.RemoteMetadataStore) {
|
||||
if p.syncer != nil {
|
||||
p.syncer.enqueue(remote, false)
|
||||
return
|
||||
}
|
||||
p.refresh(remote, store)
|
||||
}
|
||||
|
||||
// ensurePrimed returns true once the remote has at least one cached metadata
|
||||
// row. On an empty cache it enqueues a prime and polls briefly for it to land,
|
||||
// so the very first client after a remote is created gets real repodata instead
|
||||
// of an empty index or a blocking multi-minute derive. Returns false if the
|
||||
// cache is still empty after the bounded wait.
|
||||
func (p *GitHubProvider) ensurePrimed(ctx context.Context, remote models.Remote, store provider.RemoteMetadataStore) bool {
|
||||
if !p.cacheEmpty(ctx, store, remote.Name) {
|
||||
return true
|
||||
}
|
||||
if p.syncer != nil {
|
||||
p.syncer.enqueue(remote, true)
|
||||
}
|
||||
|
||||
deadline := time.Now().Add(p.coldWait)
|
||||
for time.Now().Before(deadline) {
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
return false
|
||||
case <-time.After(400 * time.Millisecond):
|
||||
}
|
||||
if !p.cacheEmpty(ctx, store, remote.Name) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func (p *GitHubProvider) cacheEmpty(ctx context.Context, store provider.RemoteMetadataStore, name string) bool {
|
||||
rows, err := store.ListRPMMetadataEntries(ctx, name)
|
||||
if err != nil {
|
||||
// Treat a failed read as "not empty" so a transient DB error becomes a
|
||||
// normal serve attempt (which reports its own error) rather than a 503.
|
||||
return false
|
||||
}
|
||||
return len(rows) == 0
|
||||
}
|
||||
|
||||
// refresh brings the derived metadata up to date without coupling the scan to
|
||||
// the inbound request. When the cache is stale it single-flights a scan: if the
|
||||
// cache already holds rows the scan runs in the background and the caller serves
|
||||
@@ -212,15 +302,31 @@ func (p *GitHubProvider) runScan(remote models.Remote, store provider.RemoteMeta
|
||||
p.mu.Unlock()
|
||||
}
|
||||
|
||||
// scan runs a full unconditional derive. Retained for the legacy inline refresh
|
||||
// path and existing tests; the syncer uses scanWithState to pass and receive the
|
||||
// releases-list ETag.
|
||||
func (p *GitHubProvider) scan(ctx context.Context, remote models.Remote, store provider.RemoteMetadataStore) error {
|
||||
releases, err := p.fetchReleases(ctx, remote)
|
||||
_, _, err := p.scanWithState(ctx, remote, store, "")
|
||||
return err
|
||||
}
|
||||
|
||||
// scanWithState derives metadata incrementally. It sends the prior releases-list
|
||||
// ETag as a conditional request: a 304 means nothing changed, so it returns
|
||||
// (etag, changed=false) without a single asset fetch. On a 200 it diffs the
|
||||
// release assets against the cache, derives only new/changed assets, prunes
|
||||
// assets that disappeared, and returns the new ETag.
|
||||
func (p *GitHubProvider) scanWithState(ctx context.Context, remote models.Remote, store provider.RemoteMetadataStore, etag string) (newEtag string, changed bool, err error) {
|
||||
releases, newEtag, notModified, err := p.fetchReleases(ctx, remote, etag)
|
||||
if err != nil {
|
||||
return err
|
||||
return etag, false, err
|
||||
}
|
||||
if notModified {
|
||||
return etag, false, nil
|
||||
}
|
||||
|
||||
existing, err := store.ListRPMMetadataEntries(ctx, remote.Name)
|
||||
if err != nil {
|
||||
return err
|
||||
return newEtag, false, err
|
||||
}
|
||||
existingByPath := make(map[string]provider.RPMMetadata, len(existing))
|
||||
for _, m := range existing {
|
||||
@@ -229,7 +335,7 @@ func (p *GitHubProvider) scan(ctx context.Context, remote models.Remote, store p
|
||||
|
||||
allow, err := compilePatterns(remote.Patterns)
|
||||
if err != nil {
|
||||
return err
|
||||
return newEtag, false, err
|
||||
}
|
||||
|
||||
seen := map[string]bool{}
|
||||
@@ -277,7 +383,7 @@ func (p *GitHubProvider) scan(ctx context.Context, remote models.Remote, store p
|
||||
_ = store.DeleteRPMMetadata(ctx, remote.Name, fp)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
return newEtag, true, nil
|
||||
}
|
||||
|
||||
type ghRelease struct {
|
||||
@@ -293,32 +399,53 @@ type ghAsset struct {
|
||||
Digest string `json:"digest"`
|
||||
}
|
||||
|
||||
func (p *GitHubProvider) fetchReleases(ctx context.Context, remote models.Remote) ([]ghRelease, error) {
|
||||
// fetchReleases lists a repo's releases. It sends the prior ETag as
|
||||
// If-None-Match on page 1 (the newest releases, where a new one first appears):
|
||||
// a 304 there means the repo is unchanged, so it returns notModified without
|
||||
// paging further — GitHub does not count 304 conditional responses against the
|
||||
// rate limit, making an unchanged repo nearly free. On a 200 it captures the
|
||||
// page-1 ETag and pages through the rest normally. Every call waits on the
|
||||
// shared limiter first.
|
||||
func (p *GitHubProvider) fetchReleases(ctx context.Context, remote models.Remote, etag string) (all []ghRelease, newEtag string, notModified bool, err error) {
|
||||
base := strings.TrimRight(remote.BaseURL, "/") + "/releases"
|
||||
var all []ghRelease
|
||||
for page := 1; page <= p.pageCap; page++ {
|
||||
u := fmt.Sprintf("%s?per_page=100&page=%d", base, page)
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodGet, u, nil)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
return nil, "", false, err
|
||||
}
|
||||
copyHeaders(req, githubHeaders(remote, true))
|
||||
if page == 1 && etag != "" {
|
||||
req.Header.Set("If-None-Match", etag)
|
||||
}
|
||||
|
||||
if err := p.limiterWait(ctx); err != nil {
|
||||
return nil, "", false, err
|
||||
}
|
||||
resp, err := p.client.Do(req)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
return nil, "", false, err
|
||||
}
|
||||
if page == 1 && resp.StatusCode == http.StatusNotModified {
|
||||
io.Copy(io.Discard, resp.Body)
|
||||
resp.Body.Close()
|
||||
return nil, etag, true, nil
|
||||
}
|
||||
body, err := io.ReadAll(resp.Body)
|
||||
respEtag := resp.Header.Get("ETag")
|
||||
resp.Body.Close()
|
||||
if err != nil {
|
||||
return nil, err
|
||||
return nil, "", false, err
|
||||
}
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
return nil, fmt.Errorf("github releases API %s: status %d", u, resp.StatusCode)
|
||||
return nil, "", false, fmt.Errorf("github releases API %s: status %d", u, resp.StatusCode)
|
||||
}
|
||||
if page == 1 {
|
||||
newEtag = respEtag
|
||||
}
|
||||
var releases []ghRelease
|
||||
if err := json.Unmarshal(body, &releases); err != nil {
|
||||
return nil, fmt.Errorf("decode releases: %w", err)
|
||||
return nil, "", false, fmt.Errorf("decode releases: %w", err)
|
||||
}
|
||||
if len(releases) == 0 {
|
||||
break
|
||||
@@ -328,7 +455,7 @@ func (p *GitHubProvider) fetchReleases(ctx context.Context, remote models.Remote
|
||||
break
|
||||
}
|
||||
}
|
||||
return all, nil
|
||||
return all, newEtag, false, nil
|
||||
}
|
||||
|
||||
func (p *GitHubProvider) deriveAsset(ctx context.Context, remote models.Remote, asset ghAsset, fp string) (*provider.RPMMetadata, error) {
|
||||
@@ -447,6 +574,9 @@ func (p *GitHubProvider) rangeGet(ctx context.Context, remote models.Remote, dow
|
||||
copyHeaders(req, githubHeaders(remote, false))
|
||||
req.Header.Set("Range", fmt.Sprintf("bytes=0-%d", n-1))
|
||||
|
||||
if err := p.limiterWait(ctx); err != nil {
|
||||
return nil, false, err
|
||||
}
|
||||
resp, err := p.client.Do(req)
|
||||
if err != nil {
|
||||
return nil, false, err
|
||||
@@ -471,6 +601,9 @@ func (p *GitHubProvider) computeSHA256(ctx context.Context, remote models.Remote
|
||||
}
|
||||
copyHeaders(req, githubHeaders(remote, false))
|
||||
|
||||
if err := p.limiterWait(ctx); err != nil {
|
||||
return "", err
|
||||
}
|
||||
resp, err := p.client.Do(req)
|
||||
if err != nil {
|
||||
return "", err
|
||||
|
||||
Reference in New Issue
Block a user