You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Part of #4496. P1 — highest priority, biggest impact.
Context
The cron enqueues a top-level backfill-registered-repos job every 30 minutes (src/index.ts:166), which fans out one per-repo backfill-registered-repos message per registered repo (src/queue/processors.ts:994-1020). When that per-repo message is processed, since repoFullName is set and requestedBy is "schedule" (never "test" in production), it routes to enqueueRepositoryOpenDataBackfill (src/queue/processors.ts:1021-1029) — not to backfillRegisteredRepositories.
enqueueRepositoryOpenDataBackfill (src/github/backfill.ts:451-499) has no check at all against the repo's last-sync recency or error state: it unconditionally reads the totals, stamps status: "running", and enqueues all 4 segment jobs (labels, open_issues, open_pull_requests, recent_merged_pull_requests) every single time it's invoked.
The intended 6-hour "fresh success" skip and 1-hour "recent error" backoff (FRESH_SYNC_MS / ERROR_BACKOFF_MS, backfill.ts:321-322, 425-444) only live inside backfillRegisteredRepositories's own inline crawl (backfillRepository), which in production is only reached via the synchronous /v1/internal/jobs/backfill-registered-repos/run admin endpoint (src/api/routes.ts:3542-3547) or requestedBy === "test" — never via the actual scheduled cron path. src/index.ts:113's own comment asserts a 6-hour cadence ("...than up to 6 hours (backfillRegisteredRepositories's freshness window)") that the scheduled path does not actually get — the comment is describing dead code, not the real behavior.
Practical consequence: a repo whose backfill keeps ending in status: "error" (bad token, repo access revoked, transient GitHub outage) gets a fresh totals-fetch + 4 new segment-sync attempts every 30 minutes indefinitely instead of backing off for the intended 1 hour. Even a healthy repo gets a full re-scan attempt every 30 min instead of the intended 6-hour cadence — mitigated only by the segment-level ETag/Last-Modified 304 fast-path for page-1-unchanged segments, which does not eliminate the per-cycle totals/DB-write/queue-message overhead and does nothing for the error-backoff case.
This is the same class of bug as the sweep-endless-reregate incident fixed tonight (#4474): a scheduled path missing the "already handled recently, back off" check its own sibling function has.
Requirements
Port backfillRegisteredRepositories's syncState freshness/error-backoff check (backfill.ts:424-445) into enqueueRepositoryOpenDataBackfill, or extract it into a shared helper both call.
Preserve the force override path (an explicit force-refresh must still bypass the freshness/backoff check, matching backfillRegisteredRepositories's own contract).
Confirm the fix doesn't regress the admin endpoint's own semantics (/v1/internal/jobs/backfill-registered-repos/run), which should presumably keep its own explicit-force behavior.
Invariant + regression tests (non-negotiable): a test asserting that a repo with a lastCompletedAt inside the fresh-success window is SKIPPED by the scheduled path (not just the admin-endpoint path); a test asserting a repo in status: "error" inside the 1-hour backoff window is SKIPPED; a regression test reproducing the exact bug (repo backfilled twice in a row within the fresh window via the scheduled dispatch path, asserting only ONE segment-fetch cycle fires); and confirm the existing force: false test at test/unit/queue.test.ts:566-588 still passes once fixed (it currently passes only because it doesn't seed prior sync state — update or add a companion test that DOES seed recent state to prove the new skip actually engages).
Deliverables
enqueueRepositoryOpenDataBackfill (or a shared helper) respects FRESH_SYNC_MS/ERROR_BACKOFF_MS on the scheduled path, matching backfillRegisteredRepositories's existing behavior
Regression test reproducing the exact incident shape (repeated scheduled dispatch within the freshness window doing real work each time)
src/index.ts:113's comment updated to describe the ACTUAL cadence once fixed
Expected outcome
Every registered repo's scheduled backfill respects the same freshness/error-backoff discipline the admin-endpoint path already has, eliminating unnecessary GitHub API load and DB writes on both healthy repos (currently re-scanned every 30 min instead of 6h) and errored repos (currently retried every 30 min instead of backing off for 1h).
References
src/queue/processors.ts:994-1029 (the routing branch)
src/github/backfill.ts:424-499 (both the correct freshness-check and the missing-it path, side by side)
src/index.ts:113,166 (the cron enqueue + its stale comment)
test/unit/queue.test.ts:566-588 (existing test with no freshness-skip coverage)
Part of #4496. P1 — highest priority, biggest impact.
Context
The cron enqueues a top-level
backfill-registered-reposjob every 30 minutes (src/index.ts:166), which fans out one per-repobackfill-registered-reposmessage per registered repo (src/queue/processors.ts:994-1020). When that per-repo message is processed, sincerepoFullNameis set andrequestedByis"schedule"(never"test"in production), it routes toenqueueRepositoryOpenDataBackfill(src/queue/processors.ts:1021-1029) — not tobackfillRegisteredRepositories.enqueueRepositoryOpenDataBackfill(src/github/backfill.ts:451-499) has no check at all against the repo's last-sync recency or error state: it unconditionally reads the totals, stampsstatus: "running", and enqueues all 4 segment jobs (labels, open_issues, open_pull_requests, recent_merged_pull_requests) every single time it's invoked.The intended 6-hour "fresh success" skip and 1-hour "recent error" backoff (
FRESH_SYNC_MS/ERROR_BACKOFF_MS,backfill.ts:321-322, 425-444) only live insidebackfillRegisteredRepositories's own inline crawl (backfillRepository), which in production is only reached via the synchronous/v1/internal/jobs/backfill-registered-repos/runadmin endpoint (src/api/routes.ts:3542-3547) orrequestedBy === "test"— never via the actual scheduled cron path.src/index.ts:113's own comment asserts a 6-hour cadence ("...than up to 6 hours (backfillRegisteredRepositories's freshness window)") that the scheduled path does not actually get — the comment is describing dead code, not the real behavior.Practical consequence: a repo whose backfill keeps ending in
status: "error"(bad token, repo access revoked, transient GitHub outage) gets a fresh totals-fetch + 4 new segment-sync attempts every 30 minutes indefinitely instead of backing off for the intended 1 hour. Even a healthy repo gets a full re-scan attempt every 30 min instead of the intended 6-hour cadence — mitigated only by the segment-level ETag/Last-Modified 304 fast-path for page-1-unchanged segments, which does not eliminate the per-cycle totals/DB-write/queue-message overhead and does nothing for the error-backoff case.This is the same class of bug as the sweep-endless-reregate incident fixed tonight (#4474): a scheduled path missing the "already handled recently, back off" check its own sibling function has.
Requirements
backfillRegisteredRepositories's syncState freshness/error-backoff check (backfill.ts:424-445) intoenqueueRepositoryOpenDataBackfill, or extract it into a shared helper both call.forceoverride path (an explicit force-refresh must still bypass the freshness/backoff check, matchingbackfillRegisteredRepositories's own contract)./v1/internal/jobs/backfill-registered-repos/run), which should presumably keep its own explicit-force behavior.lastCompletedAtinside the fresh-success window is SKIPPED by the scheduled path (not just the admin-endpoint path); a test asserting a repo instatus: "error"inside the 1-hour backoff window is SKIPPED; a regression test reproducing the exact bug (repo backfilled twice in a row within the fresh window via the scheduled dispatch path, asserting only ONE segment-fetch cycle fires); and confirm the existingforce: falsetest attest/unit/queue.test.ts:566-588still passes once fixed (it currently passes only because it doesn't seed prior sync state — update or add a companion test that DOES seed recent state to prove the new skip actually engages).Deliverables
enqueueRepositoryOpenDataBackfill(or a shared helper) respectsFRESH_SYNC_MS/ERROR_BACKOFF_MSon the scheduled path, matchingbackfillRegisteredRepositories's existing behaviorforceoverride still works end-to-endsrc/index.ts:113's comment updated to describe the ACTUAL cadence once fixedExpected outcome
Every registered repo's scheduled backfill respects the same freshness/error-backoff discipline the admin-endpoint path already has, eliminating unnecessary GitHub API load and DB writes on both healthy repos (currently re-scanned every 30 min instead of 6h) and errored repos (currently retried every 30 min instead of backing off for 1h).
References
src/queue/processors.ts:994-1029(the routing branch)src/github/backfill.ts:424-499(both the correct freshness-check and the missing-it path, side by side)src/index.ts:113,166(the cron enqueue + its stale comment)test/unit/queue.test.ts:566-588(existing test with no freshness-skip coverage)Effort
M