proposal
proposal copied to clipboard
grpc
A25: make backpressure threshold configurable
Thank you for your pull request. Before we can look at your contribution, we need to ensure all contributors are covered by a Contributor License Agreement.
After the following items are addressed, please respond with a new comment here, and the automated system will re-verify.
- User @lidavidm isn't covered by a CLA. They will need to complete the form at https://identity.linuxfoundation.org/projects/cncf
Regards, CLA GitHub bot
Just to follow up here, is there any update on whether this looks reasonable?
@ejona86 is OOO at the moment; @dfawley did you have a chance to take a look?
Just to follow up again...is this something gRPC wants to support? IMO, manual backpressure like what Alluxio did is fairly ugly (since then we have three levels of flow control in play!), and we'd like to avoid that in Arrow Flight.
One more time...we are still interested in this feature, and would appreciate any feedback (even if it's a no). Thanks!
@a11r, PTAL. The proposal seems reasonable and inline with the discussion in grpc/grpc-java#5433.
@a11r @ejona86 @dfawley It'd be great if we could figure out the next steps forward on this. This has been identified as having a significant impact on the performance of Arrow Flight in Java, which is being supported by more and more databases (such as Dremio, DuckDB, InfluxDB, Trino, PostgreSQL ...)
We are still interested in this as well for the same reason (I am the ghost/original reporter).
It's unclear to me what the current state of affairs is, here, vs. what is being proposed, since it's 5 years later.
Currently, this threshold is hard-coded in each implementation[^go-threshold][^java-threshold], and is set to a relatively small value. This limits throughput for certain applications, particularly ones dealing with large message payloads, for which the default threshold would cause backpressure immediately upon sending any message.
Go has automatic BDP detection that should optimize the setting, and users shouldn't have to think about it in order to maximize performance with one rare exception: short lived connections that won't have time to auto-detect the optimal settings.
Go already has a knob for stream-level flow control (https://pkg.go.dev/google.golang.org/grpc#WithInitialWindowSize), though I wouldn't recommend using it except in that very rare case above, as it disables BDP estimation.
The Java issue that originally prompted me to report this 5 years ago is still open, though, and the DEFAULT_ONREADY_THRESHOLD flag identified back then still exists, with no way to adjust it.
@dfawley if I'm reading this right, Go has functionality for automating the backpressure threshold while in Java it is still a fixed flag that is non-configurable. Would the right way forward then be to add to Java similar autodetection features (not sure if this is feasible yet but trying to summarize and get consensus on what the next step is).
I think I misunderstood the scope of this gRFC in my last comment. Go and Java both measure the connection BDP and scale the window size accordingly. However, the local buffer that goes beyond the window is not configurable in either. We will discuss this internally soon and update this PR.
Thanks. For context, I was originally pushed into writing it this way: https://github.com/grpc/grpc-java/issues/5433 My concern has always been the local buffer size/threshold.
Hi @dfawley , wondering if you've been able to have the internal discussion about this proposal?
Sorry, I forgot to circle back here. We discussed this in some detail and decided it's fine to proceed with this work without needing a gRFC, on the basis that there are already many tuning knobs for similar things in all the languages that are different anyway. Sorry that this is in conflict with our earlier advice to write this up as a gRFC, but at least we're fine to proceed with implementation.