Skip to content

Optimize HTTP client used for GitHub#689

Merged
fkorotkov merged 3 commits intomasterfrom
optiomized-gh-http-client
Feb 16, 2024
Merged

Optimize HTTP client used for GitHub#689
fkorotkov merged 3 commits intomasterfrom
optiomized-gh-http-client

Conversation

@fkorotkov
Copy link
Copy Markdown
Contributor

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.

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`.
@fkorotkov fkorotkov requested a review from edigaryev February 15, 2024 20:52
@fkorotkov fkorotkov enabled auto-merge (squash) February 15, 2024 20:53
@fkorotkov fkorotkov merged commit 3a71e8f into master Feb 16, 2024
@fkorotkov fkorotkov deleted the optiomized-gh-http-client branch February 16, 2024 14:12
edigaryev added a commit that referenced this pull request Sep 30, 2025
edigaryev added a commit that referenced this pull request Sep 30, 2025
edigaryev added a commit that referenced this pull request Oct 6, 2025
* Larker: enable HTTP caching for GitHub API client

* Bring back #689 tuning

* Initialize HTTPClient once

* Only use init() for tuning the HTTPClient after it's initialized

* Per-instance HTTPClient and GitHub client

* Share caching *http.Client for multiple github.New() invocations

* Per-tenant shared HTTP cache

* Use Get() methods to prevent NPE

* Introduce TestGitHubHTTPCache

* TestGitHubHTTPCache: test with two separate tenants

* $ go mod tidy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants