Skip to content

Large message benchmarks#9392

Merged
ctiller merged 3 commits intogrpc:masterfrom
ctiller:large_message_benchmarks
Jan 28, 2017
Merged

Large message benchmarks#9392
ctiller merged 3 commits intogrpc:masterfrom
ctiller:large_message_benchmarks

Conversation

@ctiller
Copy link
Copy Markdown
Member

@ctiller ctiller commented Jan 18, 2017

No description provided.

@ctiller ctiller merged commit 3d760ae into grpc:master Jan 28, 2017
# For proto payload, only the client should get the config.
scenario['client_config']['payload_config'] = EMPTY_PROTO_PAYLOAD
scenario['client_config']['payload_config'] = _payload_type(use_generic_payload, req_size, resp_size)
scenario['server_config']['payload_config'] = _payload_type(use_generic_payload, req_size, resp_size)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the problem here is that payload_config should only be set for client for non-generic server. The reason is that the non-generic server (Async and Sync) are supposed to respond with a message size that corresponds to what is requested in SimpleRequest (which is constructed based on client's payload config).

Setting payload config for a server should be illegal (and so C# qps worker is asserting that it is only set for generic server), which is what's breaking C# benchmarks (#9587).

https://github.com/grpc/grpc/blob/master/src/proto/grpc/testing/messages.proto#L76

PayloadConfig payload_config = 9;

if (!Server::SetPayload(request->response_type(), request->response_size(),

@jtattermusch
Copy link
Copy Markdown
Contributor

Two problems with this PR:

  1. is seems that the units tests didn't run at all (only CLA check has run so things look green, but they arent).
  2. the change has probably broken C# benchmarks (csharp benchmark failures #9587 and Multiple qps timeouts/failures #9499) - see inline comment.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 25, 2019
@lock lock bot unassigned sreecha Jan 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants