jenkinsci / jenkinsci/github-branch-source-plugin

ETag cache not reused across folder scans — conditional requests never sent for pull-request listing

Open
#1,547 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.