Skip to content

Commit

Permalink
repo sync: Use ChangesAreMerged to batch check submitted branches (#400)
Browse files Browse the repository at this point in the history
Make use of the ChangesAreMerged to make a single network request
for all submitted branches, rather than one per branch.

We still need to make one request per tracked branch for now.
  • Loading branch information
abhinav authored Sep 14, 2024
1 parent e2f3b24 commit 0f9d7e7
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 49 deletions.
3 changes: 3 additions & 0 deletions .changes/unreleased/Changed-20240913-185002.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
kind: Changed
body: 'repo sync: Reduce the number of network requests made to check status of submitted branches.'
time: 2024-09-13T18:50:02.036293-07:00
97 changes: 48 additions & 49 deletions repo_sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,49 +243,64 @@ func (cmd *repoSyncCmd) deleteMergedBranches(
// 2. Branches that the user submitted PRs for manually
// with 'gh pr create' or similar.
//
// For the first, we can perform a cheaper API call to check the CR status.
// For the first, we can perform a cheap API call to check the CR status.
// For the second, we need to find recently merged PRs with that branch
// name, and match the remote head SHA to the branch head SHA.
//
// We'll try to do these checks concurrently.

submittedch := make(chan *submittedBranch)
trachedch := make(chan *trackedBranch)
var (
submittedBranches []*submittedBranch
trackedBranches []*trackedBranch
)
for _, b := range knownBranches {
if b.Change != nil {
b := &submittedBranch{
Name: b.Name,
Change: b.Change.ChangeID(),
}
submittedBranches = append(submittedBranches, b)
} else {
// TODO:
// Filter down to only branches that have
// a remote tracking branch:
// either $remote/$UpstreamBranch or $remote/$branch exists.
b := &trackedBranch{Name: b.Name}
trackedBranches = append(trackedBranches, b)
}
}

var wg sync.WaitGroup
for range min(runtime.GOMAXPROCS(0), len(knownBranches)) {
if len(submittedBranches) > 0 {
wg.Add(1)
go func() {
defer wg.Done()

// We'll nil these out once they're closed.
submittedch := submittedch
trachedch := trachedch

for submittedch != nil || trachedch != nil {
select {
case b, ok := <-submittedch:
if !ok {
submittedch = nil
continue
}
changeIDs := make([]forge.ChangeID, len(submittedBranches))
for i, b := range submittedBranches {
changeIDs[i] = b.Change
}

// TODO: Once we're recording GraphQL IDs in the store,
// we can combine all submitted PRs into one query.
merged, err := remoteRepo.ChangesAreMerged(ctx, []forge.ChangeID{b.Change})
if err != nil {
log.Error("Failed to query CR status", "change", b.Change, "error", err)
continue
}
merged, err := remoteRepo.ChangesAreMerged(ctx, changeIDs)
if err != nil {
log.Error("Failed to query CR status", "error", err)
return
}

b.Merged = merged[0]
for i, m := range merged {
submittedBranches[i].Merged = m
}
}()
}

case b, ok := <-trachedch:
if !ok {
trachedch = nil
continue
}
if len(trackedBranches) > 0 {
trackedch := make(chan *trackedBranch)
for range min(runtime.GOMAXPROCS(0), len(trackedBranches)) {
wg.Add(1)
go func() {
defer wg.Done()

for b := range trackedch {
changes, err := remoteRepo.FindChangesByBranch(ctx, b.Name, forge.FindChangesOptions{
Limit: 3,
State: forge.ChangeMerged,
Expand Down Expand Up @@ -313,30 +328,14 @@ func (cmd *repoSyncCmd) deleteMergedBranches(
}

}
}
}()
}
}()
}

var (
submittedBranches []*submittedBranch
trackedBranches []*trackedBranch
)
for _, b := range knownBranches {
if b.Change != nil {
b := &submittedBranch{
Name: b.Name,
Change: b.Change.ChangeID(),
}
submittedBranches = append(submittedBranches, b)
submittedch <- b
} else {
b := &trackedBranch{Name: b.Name}
trackedBranches = append(trackedBranches, b)
trachedch <- b
for _, b := range trackedBranches {
trackedch <- b
}
close(trackedch)
}
close(submittedch)
close(trachedch)
wg.Wait()

var branchesToDelete []string
Expand Down

0 comments on commit 0f9d7e7

Please sign in to comment.