Skip to content

Commit

Permalink
Optimize HTTP client used for GitHub (#689)
Browse files Browse the repository at this point in the history
* Optimize HTTP client used for GitHub

Use a single instance with tweaked defaults across all the invocations. Plus use newer API of `go-github` (including google/go-github#3011 fix).

Previously a default client was used which doesn't have nor request timeout nor idle connection timeout. This resulted in poor `p99` performance since sometimes connections were hanging blocking others to access `github.com`.

* Renamed to `defaultGitHubClient`
  • Loading branch information
fkorotkov committed Feb 16, 2024
1 parent d7e2a52 commit 3a71e8f
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 15 deletions.
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ require (
github.com/go-git/go-git/v5 v5.6.1
github.com/go-test/deep v1.1.0
github.com/golang/protobuf v1.5.3
github.com/google/go-github/v53 v53.0.0
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
github.com/google/uuid v1.3.1
github.com/grpc-ecosystem/go-grpc-middleware v1.4.0
Expand Down Expand Up @@ -58,7 +57,7 @@ require (
github.com/yudai/pp v2.0.1+incompatible // indirect
go.starlark.net v0.0.0-20230302034142-4b1e35fe2254
golang.org/x/crypto v0.18.0
golang.org/x/oauth2 v0.13.0
golang.org/x/oauth2 v0.13.0 // indirect
golang.org/x/sys v0.16.0
golang.org/x/text v0.14.0
google.golang.org/appengine v1.6.8 // indirect
Expand All @@ -71,6 +70,7 @@ require (
require (
github.com/IGLOU-EU/go-wildcard v1.0.3
github.com/goccy/go-yaml v1.11.0
github.com/google/go-github/v59 v59.0.0
github.com/puzpuzpuz/xsync/v3 v3.0.2
github.com/samber/lo v1.38.1
github.com/shirou/gopsutil/v3 v3.23.7
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -276,8 +276,8 @@ github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeN
github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/google/go-github/v53 v53.0.0 h1:T1RyHbSnpHYnoF0ZYKiIPSgPtuJ8G6vgc0MKodXsQDQ=
github.com/google/go-github/v53 v53.0.0/go.mod h1:XhFRObz+m/l+UCm9b7KSIC3lT3NWSXGt7mOsAWEloao=
github.com/google/go-github/v59 v59.0.0 h1:7h6bgpF5as0YQLLkEiVqpgtJqjimMYhBkD4jT5aN3VA=
github.com/google/go-github/v59 v59.0.0/go.mod h1:rJU4R0rQHFVFDOkqGWxfLNo6vEk4dv40oDjhV/gH6wM=
github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8=
github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU=
github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
Expand Down
26 changes: 15 additions & 11 deletions pkg/larker/fs/github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,26 @@ import (
"errors"
"fmt"
"github.com/cirruslabs/cirrus-cli/pkg/larker/fs"
"github.com/google/go-github/v53/github"
"github.com/google/go-github/v59/github"
lru "github.com/hashicorp/golang-lru"
"golang.org/x/oauth2"
"net/http"
"os"
"path"
"syscall"
"time"
)

var ErrAPI = errors.New("failed to communicate with the GitHub API")

var defaultGitHubClient = github.NewClient(&http.Client{
Transport: &http.Transport{
MaxIdleConns: 1024,
MaxIdleConnsPerHost: 1024, // default is 2 which is too small and we mostly access the same host
IdleConnTimeout: time.Minute, // let's put something big but not infinite like the default
},
Timeout: 11 * time.Second, // GitHub has a 10-second timeout for API requests
})

type GitHub struct {
token string
owner string
Expand Down Expand Up @@ -119,17 +128,12 @@ func (gh *GitHub) Join(elem ...string) string {
return path.Join(elem...)
}

func (gh *GitHub) client(ctx context.Context) *github.Client {
var client *http.Client

func (gh *GitHub) client() *github.Client {
if gh.token != "" {
tokenSource := oauth2.StaticTokenSource(&oauth2.Token{
AccessToken: gh.token,
})
client = oauth2.NewClient(ctx, tokenSource)
return defaultGitHubClient.WithAuthToken(gh.token)
}

return github.NewClient(client)
return defaultGitHubClient
}

func (gh *GitHub) getContentsWrapper(
Expand All @@ -143,7 +147,7 @@ func (gh *GitHub) getContentsWrapper(

gh.apiCallCount++

fileContent, directoryContent, resp, err := gh.client(ctx).Repositories.GetContents(ctx, gh.owner, gh.repo, path,
fileContent, directoryContent, resp, err := gh.client().Repositories.GetContents(ctx, gh.owner, gh.repo, path,
&github.RepositoryContentGetOptions{
Ref: gh.reference,
},
Expand Down

0 comments on commit 3a71e8f

Please sign in to comment.