zio-grpc icon indicating copy to clipboard operation
zio-grpc copied to clipboard

Published 20 hours ago • verified publisher icon scalapb

Backpressure support

Open fillson-shady opened this issue 4 years ago • 6 comments

Use gRPC built-in flow control to support backpressure.

fillson-shady avatar Nov 05 '20 08:11 fillson-shady

Hello again! I've just added:

  • tests for all backpressure scenarios
  • protect sendMessageWhenReady with Semaphore (because concurrent access can override other fiber promise)
  • another isReady check before sending a message (according to gRPC doc)
  • small typo fix in another test

fillson-shady avatar Nov 13 '20 12:11 fillson-shady

Thanks for working on this. This looks really good. My main concern would be the need to support it (due to the subtleties and potential race conditions) so I'm looking for a way to better contain the change. Another, secondary concern, is potential impact on throughput.

thesamet avatar Nov 15 '20 01:11 thesamet

@thesamet thanks for the quick reply!
To keep both implementations of the calls I've splitted ZCall into two traits: ZCall and ZCallBase. Now it is possible to change parent trait for ZCallBase to switch it behaviour (with or without backpressure).
I've also mark BackpressureTestServiceSpec as ignored to keep tests passing for now.

fillson-shady avatar Nov 16 '20 09:11 fillson-shady

Would love to zio-grpc gain see back pressure support. Any blockers to getting this PR merged?

vyshane avatar Dec 16 '20 11:12 vyshane

Hey @fillson-shady / @thesamet , was this abandoned? I was surprised to find out that zio-grpc is not back pressure aware.

oridag avatar Mar 08 '22 21:03 oridag

I am hesitant on getting this merged due to the additional complexity to support and maintain this. Given that grpc-java doesn't provide this, I think that the need for backpressure support is limited or can be handled at the application layer.

thesamet avatar Mar 08 '22 23:03 thesamet