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

Setting channel option max_message_length ~30mb+ overflows int #11299

Closed
dzbarsky opened this issue May 24, 2017 · 5 comments
Closed

Setting channel option max_message_length ~30mb+ overflows int #11299

dzbarsky opened this issue May 24, 2017 · 5 comments

Comments

@dzbarsky
Copy link

Please answer these questions before submitting your issue.

What version of gRPC and what language are you using?

Python, 1.0.4 and 1.2.1, haven't tried more recent.

What operating system (Linux, Windows, …) and version?

Ubuntu 14.04

What runtime / compiler are you using (e.g. python version or version of gcc)

Python 2.7.12

What did you do?

creds = grpc.ssl_channel_credentials(
            self.ca_cert_pem, # doesn't matter where these come from, just some certs
            private_key=self.key_pem,
            certificate_chain=self.cert_pem)
options = [('grpc.max_message_length', 30 * 1024 * 1024)]
return grpc.secure_channel("127.0.0.1:4000", creds, options=options)

What did you expect to see?

Create a channel with max message length

What did you see instead?

  File "big_message_go_server_test_bin-piplib/grpc/__init__.py", line 1273, in secure_channel
    credentials._credentials)
  File "big_message_go_server_test_bin-piplib/grpc/_channel.py", line 925, in __init__
    _common.channel_args(_options(options)), credentials)
  File "big_message_go_server_test_bin-piplib/grpc/_common.py", line 105, in channel_args
    cygrpc_args.append(cygrpc.ChannelArg(encode(key), value))
  File "src/python/grpcio/grpc/_cython/_cygrpc/records.pyx.pxi", line 367, in grpc._cython.cygrpc.ChannelArg.__cinit__ (src/python/grpcio/grpc/_cython/cygrpc.c:14896)
OverflowError: value too large to convert to int

By the way, the docs say all the options are string -> string pairs, but this one is an int.

@nathanielmanistaatgoogle
Copy link
Member

Have you tried other values? What seems to be the upper limit?

In a more recent grpcio, have you tried grpc.max_send_message_length and grpc.max_receive_message_length? Do you observe the same behavior with them?

What are you doing that requires large messages? Why are streaming RPCs not the better fit for your use case?

@nathanielmanistaatgoogle nathanielmanistaatgoogle changed the title [python] Setting channel option max_message_length ~30mb+ overflows int Setting channel option max_message_length ~30mb+ overflows int May 26, 2017
@dzbarsky
Copy link
Author

dzbarsky commented Jun 2, 2017

I got an upgrade to the more recent version and things seem to be working now with 200MB. While streaming RPC is definitely the right technical solution, the reality is I am adopting grpc for an organization with 100+ services so I can't rewrite them all myself, I can only advise other teams how to design the API for their service. :)

It would still be nice to clean this up: "By the way, the docs say all the options are string -> string pairs, but this one is an int."

@nathanielmanistaatgoogle
Copy link
Member

@dzbarsky: ordinarily I'd cheerfully solicit a pull request but channel arguments are a kind of "advanced" and experimental feature, are not supported to work in their current form into the indefinite future or in ways that abide semantic versioning (look at the way that grpc.max_message_length was split into two without a major version bump), and are thus intentionally underdocumented.

Separately, fun fact: apparently 16-64 KiB is the "recommended" maximum message length.

@kpayson64
Copy link
Contributor

@dzbarsky
Where did you find the incorrect documentation stating options were string->string pairs?

@dzbarsky
Copy link
Author

@nathanielmanistaatgoogle That link refers to oral tradition (I assume from google). What are we optimizing for? Latency? Throughput? CPU/Power usage? I'd imagine the first 2 are affected by network conditions, and I would guess that at least the last one is heavily influenced by choice of language and protobuf library. For example, open source Python protos are pretty slow, so it may be better to have fewer messages that are bigger in size. But It's hard to say without writing some benchmarks :)

BTW you basically need channel arguments to create secure channel (override ssl to match cert), so I don't see it as an advanced feature. Overall we've found the documentation to be pretty sparse so we just read the code. If you don't want certain things to be used, it would be helpful if you could leave comments to that effect.

And yes, we noticed that max_message_length was split in two, these sorts of changes aren't very friendly to downstream. (This change in particular is trivial to deal with but other API change have caused us some pain).

@kpayson64 unfortunately I can't find where I saw that anymore, sorry.

I'm going to close out this issue since we've confirmed that 200MB messages work with master, but please consider improving the documentation to make life easier for users.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants