Skip to content

source: memoize remote ref existence probes - #2700

Merged
berendt merged 1 commit into
mainfrom
fix-drift-ref-probe-memo
Sep 9, 2026
Merged

berendt merged 1 commit into
mainfrom
fix-drift-ref-probe-memo

Conversation

@ideaship

@ideaship ideaship commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

release-tox-drift (periodic-daily) has been aborting with rc=2 — a source
error, not a drift finding — since the 2026-09-08 drift expansion landed. The
run exhausts GitHub's unauthenticated core API budget (60/hr) and dies before it
reports anything, so the job's red says nothing about the state of the
repositories it was asked to check.

The cause is not request volume. source.ref_exists() had no memo, unlike every
neighbouring per-run cache on Config, and callers re-ask the same question:

probe count
ref? ansible_playbooks @ v0.20260721.0 70
16 other ref? probes (kolla, kolla_ansible × release refs) 1 each

86 requests per run, 17 distinct. osism_files() probes the playbooks_version
pin on every call — the guard at playbooks.py:224-229 that closes the
404-ambiguity hole — and runtime_interface() is memoized per release while
osism_interface()/osism_files() are release-independent, so every
per-release miss re-walks them.

Verification

Full --group all run on this branch, unauthenticated, the way the job runs
it:

  • 17 core-API requests instead of 86, each ref probed exactly once — GitHub's
    own counter reads used 17 of 60 afterwards
  • rc=1, with a report byte-identical to an authenticated run on main
  • 740 tests pass; flake8 + black clean

Two findings behind the diagnosis, both verified rather than assumed, since they
are what rule the file reads out:

  • Archive mode is the default (driver.py:289, archive=not args.use_raw_get),
    so a run's ~3600 raw/list/tree progress lines are local reads out of one
    extracted tarball per (repo, ref) — not requests.
  • api.github.com/repos/<o>/<r>/tarball/<ref> is accounted in a separate
    bucket
    : its 302 returns x-ratelimit-used: 0 and leaves remaining
    untouched, while a plain /repos/<o>/<r> GET decrements it.

So the ref probes are the only consumers of the 60/hr budget, and no token or
Zuul secret is needed — 17/60 leaves ample headroom as the release range grows.

The expansion that crossed the limit

Neither is at fault: they multiplied how many callers ask an already-answered
question.

Side effect worth knowing

The run this unmasks confirms the 2026.1 mirror re-sync is closed — the 230
kolla_mirror_verbatim findings are gone:

Summary: 5 to act on, 15 advisory, 60 allowlisted, 2 stale entries (80 total)

The two stale allowlist entries (neutron_ovn_vpn_agent_image/_tag, upstream
at master ahead of the supported range) are the pair predicted once the round
landed. They are a separate cleanup, not part of this PR — so expect the job to
go green on source errors and stay red on those two until they are removed.

Not addressed here

A future budget crossing would again abort rather than degrade the report.
Failing soft on a source error — the ProbeInconclusive pattern
kolla_source_ref_phase already establishes, lifted to the driver — is left to
its own change.

🤖 Generated with Claude Code

ref_exists() asked GitHub the same question once per caller. Nothing
memoized it, and osism_files() (playbooks.py) re-probes the
playbooks_version pin on every call: runtime_interface() is memoized per
release, but osism_interface() is release-independent and so gets
re-walked. One --group all run therefore spent 70 of its 86 requests
re-asking whether a single tag exists in osism/ansible-playbooks.

Those probes are the only part of a run that draws on GitHub's core API
budget. Archive reads -- the default -- serve every file, listing and
tree from one tarball per (repo, ref), and the tarball endpoint is
accounted in a separate bucket, so the thousands of reads a run reports
cost nothing here. 86 requests is over the 60/hr unauthenticated limit;
the periodic-daily release-tox-drift job crossed it on 2026-09-09 and
aborted with a source error before reporting any drift, which makes a
red gate that says nothing about the state of the repositories it was
asked to check.

Memoize the remote answer per (repo, ref) on Config, alongside the
existing per-run caches. Absence is cached too, so release_to_ref()'s
walk through its four candidates does not pay for its misses again on a
later resolve -- only its hit is memoized there. Local checkouts stay
unmemoized: they answer from the clone at no request cost and stay live
for a caller reading a working tree.

A full run now issues 17 requests instead of 86, each ref probed once,
and completes unauthenticated with a report identical to an
authenticated one.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Roger Luethi <luethi@osism.tech>
@ideaship
ideaship marked this pull request as ready for review September 9, 2026 12:39
@ideaship
ideaship requested a review from berendt September 9, 2026 12:40
@berendt
berendt merged commit d9c45b1 into main Sep 9, 2026
3 checks passed
@berendt
berendt deleted the fix-drift-ref-probe-memo branch September 9, 2026 12:48
@github-project-automation github-project-automation Bot moved this from New to Done in Human Board Sep 9, 2026
@ideaship ideaship self-assigned this Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants