Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

gitserver(client): Reintroduce 500 maximum connections limit#63064

Merged
eseliger merged 2 commits into
mainfrom
es/06-04-gitserverclientreintroduce500maximumconnectionslimit
Jun 6, 2024
Merged

gitserver(client): Reintroduce 500 maximum connections limit#63064
eseliger merged 2 commits into
mainfrom
es/06-04-gitserverclientreintroduce500maximumconnectionslimit

Conversation

@eseliger

@eseliger eseliger commented Jun 4, 2024

Copy link
Copy Markdown
Member

This used to exist in the HTTP world, and we currently have zero safeguards to prevent clients from making one billion requests concurrently.
Until we invest more into server-side rate limiting, or per tenant rate limiting, we reintroduce this limiter, to prevent resource usage spikes.

Test plan:

Added a test suite.

@cla-bot cla-bot Bot added the cla-signed label Jun 4, 2024

eseliger commented Jun 4, 2024

Copy link
Copy Markdown
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @eseliger and the rest of your teammates on Graphite Graphite

@github-actions github-actions Bot added team/product-platform team/source Tickets under the purview of Source - the one Source to graph it all labels Jun 4, 2024
@eseliger eseliger force-pushed the es/06-04-gitserverclientreintroduce500maximumconnectionslimit branch from f9da7fc to a5989f5 Compare June 4, 2024 07:40
@eseliger eseliger marked this pull request as ready for review June 4, 2024 07:58
@eseliger eseliger requested a review from a team June 4, 2024 07:58
Comment thread internal/gitserver/connection/addrs.go Outdated
@eseliger eseliger requested a review from a team June 5, 2024 07:56
This used to exist in the HTTP world, and we currently have zero safeguards to prevent clients from making one billion requests concurrently.
Until we invest more into server-side rate limiting, or per tenant rate limiting, we reintroduce this limiter, to prevent resource usage spikes.

Test plan:

Added a test suite.
@eseliger eseliger force-pushed the es/06-04-gitserverclientreintroduce500maximumconnectionslimit branch 3 times, most recently from f592bed to 939a586 Compare June 6, 2024 12:50
Co-authored-by: Geoffrey Gilmore <geoffrey@sourcegraph.com>
@eseliger eseliger force-pushed the es/06-04-gitserverclientreintroduce500maximumconnectionslimit branch from 939a586 to 12313e6 Compare June 6, 2024 12:59
@eseliger eseliger merged commit 9185da3 into main Jun 6, 2024
@eseliger eseliger deleted the es/06-04-gitserverclientreintroduce500maximumconnectionslimit branch June 6, 2024 13:18
eseliger added a commit that referenced this pull request Jun 6, 2024
eseliger added a commit that referenced this pull request Jun 6, 2024
…63064)" (#63132)

This reverts commit 9185da3.

Noticed there are some bad callers in worker and symbols that don't
properly return a connection. Will need to investigate and fix that
first.

## Test plan

Worked before, CI passes.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla-signed team/product-platform team/source Tickets under the purview of Source - the one Source to graph it all

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants