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

Please add support for server compression #1823

Open
jlecomte opened this issue Jun 21, 2021 · 7 comments
Open

Please add support for server compression #1823

jlecomte opened this issue Jun 21, 2021 · 7 comments

Comments

@jlecomte
Copy link

jlecomte commented Jun 21, 2021

Is your feature request related to a problem? Please describe.

Not really, other than the fact that I cannot migrate my server from grpc to @grpc/grpc-js until @grpc/grpc-js supports compression. Our clients (millions of them...) send messages compressed (gzip) in a bi-directional stream. Also, @grpc/grpc-js does not return an UNIMPLEMENTED error when the client sends the grpc-compression header with a value other than identity...

Describe the solution you'd like

Something along the lines of what is implemented in https://www.npmjs.com/package/grpc-server-js (I just perused through that code and found out that they already support compression) It also looks like this has been partially implemented in compression-filter.ts already... And I see a lot of TODO(cjihrig) in the code to point to the areas that should be modified.

Describe alternatives you've considered

Keep using the grpc package for now, even though it explicitly says that it has been deprecated. Compression is a major feature IMO, so maybe the deprecation was a little hasty?

@murgatroid99
Copy link
Member

We have prioritized feature work based on what features we were aware of our users using. The grpc-js project has existed for 3 years and as far as I know this is the first time someone has mentioned server compression, which is why we have not implemented it. The only part of compression that is fully implemented is client-side parsing of compressed messages, because we knew of users that needed that.

It looks like the server does have a bug where it assumes that all messages it receives are uncompressed, without checking. Technically, the server is not required to return UNIMPLEMENTED as soon as it gets a grpc-compression header other than identity. That client can subsequently send every message uncompressed, and if it does, the server can accept them.

@murgatroid99 murgatroid99 self-assigned this Jun 22, 2021
@jlecomte
Copy link
Author

Yup, understood on the prioritization. I only recently started looking at the grpc-js project. I wish I had done so sooner to let you know of my need. Thanks for the help!

@merlinnot
Copy link
Contributor

If I only knew user feedback is needed, I'd open an issue a long time ago 🙂 Me and other devs at my company are waiting for it ever since we switched from grpc. We just just accepted lack of it for now.

We weren't putting pressure on it because we've seen continuous progress on other features that were useful to us, thank you for that. And I still think there might be more important ones, such as addressing the performance differences (raised in other issues), so I get your point about priorities - I'm dropping this message here just to let you know it would be useful for more people.

@murgatroid99
Copy link
Member

I'm sorry for not communicating that more effectively. I mentioned it in the 1.0 announcement blog post, but I don't know how many people saw that. Other than that, I expected that people would say something if a feature they needed was missing.

@Lutzifer
Copy link

We also have a use case for this feature in our projects.

@murgatroid99
Copy link
Member

Compression support in grpc-js has been expanded in the 1.5.x release: clients can now send compressed messages, and servers can now receive compressed messages. Support for sending compressed messages from servers is still pending.

@jlecomte
Copy link
Author

I verified that our existing clients can talk to a test server after it was upgraded to grpc-js@1.5.0, it seems to work well. Thanks a lot @murgatroid99! I am holding off on phasing out our existing server implementation, which uses grpc-node, because of #2026. Once that issue (which is unrelated to this one) is out of the way, I think I'll be in great shape! :)

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

No branches or pull requests

4 participants