Skip to content

Remove repository config caching logic - #410

Draft
alecbcs wants to merge 4 commits into
llnl:mainfrom
alecbcs:remove/repository-config-cache
Draft

Remove repository config caching logic#410
alecbcs wants to merge 4 commits into
llnl:mainfrom
alecbcs:remove/repository-config-cache

Conversation

@alecbcs

@alecbcs alecbcs commented Aug 19, 2026

Copy link
Copy Markdown
Member

For a long time, the one sticking point when thinking about how to scale Hubcast to multiple Kubernetes replicas had been the repository config cache. Caching repository configs seemed like an obvious win. But with multiple replicas, only one replica would receive the config update event and the others could keep syncing events against a stale config for up to 30 minutes.

Ultimately the simpler answer is that the cache isn't worth its complexity. GitHub grants apps 5000 requests/hr per installation and we're no where near that limit. Additionally we already hit GitHub on every event either by responding to a comment or by fetching a packfile so removing the cache doesn't introduce another point of failure.

As such this PR proposes the kinda radical idea to remove the repository config cache entirely. Instead every event reads the current config so all replica pods are always consistent.

If we ever get close to the 5,000 req/hr/installation limit we could swap to using conditional requests. GitHub returns an ETag header on API responses; sending it back via If-None-Match yields a 304 "Not Modified" when the content hasn't changed. Also 304 responses don't count against the rate limit. Since repo configs change rarely, nearly all config fetches would be 304 responses and wouldn't count against our limit.

Signed-off-by: Alec Scott <alec@llnl.gov>
@alecbcs
alecbcs requested a review from cmelone August 19, 2026 22:15
cmelone
cmelone previously approved these changes Aug 19, 2026

@cmelone cmelone left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couple nits but LGTM

Comment on lines +30 to +32
Fetched fresh on every event so all replicas always see the current
destination repo; a single contents-API call is well within App
installation rate limits.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be simplified, not sure if the last clause is needed in the codebase

Comment on lines +48 to +49
# we don't want to raise this as a RepoConfigError because telling users
# about the absence of the config will create noise and confusion

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-- because they may have installed the app but have not submitted a config file yet

@github-actions github-actions Bot added ci Involving Project CI & Unit Tests dependencies Modifications to a Dependency File labels Aug 19, 2026
Signed-off-by: Alec Scott <alec@llnl.gov>
@alecbcs
alecbcs force-pushed the remove/repository-config-cache branch from 31f8cb3 to f0d7ae0 Compare August 20, 2026 20:37
@alecbcs
alecbcs force-pushed the remove/repository-config-cache branch from 85b8c9c to 7f98e31 Compare August 21, 2026 02:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Involving Project CI & Unit Tests dependencies Modifications to a Dependency File

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants