wolf4ood / gremlin-rs

Gremlin Rust
Apache License 2.0
106 stars 30 forks source link

Exposed websocket max_message_size through ConnectionOptions #189

Closed criminosis closed 1 year ago

criminosis commented 1 year ago

Hey @wolf4ood I bumped into what seems to be an implicit default that wasn't exposed by this crate. These functions:

https://github.com/wolf4ood/gremlin-rs/blob/master/gremlin-client/src/connection.rs#L51 https://github.com/wolf4ood/gremlin-rs/blob/master/gremlin-client/src/aio/connection.rs#L133-L136

Appeared to be leveraging tungstenite's default websocket settings by using the method that didn't specify a websocket config:

https://github.com/snapview/tungstenite-rs/blob/371f8230444e209cc37ec481e94d621eddf5f0af/src/tls.rs#L180 https://github.com/sdroege/async-tungstenite/blob/2365647978412f47e681d498f25b8469f4355f11/src/tokio.rs#L267

This None then becomes the defaults defined by tungstenite: https://github.com/snapview/tungstenite-rs/blob/master/src/protocol/mod.rs#L39-L70

I was having I/O issues bumping into these implicit limits. Fwiw they are "supposed" to be configured to the target graph provider's max_message_size, and JanusGraph at least sets its default to just 65k, obviously way less than the 64MiB implicitly being used from tungstenite.

I tried to mimic prior builders / config patterns, but happy to make any changes.

If accepted & merged mind cutting a new release?

criminosis commented 1 year ago

@wolf4ood Thank you!