perf(github): cache check-run lookups with retry

Cache GitHub check-run API responses to avoid repeated
paginated API calls during status updates. Use a mutex+channel
pattern so concurrent goroutines share a single in-flight fetch.
Failed fetches are not cached, allowing retries with exponential
backoff on transient API errors.

Signed-off-by: Akshay Pant <akpant@redhat.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>

Author: theakshaypant

pkg/provider/github/github.go

@@ -72,21 +72,31 @@ type Provider struct {
 	pacUserLogin       string // user/bot login used by PAC
 	clock              clockwork.Clock
 	graphQLClient      *graphQLClient
+	checkRunsCache     checkRunsCache
 }
 
 type skippedRun struct {
 	mutex      *sync.Mutex
 	checkRunID int64
 }
 
+type checkRunsCache struct {
+	mu        sync.Mutex
+	runs      []*github.CheckRun
+	loading   bool
+	done      chan struct{}
+	populated bool
+}
+
 func New() *Provider {
 	return &Provider{
 		APIURL:        github.Ptr(keys.PublicGithubAPIURL),
 		PaginedNumber: defaultPaginedNumber,
 		skippedRun: skippedRun{
 			mutex: &sync.Mutex{},
 		},
-		clock: clockwork.NewRealClock(),
+		clock:          clockwork.NewRealClock(),
+		checkRunsCache: checkRunsCache{},
 	}
 }
 

pkg/provider/github/status.go

@@ -23,8 +23,10 @@ import (
 )
 
 const (
-	botType         = "Bot"
-	pendingApproval = "Pending approval, waiting for an /ok-to-test"
+	botType                      = "Bot"
+	pendingApproval              = "Pending approval, waiting for an /ok-to-test"
+	checkRunsFetchMaxRetries     = 2
+	checkRunsFetchInitialBackoff = 200 * time.Millisecond
 )
 
 const taskStatusTemplate = `
@@ -42,7 +44,9 @@ const taskStatusTemplate = `
 {{- end }}
 </table>`
 
-func (v *Provider) getExistingCheckRunID(ctx context.Context, runevent *info.Event, status providerstatus.StatusOpts) (*int64, error) {
+// fetchAllCheckRunPages retrieves every page of check runs for the event SHA.
+func (v *Provider) fetchAllCheckRunPages(ctx context.Context, runevent *info.Event) ([]*github.CheckRun, error) {
+	var all []*github.CheckRun
 	opt := github.ListOptions{PerPage: v.PaginedNumber}
 	for {
 		res, resp, err := wrapAPI(v, "list_check_runs_for_ref", func() (*github.ListCheckRunsResults, *github.Response, error) {
@@ -55,25 +59,95 @@ func (v *Provider) getExistingCheckRunID(ctx context.Context, runevent *info.Eve
 		if err != nil {
 			return nil, err
 		}
-
-		for _, checkrun := range res.CheckRuns {
-			// if it is a Pending approval CheckRun then overwrite it
-			if isPendingApprovalCheckrun(checkrun) || isFailedCheckrun(checkrun) {
-				if v.canIUseCheckrunID(checkrun.ID) {
-					return checkrun.ID, nil
-				}
-			}
-			if *checkrun.ExternalID == status.PipelineRunName {
-				return checkrun.ID, nil
-			}
-		}
+		all = append(all, res.CheckRuns...)
 		if resp.NextPage == 0 {
 			break
 		}
 		opt.Page = resp.NextPage
 	}
+	return all, nil
+}
+
+func (v *Provider) searchCheckRuns(runs []*github.CheckRun, status providerstatus.StatusOpts) *int64 {
+	for _, checkrun := range runs {
+		if isPendingApprovalCheckrun(checkrun) || isFailedCheckrun(checkrun) {
+			if v.canIUseCheckrunID(checkrun.ID) {
+				return checkrun.ID
+			}
+		}
+		if checkrun.ExternalID != nil && *checkrun.ExternalID == status.PipelineRunName {
+			return checkrun.ID
+		}
+	}
+	return nil
+}
+
+func (v *Provider) getExistingCheckRunID(ctx context.Context, runevent *info.Event, status providerstatus.StatusOpts) (*int64, error) {
+	for {
+		v.checkRunsCache.mu.Lock()
+		if v.checkRunsCache.loading {
+			done := v.checkRunsCache.done
+			v.checkRunsCache.mu.Unlock()
+			<-done
+			continue
+		}
+		if v.checkRunsCache.populated {
+			runs := v.checkRunsCache.runs
+			v.checkRunsCache.mu.Unlock()
+			return v.searchCheckRuns(runs, status), nil
+		}
+
+		v.checkRunsCache.loading = true
+		v.checkRunsCache.done = make(chan struct{})
+		v.checkRunsCache.mu.Unlock()
+
+		runs, err := v.fetchAllCheckRunPagesWithRetry(ctx, runevent)
+
+		v.checkRunsCache.mu.Lock()
+		if err != nil {
+			v.checkRunsCache.loading = false
+		} else {
+			v.checkRunsCache.runs = runs
+			v.checkRunsCache.loading = false
+			v.checkRunsCache.populated = true
+		}
+		close(v.checkRunsCache.done)
+		v.checkRunsCache.mu.Unlock()
+
+		if err != nil {
+			return nil, err
+		}
+		return v.searchCheckRuns(runs, status), nil
+	}
+}
+
+func (v *Provider) fetchAllCheckRunPagesWithRetry(ctx context.Context, runevent *info.Event) ([]*github.CheckRun, error) {
+	var lastErr error
+	for attempt := range checkRunsFetchMaxRetries + 1 {
+		runs, err := v.fetchAllCheckRunPages(ctx, runevent)
+		if err == nil {
+			return runs, nil
+		}
+		lastErr = err
+
+		if attempt == checkRunsFetchMaxRetries {
+			return nil, err
+		}
+
+		backoff := time.Duration(1<<uint(attempt)) * checkRunsFetchInitialBackoff
+		if v.Logger != nil {
+			v.Logger.Debugf("check-runs lookup failed for %s/%s@%s (attempt %d/%d): %v; retrying in %v",
+				runevent.Organization, runevent.Repository, runevent.SHA,
+				attempt+1, checkRunsFetchMaxRetries+1, err, backoff)
+		}
 
-	return nil, nil
+		select {
+		case <-ctx.Done():
+			return nil, ctx.Err()
+		case <-v.getClock().After(backoff):
+		}
+	}
+	return nil, lastErr
 }
 
 func isPendingApprovalCheckrun(run *github.CheckRun) bool {

pkg/provider/github/status_test.go

@@ -8,6 +8,8 @@ import (
 	"reflect"
 	"strconv"
 	"strings"
+	"sync"
+	"sync/atomic"
 	"testing"
 
 	"github.com/google/go-github/v84/github"
@@ -686,3 +688,88 @@ func TestProviderGetExistingCheckRunID(t *testing.T) {
 		})
 	}
 }
+
+func TestGetExistingCheckRunIDCache(t *testing.T) {
+	tests := []struct {
+		name            string
+		jsonret         string
+		failFirstN      int
+		goroutines      int
+		secondLookup    string
+		expectedID      int64
+		expectedAPIHits int64
+	}{
+		{
+			name:            "second call serves from cache",
+			jsonret:         `{"total_count": 2, "check_runs": [{"id": 55555, "external_id": "mypr"}, {"id": 55556, "external_id": "mypr2"}]}`,
+			secondLookup:    "mypr2",
+			expectedID:      55555,
+			expectedAPIHits: 1,
+		},
+		{
+			name:            "concurrent calls share single fetch",
+			jsonret:         `{"total_count": 2, "check_runs": [{"id": 55555, "external_id": "mypr"}, {"id": 55556, "external_id": "mypr2"}]}`,
+			goroutines:      10,
+			expectedAPIHits: 1,
+		},
+		{
+			name:            "retries on transient error",
+			jsonret:         `{"total_count": 1, "check_runs": [{"id": 77777, "external_id": "mypr"}]}`,
+			failFirstN:      1,
+			expectedID:      77777,
+			expectedAPIHits: 2,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx, _ := rtesting.SetupFakeContext(t)
+			client, mux, _, teardown := ghtesthelper.SetupGH()
+			defer teardown()
+
+			event := &info.Event{
+				Organization: "owner",
+				Repository:   "repository",
+				SHA:          "sha",
+			}
+
+			var apiHits atomic.Int64
+			mux.HandleFunc(fmt.Sprintf("/repos/%v/%v/commits/%v/check-runs", event.Organization, event.Repository, event.SHA), func(w http.ResponseWriter, _ *http.Request) {
+				hit := apiHits.Add(1)
+				if int(hit) <= tt.failFirstN {
+					w.WriteHeader(http.StatusBadGateway)
+					return
+				}
+				fmt.Fprint(w, tt.jsonret)
+			})
+
+			cnx := New()
+			cnx.SetGithubClient(client)
+
+			if tt.goroutines > 1 {
+				var wg sync.WaitGroup
+				wg.Add(tt.goroutines)
+				for range tt.goroutines {
+					go func() {
+						defer wg.Done()
+						_, _ = cnx.getExistingCheckRunID(ctx, event, providerstatus.StatusOpts{PipelineRunName: "mypr"})
+					}()
+				}
+				wg.Wait()
+			} else {
+				id, err := cnx.getExistingCheckRunID(ctx, event, providerstatus.StatusOpts{PipelineRunName: "mypr"})
+				assert.NilError(t, err)
+				if tt.expectedID != 0 {
+					assert.Assert(t, id != nil)
+					assert.Equal(t, *id, tt.expectedID)
+				}
+				if tt.secondLookup != "" {
+					id2, err := cnx.getExistingCheckRunID(ctx, event, providerstatus.StatusOpts{PipelineRunName: tt.secondLookup})
+					assert.NilError(t, err)
+					assert.Assert(t, id2 != nil)
+				}
+			}
+
+			assert.Equal(t, apiHits.Load(), tt.expectedAPIHits)
+		})
+	}
+}