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

Support for gRPC compression #46

Open
tmcgilchrist opened this issue Oct 31, 2023 · 4 comments
Open

Support for gRPC compression #46

tmcgilchrist opened this issue Oct 31, 2023 · 4 comments

Comments

@tmcgilchrist
Copy link
Collaborator

Provide a minimal implementation of compression using gzip (which is supported in Go providing a popular interoperability target). There is a general spec outlining how compression should work https://github.com/grpc/grpc/blob/master/doc/compression.md. The decompress library looks promising, it provides gzip and zlib plus it is fairly popular and implemented in pure OCaml.

Bonus feature would be to design an API such that compression is extensible and library users can provide their own compression algorithms.

@joprice
Copy link

joprice commented Jan 29, 2024

I tried using a service with grpcurl but the underlying library it uses by default sends a grpc-accept-encoding: gzip header. This results in the error 406 (Not Acceptable); malformed header: missing HTTP content-type due to the check here

else respond_with `Not_acceptable)
.

According to https://grpc.io/docs/guides/compression/#compression-method-asymmetry-between-peers, in the case of an unsupported compression type, an unimplemented error should be returned. https://github.com/grpc/grpc-go/blob/5051eeae537cb2839dd499e1a63a141098a3a03a/server.go#L1316.

As for the compression itself, I took a stab at it and got it partly working, decompressing an inbound body https://github.com/dialohq/ocaml-grpc/compare/main...joprice:ocaml-grpc:gzipSupport?expand=1#diff-2592ab6cc5a62541ce6f428cc52bed5bc738dbbdfbb6be87042a0fb5bfe390b2R56. That branch also has the Unimplemented response on it, along with some other local changes I had made for error handling that I can clean up if this general approach makes sense.

@tmcgilchrist
Copy link
Collaborator Author

That would be great @joprice this library should interop correctly against other implementations.
Have a look at the Typed RPC PR #55 that should be the new API going forward.

You might also be interested in https://github.com/tmcgilchrist/ocaml-grpc/tree/fixes which has the initial support for running tests against the Go gRPC bindings. That is what I would eventually like to have but as your found out there are plenty of corner cases where ocaml-grpc doesn't do the right thing.

@joprice
Copy link

joprice commented Jan 29, 2024

I'll check both of those out. I actually just got the full end to end working https://github.com/dialohq/ocaml-grpc/compare/main...joprice:ocaml-grpc:gzipSupport?expand=1#diff-2592ab6cc5a62541ce6f428cc52bed5bc738dbbdfbb6be87042a0fb5bfe390b2R49-R69.

I added a codec type and the negotiation of client and server encodings. It combines the two together into a new codec to support asymmetric encoding. I added a parameter to the server v builder function that takes in a list of supported compression implementations. The implementations could be moved to separate libraries to avoid having to build things unnecessarily, and since it's just a record of functions, you can provide your own implementation easily.

@joprice
Copy link

joprice commented Jan 29, 2024

Here's some more docs I found on compression. The last one looks especially useful as it has some nice flowcharts that can be used to validate the implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants