Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add config for connect params for the gRPC client #296

Merged
merged 3 commits into from
May 18, 2023

Conversation

chaudum
Copy link
Contributor

@chaudum chaudum commented May 12, 2023

What this PR does:

This PR adds configuration options for the gRPC client, namely the connect connect timeout, and backoff delays.

In Loki we observed that the default gRPC connect timeout caused tail latency when the client is used in a client pool for the ring, e.g. index-gateway clients. In case a server is unexpectedly restarted (e.g. due to OOMing) the server still remains in the ring even though it is may be unavailable or not ready (during startup). However, the client pool still create the client for the address of the server and tries to connect. A long timeout when establishing a connection introduces the latency until the next server is tried.

https://github.com/grafana/loki/blob/main/pkg/storage/stores/indexshipper/gatewayclient/gateway_client.go#L327-L357

Checklist

  • Tests updated
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

chaudum and others added 2 commits May 12, 2023 16:23
Signed-off-by: Christian Haudum <christian.haudum@gmail.com>
@DylanGuedes DylanGuedes merged commit 3c92c53 into main May 18, 2023
3 checks passed
@DylanGuedes DylanGuedes deleted the chaudum/grpc-connect-timeout-config branch May 18, 2023 16:23
DylanGuedes added a commit to grafana/loki that referenced this pull request May 23, 2023
**What this PR does / why we need it**:
Bump `dskit` to inherit grafana/dskit#296.
It will allow us to configure custom backoff timeouts.

**Which issue(s) this PR fixes**:
N/A
DylanGuedes added a commit to grafana/loki that referenced this pull request May 23, 2023
**What this PR does / why we need it**:
Bump `dskit` to inherit grafana/dskit#296.
It will allow us to configure custom backoff timeouts.

**Which issue(s) this PR fixes**:
N/A
@charleskorn charleskorn mentioned this pull request Jun 7, 2023
2 tasks
masonmei pushed a commit to masonmei/dskit that referenced this pull request Sep 6, 2023
* Switch uses of go-kit/kit/log to go-kit/log

Fixes grafana#296

Signed-off-by: Nick Pillitteri <nick.pillitteri@grafana.com>

* Use faillint for checking dependencies per code review

Signed-off-by: Nick Pillitteri <nick.pillitteri@grafana.com>

* Include all of go-kit/kit/log in lint check

Signed-off-by: Nick Pillitteri <nick.pillitteri@grafana.com>
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.

None yet

3 participants