-
Notifications
You must be signed in to change notification settings - Fork 795
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
Upgrade ETCD/GRPC #3884
Upgrade ETCD/GRPC #3884
Conversation
Signed-off-by: Jacob Lisi <jacob.t.lisi@gmail.com>
Signed-off-by: Jacob Lisi <jacob.t.lisi@gmail.com>
Signed-off-by: Jacob Lisi <jacob.t.lisi@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm good with the copy, especially considering this may reduce the blast radius of issues we may potentially introduce with this upgrade (gRPC and etcd). I just left a comment about package naming.
pkg/util/naming/dns_resolver.go
Outdated
package naming | ||
|
||
// Copied from https://github.com/grpc/grpc-go/tree/v1.29.x/naming. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think to copy all these files to pkg/util/grpc/naming
to make it more clear it's about grpc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, I'll move the package.
Signed-off-by: Jacob Lisi <jacob.t.lisi@gmail.com>
This brings in at least one optimisation from gRPC: grpc/grpc-go#3568 (should be visible in streaming queries, also in hand-over from one ingester to another which I for one don't run any more). Please look out for any differences visible in your dashboards, and consider including in CHANGELOG. |
ETCD has been updated here: cortexproject/cortex#3884
ETCD has been updated here: cortexproject/cortex#3884
ETCD has been updated here: cortexproject/cortex#3884
* Upgrade Cortex to latest release v1.9.0 * Don't set CompressResponses as this is a no-op CompressResponses has been removed in v1.9.0 of Cortex. Additionally, this setting actually did not change anything when used in Tempo. In Cortex this setting was used when creating the handler in query-frontend: https://github.com/cortexproject/cortex/blob/4afaa357469fb37fd92cb1e2a0c1d10cdddc5ecd/pkg/cortex/modules.go#L557-L559 Since we do not use this code and have our own initialisation code for query-frontend, this setting never changed anything. * Update github.com/go-openapi, match with prometheus/alertmanager * Bump golangci-lint * Check in all vendor files (some were ignored for some reason?) * Update GRPC as ETCD has been updated in Cortex ETCD has been updated here: cortexproject/cortex#3884 * make vendor-check * Drop unnecessary replace statement
What this PR does:
v1.32.0
etcd
imports to match the new etcd module packagesnaming
naming package that was deprecated topkg/util/naming
to ensure no changes to behavior are introduced. I may be wrong but I don't think upstreamgrpc
library provides a resolver interface that meets our use case anymore and we may be better off rolling our own implementation. I am open to suggestion on whether porting is the right approach.Which issue(s) this PR fixes:
Fixes #2015
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]