diagnostics: Add tests for graceful handling of oversized gRPC messages |
|||||||||
Issue descriptionIt looks like gRPC has a hardcoded constraint on the messages that it can handle - 4MB. The problem is that the wrappers from grpc_async_adapter don't return any error in this case, and behave is if the request was hung forever. We should find a way to handle this gracefully - i.e., bail out such oversized requests instead of silently hanging them. Perhaps the same problem appears with the responses.
,
Oct 31
Maksim kindly provided a demo in https://chromium-review.googlesource.com/c/chromiumos/platform2/+/1311475 .
,
Nov 5
gRPC is behaving correctly here, the test was hanging because it waited for an incoming RPC request on the "server" side, but the oversize message was rejected on the library level before it could reach that - see discussion on the CL. Maksim has repurposed the CL to add tests for this, so assigning to him. I will create a CL to clarify naming in the tests to make sure what is server-side and what is client-side.
,
Nov 5
,
Nov 7
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/b984489643611ff942f9a2711c08828ce8bab510 commit b984489643611ff942f9a2711c08828ce8bab510 Author: Maksim Ivanov <emaxx@chromium.org> Date: Wed Nov 07 04:19:25 2018 diagnostics: Add tests for huge gRPC messages Add tests for the cases when gRPC requests and/or responses are either excessive big or big although within the acceptable bounds. For the context, the threshold is determined by the gRPC library itself and is about 4 MiB. BUG= chromium:900753 TEST=unit tests Change-Id: Idaf48f65bbf6a02b41bc71a287e1d610f0dbc067 Reviewed-on: https://chromium-review.googlesource.com/1311475 Commit-Ready: Maksim Ivanov <emaxx@chromium.org> Tested-by: Maksim Ivanov <emaxx@chromium.org> Reviewed-by: Pavol Marko <pmarko@chromium.org> [modify] https://crrev.com/b984489643611ff942f9a2711c08828ce8bab510/diagnostics/grpc_async_adapter/async_grpc_client_server_test.cc [modify] https://crrev.com/b984489643611ff942f9a2711c08828ce8bab510/diagnostics/grpc_async_adapter/test_rpcs.proto
,
Nov 14
Reassigning back to Pavol as per comment 3.
,
Nov 26
,
Nov 26
,
Nov 26
,
Nov 26
,
Jan 14
I think we can close this bug - the only thing that is left here is variables renaming in tests, which seems super low-priority and also possible to be done later in a batch with other refactoring/polishing. |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by pmarko@chromium.org
, Oct 31