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

gRPC channel config set limits #12539

Merged
merged 12 commits into from Jun 3, 2019

Conversation

Projects
None yet
4 participants
@Rick-Anderson
Copy link
Contributor

commented May 25, 2019

luizcarlosfaria and others added some commits May 21, 2019

Add Channel Configuration
Adding channel configuration and expose the relation between server and client configurations to address same points.

@Rick-Anderson Rick-Anderson changed the title Rick anderson/g rpc channel config WIP: gRPC channel config set limits May 25, 2019

Rick-Anderson added some commits May 25, 2019

@luizcarlosfaria

This comment has been minimized.

Copy link

commented on aspnetcore/grpc/configuration.md in 6a64f2a May 25, 2019

this content broke the markdown

Rick-Anderson added some commits May 25, 2019

@Rick-Anderson Rick-Anderson changed the title WIP: gRPC channel config set limits gRPC channel config set limits May 25, 2019

@Rick-Anderson

This comment has been minimized.

Copy link
Contributor Author

commented May 25, 2019

@luizcarlosfaria
To review the doc build

  • download this DB file
  • unzip
  • view HTML file in FF (Fire Fox). It best to view the doc with the widest window that has no left or right TOC (Table of contents)

@Rick-Anderson Rick-Anderson requested a review from JamesNK May 25, 2019

@Rick-Anderson

This comment has been minimized.

Copy link
Contributor Author

commented May 25, 2019

@luizcarlosfaria Thanks!
Can you check my last commit?

@luizcarlosfaria
Copy link

left a comment

@Rick-Anderson thank you, but we have small issue.

Description:
GrpcServiceOptions.ReceiveMaxMessageSize must be a same value of ChannelOptions.MaxSendMessageLength.

Changes need:
FROM

options.ReceiveMaxMessageSize = 10 * 1024 * 1024;  // 10 megabytes

TO

options.ReceiveMaxMessageSize = 2 * 1024 * 1024;  // 2 megabytes
{
services.AddGrpc().AddServiceOptions<MyService>(options =>
{
options.ReceiveMaxMessageSize = 10 * 1024 * 1024; // 10 megabytes

This comment has been minimized.

Copy link
@luizcarlosfaria

luizcarlosfaria May 25, 2019

To be consistent between client and server configs, need change:
FROM

options.ReceiveMaxMessageSize = 10 * 1024 * 1024;  // 10 megabytes

TO

options.ReceiveMaxMessageSize = 2 * 1024 * 1024;  // 2 megabytes
@luizcarlosfaria

This comment has been minimized.

Copy link

commented May 25, 2019

@luizcarlosfaria Thanks!
Can you check my last commit?

Ok, it's ok for me!

@luizcarlosfaria

This comment has been minimized.

Copy link

commented May 25, 2019

Thank you.


Kestrel server has configuration options that affect the behavior of gRPC on ASP.NET Core.

### Request body data rate limit

This comment has been minimized.

Copy link
@JamesNK

JamesNK May 25, 2019

Member

Remove this section (and Configure Kestrel options since this is the only section under it, unless you want).

gRPC framework now raised this limit for you automatically.


The following highlighted code sets the server options listed in the preceding table:

[!code-csharp[](~/grpc/configuration/sample/GrcpService/Startup2.cs?name=snippet)]

This comment has been minimized.

Copy link
@JamesNK

JamesNK May 25, 2019

Member

Setting the options in AddServiceOptions<MyService> will only change the limits for that service (MyService). Setting them in AddGrpc will change them for all services (unless overridden).

IMO change this to set these values in AddGrpc. That is what most people will do.


The following code sets the client options listed in the preceding table:

[!code-csharp[](~/grpc/configuration/sample/Program.cs?name=snippet&highlight=3-6)]

This comment has been minimized.

Copy link
@JamesNK

JamesNK May 25, 2019

Member

This is fine but hopefully next week we'll have a new managed client checked in. Note that it won't support message size limits in preview 6.

When the managed client ships in preview 6 a note should be added here that it does not currently have a limit.

@Rick-Anderson

This comment has been minimized.

Copy link
Contributor Author

commented May 28, 2019

@luizcarlosfaria please review.

@Rick-Anderson

This comment has been minimized.

Copy link
Contributor Author

commented Jun 1, 2019

@luizcarlosfaria your english is fine. I just like to use the google translator.

@luizcarlosfaria

This comment has been minimized.

Copy link

commented Jun 1, 2019

Hi,
i'm looking the final version of wiki and the result doesn't have a topic about Client Configuration.

We have one topic "Configure services options", but we don't have any topic like "Configure a client options" to explain with a table the options on client, it's ok?

@Rick-Anderson

This comment has been minimized.

Copy link
Contributor Author

commented Jun 3, 2019

@luizcarlosfaria added ## Configure client options

@luizcarlosfaria

This comment has been minimized.

Copy link

commented Jun 3, 2019

Thank you @Rick-Anderson! How can i see the final result?

@Rick-Anderson

This comment has been minimized.

@luizcarlosfaria

This comment has been minimized.

Copy link

commented Jun 3, 2019

Thank you, thanks about your time.
It's ok for me! All things are addressed.

@Rick-Anderson

This comment has been minimized.

Copy link
Contributor Author

commented Jun 3, 2019

@JamesNK you OK with publishing?

@JamesNK

JamesNK approved these changes Jun 3, 2019

Copy link
Member

left a comment

:shipit:

@Rick-Anderson Rick-Anderson merged commit 129a425 into master Jun 3, 2019

6 checks passed

OpenPublishing.Build Validation status: passed
Details
OpenPublishing.Build (1 of 3) Waiting for processor completed at 11:56:09 PST
OpenPublishing.Build (2 of 3) Preparing completed at 11:57:23 PST
OpenPublishing.Build (3 of 3) Building completed at 11:57:33 PST
WIP Ready for review
Details
license/cla All CLA requirements met.
Details

@delete-merged-branch delete-merged-branch bot deleted the rick-anderson/gRPC-channel-config branch Jun 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.