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

*: fragment watch response by server request limit #9291

Merged
merged 12 commits into from
May 14, 2018

Conversation

gyuho
Copy link
Contributor

@gyuho gyuho commented Feb 7, 2018

Replace #8371.
Address #8188 and #9294.

If watch response exceeds server-side request limit, it automatically splits watch response events and sends fragments over RPC. Clients still receive combined results, while the server-to-client dispatch happens with fragments.

Given this use case:

  • --max-request-bytes '1572864' (1.5 MiB)
  • Total 100 MiB watch response of 100 events; each event is 1 MiB
  • clientv3.MaxCallRecvMsgSize is 1.5 MiB

Without this patch, fail with error code = ResourceExhausted desc = grpc: received message larger than max (10486027 vs. 1572864). With this patch, 100 events will be split over 100 fragmented watch response in server-side, then joined back in client before it being returned to the user.

Nothing changes from user's perspective.

@gyuho gyuho added the WIP label Feb 7, 2018
@gyuho gyuho force-pushed the fragment-watch branch 3 times, most recently from 742810c to 04d7f90 Compare February 7, 2018 07:55
@gyuho gyuho changed the title *: fragment watch response with server request limit *: fragment watch response by server request limit Feb 7, 2018
@gyuho gyuho force-pushed the fragment-watch branch 3 times, most recently from 9073a41 to e3ff373 Compare February 8, 2018 15:17
@gyuho gyuho requested review from jpbetz and xiang90 February 8, 2018 17:40
@gyuho
Copy link
Contributor Author

gyuho commented Feb 13, 2018

@SuhasAnand Can you give this a try?

The client receive limit error should be resolved via MaxCallRecvMsgSize in clientv3.Config (it was 4 MiB in <= 3.2.11). This PR splits events if combined response size exceeds server-side request limit. It's 1 watch send of 100 MiB vs. 100 watch sends (each send is 1 MiB).

gyuho added 9 commits May 14, 2018 12:35
Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
gyuho added 2 commits May 14, 2018 13:34
Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
@gyuho
Copy link
Contributor Author

gyuho commented May 14, 2018

Did benchmark of total 120 MiB watch events, each of which contains 1.2 MiB value, so that server would split one watch response with 100 fragmented event:

watch with no fragmentation took 402.968818ms
watch with fragmentation took 441.428922ms

So, fragmentation would only have minimal overhead (mostly over the network), which is expected. There's no noticeable performance change and fragmentation will be disabled by default.

@etcd-io etcd-io deleted a comment from codecov-io May 14, 2018
@gyuho gyuho merged commit 53373fe into etcd-io:master May 14, 2018
@gyuho gyuho deleted the fragment-watch branch May 14, 2018 22:09
@SuhasAnand
Copy link

@gyuho lgtm. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants