jenkinsci / jenkinsci/github-branch-source-plugin
ETag cache not reused across folder scans — conditional requests never sent for pull-request listing
- Dominant language
- Java
- Stars
- 217
- Forks
- 398
- Avg merge
- 30m
- Merged PRs (30d)
- 1
Description
## Summary
On each Multibranch/Organization Folder scan, the plugin lists open PRs via `GET /repos/{owner}/{repo}/pulls?state=open`. The `github-api` client is configured with an OkHttp cache that supports conditional requests (`If-None-Match`/`ETag` → `304`), but the cache is not reused between scans — so nearly every scan sends an unconditional request and gets a full `200` even when the list is unchanged.
Cause: a pooled connection (and its OkHttp `Cache`) is discarded after 30 min idle, while scans run less often (hourly by default). The next scan can't reuse the previous scan's cache.
## Details (`master` @ `fa27ed96`)
`UnusedConnectionDestroyer` evicts pooled connections idle > 30 min ([L619-634](https://github.com/jenkinsci/github-branch-source-plugin/blob/fa27ed9618538f98b8532c9c1752a7e6e6c959b1/src/main/java/org/jenkinsci/plugins/github_branch_source/Connector.java#L619-L634)). On eviction, `removeAllUnused()` handles the `Cache` by credential type ([L694-715](https://github.com/jenkinsci/github-branch-source-plugin/blob/fa27ed9618538f98b8532c9c1752a7e6e6c959b1/src/main/java/org/jenkinsci/plugins/github_branch_source/Connector.java#L694-L715)):
1. **GitHub App creds:** `cleanupCacheFolder=true` ([L423](https://github.com/jenkinsci/github-branch-source-plugin/blob/fa27ed9618538f98b8532c9c1752a7e6e6c959b1/src/main/java/org/jenkinsci/plugins/github_branch_source/Connector.java#L423)) → eviction runs `cache.delete()` ([L700-702](https://github.com/jenkinsci/github-branch-source-plugin/blob/fa27ed9618538f98b8532c9c1752a7e6e6c959b1/src/main/java/org/jenkinsci/plugins/github_branch_source/Connector.java#L700-L702)), so the next scan starts from an empty directory.
2. **PAT / other creds:** `cleanupCacheFolder=false`, so that branch is skipped and the `Cache` is neither deleted nor `close()`d — it's dropped still-open. The next scan opens a new `Cache` on the same directory; it's unclear whether entries from the prior (never-closed) `Cache` are reliably reused here, and conditional requests do not appear to be sent in this path. (Worth confirming with a reproduction.)
The cache is also disabled by default on Windows ([`cacheSize = isWindows() ? 0 : 20`, L168-170](https://github.com/jenkinsci/github-branch-source-plugin/blob/fa27ed9618538f98b8532c9c1752a7e6e6c959b1/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource.java#L168-L170)).
## Why it matters
GitHub doesn't count `304` responses against the REST rate limit ([docs](https://docs.github.com/en/rest/overview/resources-in-the-rest-api#conditional-requests)). Reusing the cache means unchanged PR lists return `304` — no quota spent, no body transferred. For controllers scanning many repos on a short interval, that lowers rate-limit consumption (and the back-off it triggers) and cuts redundant transfer/parsing.
Benefit scales with how often listings are unchanged; changed listings still return `200` as today. This restores existing-but-inert caching and doesn't change scan results.
## Proposed fix
**1. Preserve the cache between scans.** On eviction, always `close()` the `Cache` (flush + release) but never `delete()`, for all credential types. It's already `cacheSize`-bounded (LRU) and keyed by a stable per-credential hash, so keeping it on disk is bounded/safe:
```diff
- if (record.cache != null && record.cleanupCacheFolder) {
- record.cache.delete();
- record.cache.close();
- }
+ if (record.cache != null) {
+ record.cache.close();
+ }
```
The next scan can then reopen a warm cache and send `If-None-Match`. The `cleanupCacheFolder` flag can be removed.
**2. (Optional) Enable the cache on Windows** — default `cacheSize` to `20` instead of `isWindows() ? 0 : 20`.
## Validation
Regression test: prime the cache, force eviction, re-request, assert the cache is reused / `If-None-Match` is sent — for both GitHub App and PAT credentials.
Contributor guide
Research direction
Start in src/main/java/org/jenkinsci/plugins/github_branch_source/Connector.java, especially UnusedConnectionDestroyer and removeAllUnused(), then inspect cacheSize in GitHubSCMSource.java. Reproduce eviction and a later pull-request listing for GitHub App and PAT credentials. Done means the cache survives eviction, reopens with prior entries, and sends If-None-Match; also assess the Windows default if included.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- github, java
- Domain
- api, backend, performance
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 67/100