Skip to content

feat: round-robin repo-enumeration checkpoint + secondary rate-limit throttle#3

Merged
sebastien-boulle merged 1 commit into
vendor-upstream-sourcefrom
round-robin-checkpoint
Jul 17, 2026
Merged

feat: round-robin repo-enumeration checkpoint + secondary rate-limit throttle#3
sebastien-boulle merged 1 commit into
vendor-upstream-sourcefrom
round-robin-checkpoint

Conversation

@ttomalak-bkpr

Copy link
Copy Markdown
Contributor

Summary

This is the actual motivation for forking tap-github in the first place (see lumapps/github-automation#2571).

Root cause: RepositoryStream (the organizations/searches config modes) re-walks its full repo list in the same endpoint-imposed order every single run — sorted by updated_at, a field the since param can't actually filter on for either the org-listing or search endpoints. So every run restarts at the same front-of-queue repos, and once heavy child streams (pull_request_commits, reviews — both full_table, no replication_key at all) exhaust GitHub's secondary rate limit, the tap never advances into the rest of the catalog. Most of an org's repos have never had PR/commit data synced, at all, despite running on schedule for months.

Changes

  • Round-robin checkpoint (RepositoryStream._get_records_round_robin): resumes right after the last repo whose children fully synced last run, wraps at the end. Persisted under a dedicated round_robin_checkpoint state key — deliberately not nested under progress_markers, since singer_sdk wipes that key entirely on every partition finalize.
  • Traversal order sorted by immutable repo id, not the API's updated_at order. Verified via simulation that using the volatile updated_at order (even though never incorrect — no repo is ever permanently skipped) costs ~2.8x more runs to guarantee full coverage under continuous repo activity, because reshuffling keeps pushing already-synced repos back in front of genuinely new ones. Sorting by id removes that churn — simulation showed coverage time drop to within ~1% of the theoretical minimum.
  • End-of-run summary log: one line per partition (repos processed, elapsed time, full lap or not, next resume point). Fires even on a mid-run crash — verified that when child-stream syncing throws and unwinds singer_sdk's outer sync loop, the paused get_records() generator is torn down (GeneratorExit), which still runs our finally block.
  • Adaptive secondary rate-limit throttle (GitHubRestStream): a shared, tap-wide inter-request delay that escalates (doubles, capped at 30s) on each secondary-limit hit and applies to every subsequent request from any stream — not just the one retried call, which is all the existing sleep-and-retry did. Decays back down after 25 consecutive clean requests.

Verification

  • Round-robin + id-sort: live-GitHub test (fresh run + hand-crafted mid-cycle-crash resume, both directions), plus logic simulations for the reshuffling-robustness and adversarial-starvation properties (no repo ever permanently skipped, even under worst-case continuous churn).
  • Summary logging: confirmed the crash path specifically by closing the generator mid-iteration in a standalone script and observing the finally block still fire with the correct partial count.
  • Throttle: fake-clock simulation of escalation/decay (network calls to trigger GitHub's real secondary limit deliberately would be inappropriate to test against live infra); confirmed zero overhead on a normal run with no hits.

Next steps

Once merged, lumapps/dataplatform's bi/meltano/meltano.yml tap-github pip_url swaps from MeltanoLabs/tap-github to this fork.

@ttomalak-bkpr
ttomalak-bkpr requested review from a team and lumautomation as code owners July 17, 2026 10:54
…throttle

The upstream RepositoryStream re-walks its full repo list in the same
endpoint-imposed order every run (by updated_at, which the "since"
param can't actually filter on either the org-listing or search
endpoints), so it perpetually re-verifies the same front-of-queue
repos and never advances into the rest of the catalog before hitting
GitHub's secondary rate limit. Most of an org's repos never get
PR/commit data synced at all, despite running on schedule for months.

- RepositoryStream now round-robins through organizations/searches
  mode partitions: resumes right after the last repo whose children
  fully synced last run (tracked via a `round_robin_checkpoint` state
  key, deliberately not nested under `progress_markers`, which
  singer_sdk wipes on every partition finalize), wrapping at the end.
  Traversal order is sorted by immutable repo `id`, not the API's
  volatile `updated_at` order, so reshuffling from ordinary repo
  activity doesn't cause redundant re-syncs of already-current repos
  before genuinely new ones are reached.
- Logs a one-line round-robin summary at the end of each partition's
  enumeration (repos processed, elapsed time, full lap or not, next
  resume point) - fires even on a mid-run crash via a `finally` block,
  since a crash from child-stream syncing tears down the paused
  generator (GeneratorExit) rather than skipping straight to process
  exit.
- Adds an adaptive, tap-wide throttle for the secondary (frequency)
  rate limit: escalates a shared inter-request delay on each hit
  (capped), applied to every subsequent request from any stream, and
  decays it back down after a run of clean requests. Previously a
  secondary-limit hit only paced the one retried request while
  everything else kept firing at the same rate that triggered it.

Verified against live GitHub data (fresh + simulated mid-cycle-crash
resume) and via fake-clock/logic simulations for the throttle and
reshuffling-robustness properties - see PR description for details.
@ttomalak-bkpr
ttomalak-bkpr force-pushed the round-robin-checkpoint branch from 8cc65b9 to d7168c2 Compare July 17, 2026 11:16
@sebastien-boulle
sebastien-boulle merged commit fdfd086 into vendor-upstream-source Jul 17, 2026
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