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

server: expose API to set send compressor #5744

Merged
merged 13 commits into from
Jan 31, 2023

Conversation

jronak
Copy link
Contributor

@jronak jronak commented Oct 26, 2022

Fixes #5792

Follow-up of #5541 and #2786

Description:
In the current implementation, the server compresses the send payload when the client sends a compressed request payload or explicitly uses the grpc.RPCCompressor option. It cannot be controlled by the user to either use a different compression algorithm or disable compression (identity).

This PR exposes an API grpc.SetSendCompressor(ctx, name) similar to the grpc.SetHeader(ctx, md), which allows the user to set the compressor to be used for compressing response payload (sent back to the client). The provided compressor is used when the below conditions are met:

  • the compressor name provided must be registered via encoding.RegisterCompressor in the server.
  • compressor name exists in the client advertised compressor names sent in grpc-accept-encoding metadata.

Other gRPC platforms expose similar APIs, for example grpc-java.

RELEASE NOTES:

  • server: expose SetSendCompressor API to set send compressor name

@easwars easwars added the Type: Feature New features or improvements in behavior label Nov 1, 2022
@easwars easwars added this to the 1.51 Release milestone Nov 1, 2022
@easwars easwars self-assigned this Nov 1, 2022
@zasweq zasweq modified the milestones: 1.51 Release, 1.52 Release Nov 8, 2022
Copy link
Contributor

@easwars easwars left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sincere apologies for the delay in getting to this PR.

server.go Show resolved Hide resolved
server.go Outdated Show resolved Hide resolved
internal/transport/transport.go Outdated Show resolved Hide resolved
internal/transport/transport.go Outdated Show resolved Hide resolved
server.go Outdated Show resolved Hide resolved
server.go Show resolved Hide resolved
server.go Outdated Show resolved Hide resolved
server.go Outdated Show resolved Hide resolved
stream.go Show resolved Hide resolved
@easwars easwars assigned jronak and unassigned easwars Nov 18, 2022
@jronak jronak force-pushed the set-send-compressor branch from e890733 to b19a201 Compare November 21, 2022 17:34
@jronak
Copy link
Contributor Author

jronak commented Nov 21, 2022

@easwars thanks much for the review, I have addressed the comments. Please review when you get a chance

@jronak jronak force-pushed the set-send-compressor branch from 4128531 to e05f066 Compare November 21, 2022 17:39
@jronak jronak requested a review from easwars November 22, 2022 05:06
@easwars easwars assigned easwars and unassigned jronak Nov 23, 2022
internal/grpcutil/compressor.go Outdated Show resolved Hide resolved
server.go Outdated Show resolved Hide resolved
server.go Outdated Show resolved Hide resolved
internal/transport/http2_server.go Outdated Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented Dec 7, 2022

This PR is labeled as requiring an update from the reporter, and no update has been received after 6 days. If no update is provided in the next 7 days, this issue will be automatically closed.

@github-actions github-actions bot added the stale label Dec 7, 2022
@jronak jronak force-pushed the set-send-compressor branch from e05f066 to 8be817f Compare December 7, 2022 20:30
internal/transport/http2_server.go Show resolved Hide resolved
server.go Outdated Show resolved Hide resolved
server.go Outdated Show resolved Hide resolved
test/end2end_test.go Show resolved Hide resolved
test/end2end_test.go Show resolved Hide resolved
internal/transport/http2_server.go Outdated Show resolved Hide resolved
@jronak jronak requested a review from dfawley January 23, 2023 15:30
@arvindbr8 arvindbr8 assigned dfawley and unassigned jronak Jan 24, 2023
Copy link
Member

@dfawley dfawley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM modulo the one testing nit. Thanks!

test/end2end_test.go Outdated Show resolved Hide resolved
@dfawley
Copy link
Member

dfawley commented Jan 25, 2023

@easwars please take another pass on this after the latest set(s) of changes.

@easwars easwars removed their assignment Jan 25, 2023
@arvindbr8 arvindbr8 modified the milestones: 1.53 Release, 1.54 Release Jan 26, 2023
Copy link
Member

@dfawley dfawley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see previous comment re: adding a comment. (Updating state.)

@dfawley dfawley merged commit 0954097 into grpc:master Jan 31, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Feature New features or improvements in behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Server Compression Asymmetry
5 participants