-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
embed: add GRPCAdditionalServerOptions
config
#14066
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1923,7 +1923,71 @@ func TestV3LargeRequests(t *testing.T) { | |
t.Errorf("#%d: range expected no error, got %v", i, err) | ||
} | ||
} | ||
}) | ||
} | ||
} | ||
|
||
// TestV3AdditionalGRPCOptions ensures that configurable GRPCAdditionalServerOptions works as intended. | ||
func TestV3AdditionalGRPCOptions(t *testing.T) { | ||
integration.BeforeTest(t) | ||
tests := []struct { | ||
name string | ||
maxRequestBytes uint | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add one more field |
||
grpcOpts []grpc.ServerOption | ||
valueSize int | ||
expectError error | ||
}{ | ||
{ | ||
name: "requests will get a gRPC error because it's larger than gRPC MaxRecvMsgSize", | ||
maxRequestBytes: 8 * 1024 * 1024, | ||
grpcOpts: nil, | ||
valueSize: 9 * 1024 * 1024, | ||
expectError: status.Errorf(codes.ResourceExhausted, "grpc: received message larger than max"), | ||
}, | ||
{ | ||
name: "requests will get an etcd custom gRPC error because it's larger than MaxRequestBytes", | ||
maxRequestBytes: 8 * 1024 * 1024, | ||
grpcOpts: []grpc.ServerOption{grpc.MaxRecvMsgSize(10 * 1024 * 1024)}, | ||
valueSize: 9 * 1024 * 1024, | ||
expectError: rpctypes.ErrGRPCRequestTooLarge, | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please consider to add a case that the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. any update on this? |
||
{ | ||
name: "requests size is smaller than MaxRequestBytes but larger than MaxRecvMsgSize", | ||
maxRequestBytes: 8 * 1024 * 1024, | ||
grpcOpts: []grpc.ServerOption{grpc.MaxRecvMsgSize(4 * 1024 * 1024)}, | ||
valueSize: 6 * 1024 * 1024, | ||
expectError: status.Errorf(codes.ResourceExhausted, "grpc: received message larger than max"), | ||
}, | ||
} | ||
for _, test := range tests { | ||
t.Run(test.name, func(t *testing.T) { | ||
clus := integration.NewCluster(t, &integration.ClusterConfig{ | ||
Size: 1, | ||
MaxRequestBytes: test.maxRequestBytes, | ||
ClientMaxCallSendMsgSize: 12 * 1024 * 1024, | ||
GRPCAdditionalServerOptions: test.grpcOpts, | ||
}) | ||
defer clus.Terminate(t) | ||
kvcli := integration.ToGRPC(clus.Client(0)).KV | ||
reqput := &pb.PutRequest{Key: []byte("foo"), Value: make([]byte, test.valueSize)} | ||
if _, err := kvcli.Put(context.TODO(), reqput); err != nil { | ||
if _, ok := err.(rpctypes.EtcdError); ok { | ||
if err.Error() != status.Convert(test.expectError).Message() { | ||
t.Errorf("expected %v, got %v", status.Convert(test.expectError).Message(), err.Error()) | ||
} | ||
} else if !strings.HasPrefix(err.Error(), test.expectError.Error()) { | ||
t.Errorf("expected error starting with '%s', got '%s'", test.expectError.Error(), err.Error()) | ||
} | ||
} | ||
// request went through, expect large response back from server | ||
if test.expectError == nil { | ||
reqget := &pb.RangeRequest{Key: []byte("foo")} | ||
// limit receive call size with original value + gRPC overhead bytes | ||
_, err := kvcli.Range(context.TODO(), reqget, grpc.MaxCallRecvMsgSize(test.valueSize+512*1024)) | ||
if err != nil { | ||
t.Errorf("range expected no error, got %v", err) | ||
} | ||
} | ||
}) | ||
} | ||
} | ||
|
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.
It would be better if we also add a page under the https://etcd.io/docs/v3.5/dev-guide/ to document this. Can you raise a ticket in https://github.com/etcd-io/website/issues?
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.
etcd-io/website#862