yarpc / yarpc-go

A message passing platform for Go
MIT License
401 stars 101 forks source link

grpc: apply compression only on outbounds with compression enabled #2199

Closed jronak closed 1 year ago

jronak commented 1 year ago

When multiple outbounds exist for the same host port, compression is incorrectly enabled for all even though just one outbound has compression enabled. This happens as yarpc maintains at most one connection to any host port, multiple outbounds share the same connection which has compression enabled for all the RPCs.

This PR fixes it by disabling connection level compression (dial option) and enabling compression per RPC call by passing the grpc call option in the outbound call.

codecov[bot] commented 1 year ago

Codecov Report

Base: 85.36% // Head: 85.33% // Decreases project coverage by -0.03% :warning:

Coverage data is based on head (e4022c8) compared to base (7a4a0a8). Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev #2199 +/- ## ========================================== - Coverage 85.36% 85.33% -0.03% ========================================== Files 270 270 Lines 15465 15471 +6 ========================================== + Hits 13201 13202 +1 - Misses 1844 1847 +3 - Partials 420 422 +2 ``` | [Impacted Files](https://codecov.io/gh/yarpc/yarpc-go/pull/2199?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yarpc) | Coverage Δ | | |---|---|---| | [transport/grpc/config.go](https://codecov.io/gh/yarpc/yarpc-go/pull/2199?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yarpc#diff-dHJhbnNwb3J0L2dycGMvY29uZmlnLmdv) | `97.29% <100.00%> (ø)` | | | [transport/grpc/options.go](https://codecov.io/gh/yarpc/yarpc-go/pull/2199?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yarpc#diff-dHJhbnNwb3J0L2dycGMvb3B0aW9ucy5nbw==) | `94.53% <100.00%> (+0.17%)` | :arrow_up: | | [transport/grpc/outbound.go](https://codecov.io/gh/yarpc/yarpc-go/pull/2199?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yarpc#diff-dHJhbnNwb3J0L2dycGMvb3V0Ym91bmQuZ28=) | `77.33% <100.00%> (+0.22%)` | :arrow_up: | | [dispatcher\_startup.go](https://codecov.io/gh/yarpc/yarpc-go/pull/2199?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yarpc#diff-ZGlzcGF0Y2hlcl9zdGFydHVwLmdv) | `91.07% <0.00%> (-1.79%)` | :arrow_down: | | [peer/hashring32/internal/hashring32/hashring32.go](https://codecov.io/gh/yarpc/yarpc-go/pull/2199?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yarpc#diff-cGVlci9oYXNocmluZzMyL2ludGVybmFsL2hhc2hyaW5nMzIvaGFzaHJpbmczMi5nbw==) | `96.21% <0.00%> (-1.09%)` | :arrow_down: | | [internal/observability/graph.go](https://codecov.io/gh/yarpc/yarpc-go/pull/2199?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yarpc#diff-aW50ZXJuYWwvb2JzZXJ2YWJpbGl0eS9ncmFwaC5nbw==) | `93.12% <0.00%> (-0.63%)` | :arrow_down: | | [transport/tchannel/peer.go](https://codecov.io/gh/yarpc/yarpc-go/pull/2199?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yarpc#diff-dHJhbnNwb3J0L3RjaGFubmVsL3BlZXIuZ28=) | `97.67% <0.00%> (+1.16%)` | :arrow_up: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yarpc). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yarpc)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.