-
Notifications
You must be signed in to change notification settings - Fork 0
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
experiment with gRPC outlier detection balancer #10
base: client_fail_over_poc_internal
Are you sure you want to change the base?
experiment with gRPC outlier detection balancer #10
Conversation
03eea6a
to
d6401d4
Compare
@@ -24,6 +24,7 @@ import ( | |||
"github.com/stretchr/testify/require" | |||
"google.golang.org/grpc" | |||
_ "google.golang.org/grpc/health" | |||
_ "google.golang.org/grpc/xds" |
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.
This is used to import https://github.com/grpc/grpc-go/blob/master/xds/internal/balancer/outlierdetection/balancer.go#L57-L61 to register the outlierdetection balancer in the global registry. Unfortunately, it has not yet moved out to public.. See the gRPC-go maintainer comment in grpc/grpc-go#5672 (comment) and grpc/grpc-go#5899 (comment)
/cc @tjungblu since you summarized the use case pretty well in etcd-io#14501
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.
that's really awesome @chaochn47! I'll give this a bash on Friday when I find some time.
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.
works pretty well, I just gave this a quick run with an apiserver.
We also want to tune the retry policies in the future, I'm still a little unsure how to expose this in the etcd client. Exposing the raw service config is probably not a good idea given the potential changes to xds...
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.
works pretty well, I just gave this a quick run with an apiserver.
Thanks! That's a good data point.
We also want to tune the retry policies in the future, I'm still a little unsure how to expose this in the etcd client.
Retry policy is on my to-do list to explore. Will ping you when I find a reliable way to do this with etcd client. This might need some changes in v3 etcd client code base.
Exposing the raw service config is probably not a good idea given the potential changes to xds...
Changes to xds should not affect how we configure outlier detection load balancing policy. It is part of the service config proto definition in the main grpc repo. https://github.com/grpc/grpc-proto/blob/master/grpc/service_config/service_config.proto#L241
d6401d4
to
35ea4f1
Compare
Signed-off-by: Chao Chen <chaochn@amazon.com>
35ea4f1
to
90b5e66
Compare
This is an experiment on top of #9 which shows better etcd availability combined with gRPC health check.
The configured outlier detection config is if within 2s, there are more than 5 requests (RPC) failed, it will at most eject one backend address out of the 3 etcd addresses. It will kick in earlier than current configured 6s latency that costs server push unhealthy status to client.
Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.