-
Notifications
You must be signed in to change notification settings - Fork 9.9k
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
Watch sync fails when Fragment is false #19441
Comments
Thanks @kevburnsjr for raising this issue. We might need to figure out starting from which grpc version the behavour changed. @dfawley, @easwars is there any behaviour change on grpc side in term of the default max response size the grpc support? |
FYI. etcd sets the response limit as Line 40 in 1e3710a
|
We did have this recent change which might be related: grpc/grpc-go#7918 @arjan-bal might have more context since he was involved in that change's review. |
I found this client test which seems to indicate that this might have been expected behavior for the last 7 years. |
@easwars the change in grpc/grpc-go#7918 effected the decompression code path. The error seen here is returned before that, at the time of splitting the http/2 data frame into grpc header and body.
etcd/tests/framework/integration/cluster.go Lines 682 to 684 in deb9178
etcd/server/etcdserver/api/v3rpc/grpc.go Line 77 in deb9178
|
|
After adding logs within gRPC to debug this, the problem is happening in the gRPC client. The max receive size is being set using a call option. The call options are not set on the entire ClientConn, instead they are stored in a field of the client and used every time a stream is created. Line 75 in deb9178
Based on the logs, it looks like the call options are not provided when the watch is created by the following lines in the test code: ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
wStream, err := integration.ToGRPC(clus.RandClient()).Watch.Watch(ctx) // No call options passed to gRPC
require.Nil(t, err) As a result gRPC uses the default value of 4MB as the max receive size. Passing a call option during stream creation makes the test pass. wStream, err := integration.ToGRPC(clus.RandClient()).Watch.Watch(ctx, grpc.MaxCallRecvMsgSize(math.MaxInt32)) |
Thanks @arjan-bal . Also Thanks @easwars . Actually the etcd client automatically adds such grpc option by default. @kevburnsjr Note that you won't see this issue if you call the client.Watch interface. See below, let me know whether if it resolve your problem. Line 82 in ef65923
Line 298 in ef65923
|
It's because the test case intentionally sets a 1.5MiB max recv msg size.
Please feel free to reopen this ticket if you still have any concern. Thanks all again! |
Bug report criteria
What happened?
Server having four revisions to sync returns 1 response with 48 events @ 4.8 MB resulting in an error
What did you expect to happen?
Server having four revisions to sync returns 4 responses with 12 events each @ 1.2 MB per response
How can we reproduce it (as minimally and precisely as possible)?
See failing test in #19442
StartRevision
= The revision of the first of the four transactionsFragment
=false
Anything else we need to know?
Max response size is set to default of 1.5 MB
Etcd version (please run commands below)
Reproduced in
v3.3.27
v3.4.35
v3.5.18
main
Possibly related
#8188 (comment)
Etcd configuration (command line flags or environment variables)
docker-compose.yml
Etcd debug information (please run commands below, feel free to obfuscate the IP address or FQDN in the output)
See failing test in #19442
Relevant log output
The text was updated successfully, but these errors were encountered: