andygrove commented on PR #2173: URL: https://github.com/apache/datafusion-ballista/pull/2173#issuecomment-5133044191
Thanks both, I've reworked this based on your feedback and updated the title and description. @martin-g fair point about losing the signal that messages are getting heavy. I've dropped the default bump entirely, so everything stays at 16 MiB and optimisations like #1853 still get noticed when a message grows too big. The PR is now only about making the ceiling configurable where it wasn't, so operators who genuinely need a bigger one can raise it. I've also taken your MiB wording suggestions, just with the value left at 16MiB. @phillipleblanc good catch, and it turned out to be worse than "not wired". `ExecutorManager` was computing `grpc_client_config.max_message_size` and then dropping it on the floor, because `get_client` built the client with a plain `ExecutorGrpcClient::new(connection)`. It now applies both `max_encoding_message_size` and `max_decoding_message_size`, with a comment explaining that these are tonic codec settings so `create_grpc_client_endpoint` cannot apply them. I checked every other client construction in the tree (`executor_process.rs`, `executor_server.rs`, `standalone.rs`, `flight_proxy_service.rs`, `client.rs`) and they all set the limits already, so this path was the only inconsistent one. The net diff against main is now purely additive. One new scheduler flag, `--grpc-client-max-message-size`, defaulting to the 16 MiB the scheduler already used, plus tests that the flag reaches the client config and that an `override_config_producer` still takes precedence over it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
