Skip to content

Commit 991e3f8

Browse files
committed
Add delay between pull request updates
In projects with many open PRs using the automatic updates feature, Bulldozer can trigger GitHub's secondary rate limits by updating (and possibly querying) pull requests too quickly. To avoid this, add some delay between operations. For queries, use a fixed 250ms delay for now. This shouldn't slow things down too much, but does prevent Bulldozer from blasting through PRs as fast as possible. For updates, use an exponential delay that starts at 1s and maxes out at 60s. I believe the secondary limits (at least for GitHub Enterprise) are computed over a minute, so this will limit us to one update per minute for large batches. I don't think this will be prohibitively slow, but we could explore making this configurable in the future. As part of this, refactor the logic so that we don't block queries on previous updates.
1 parent d4e4302 commit 991e3f8

1 file changed

Lines changed: 60 additions & 4 deletions

File tree

server/handler/push.go

Lines changed: 60 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,23 @@ package handler
1717
import (
1818
"context"
1919
"encoding/json"
20+
"time"
2021

2122
"github.com/google/go-github/v41/github"
23+
"github.com/palantir/bulldozer/bulldozer"
2224
"github.com/palantir/bulldozer/pull"
2325
"github.com/palantir/go-githubapp/githubapp"
2426
"github.com/pkg/errors"
2527
)
2628

29+
const (
30+
PullQueryDelay = 250 * time.Millisecond
31+
32+
PullUpdateBaseDelay = 1 * time.Second
33+
PullUpdateMaxDelay = 60 * time.Second
34+
PullUpdateDelayMult = 1.5
35+
)
36+
2737
type Push struct {
2838
Base
2939
}
@@ -75,18 +85,64 @@ func (h *Push) Handle(ctx context.Context, eventType, deliveryID string, payload
7585
return err
7686
}
7787

78-
for _, pr := range prs {
88+
if config == nil {
89+
logger.Debug().Msg("Skipping pull request updates to missing configuration")
90+
return nil
91+
}
92+
93+
var toUpdate []updateCtx
94+
for i, pr := range prs {
7995
logger := logger.With().Int(githubapp.LogKeyPRNum, pr.GetNumber()).Logger()
80-
logger.Debug().Msgf("Considering pull request for update")
96+
logger.Debug().Msg("Checking if pull request should update")
8197

98+
ctx := logger.WithContext(ctx)
8299
pullCtx := pull.NewGithubContext(client, pr)
83-
if _, err := h.UpdatePullRequest(logger.WithContext(ctx), pullCtx, client, config, pr, baseRef); err != nil {
84-
logger.Error().Err(errors.WithStack(err)).Msg("Error updating pull request")
100+
101+
shouldUpdate, err := bulldozer.ShouldUpdatePR(ctx, pullCtx, config.Update)
102+
if err != nil {
103+
logger.Error().Err(err).Msg("Error determining if pull request should update, skipping")
104+
continue
105+
}
106+
if shouldUpdate {
107+
toUpdate = append(toUpdate, updateCtx{
108+
ctx: ctx,
109+
pullCtx: pullCtx,
110+
})
111+
}
112+
113+
if i < len(prs)-1 {
114+
time.Sleep(delay(i, PullQueryDelay, 1, PullQueryDelay))
115+
}
116+
}
117+
118+
logger.Info().Msgf("Found %d pull requests that need updates", len(toUpdate))
119+
for i, pr := range toUpdate {
120+
bulldozer.UpdatePR(pr.ctx, pr.pullCtx, client, config.Update, baseRef)
121+
if i < len(toUpdate)-1 {
122+
d := delay(i, PullUpdateBaseDelay, PullUpdateDelayMult, PullUpdateMaxDelay)
123+
logger.Debug().Msgf("Waiting %v until next update to avoid GitHub rate limits", d)
124+
time.Sleep(d)
85125
}
86126
}
87127

88128
return nil
89129
}
90130

131+
type updateCtx struct {
132+
ctx context.Context
133+
pullCtx pull.Context
134+
}
135+
136+
func delay(iter int, base time.Duration, mult float64, max time.Duration) time.Duration {
137+
t := base
138+
for ; iter > 0; iter-- {
139+
t = time.Duration(mult * float64(t))
140+
if t > max {
141+
return max
142+
}
143+
}
144+
return t
145+
}
146+
91147
// type assertion
92148
var _ githubapp.EventHandler = &Push{}

0 commit comments

Comments
 (0)