Allow the queued byte threshold for a Stream to be ready to be configurable

[jduo]

• on clients this is exposed by setting a CallOption
• on servers this is configured by calling a method on ServerCall or ServerStreamListener

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: jduo / name: James Duong (1a6b599bdfbe1cba5e3e3fb856c7ac5a117f405c, 9024da00ca71882b45d9d0f0b62b29fbacbf9ef3, 23bdd1f79c604f627379bc298168fbf59aba69ab, 03d81e7a7fa3f33c7b0a65d6bd9df2fc87a0c33d, 63a454dcbff434e147bb6ba4c8b9443c990a4099, ea79d99b9e366c858267b132585f764812a15539, 673e2cefbf5ddca7694ecdfd35c60398fdb07ce6, 7cf43d2dfe0e0f0f5141e7235610f3ad2d447103, f0cf7267074723c7c501bc41bb457bff481ebc6b, 3c78fe042e109a63cc4de7ef837dccee859a3f99)

[jduo]

This is a draft to demonstrate the idea discussed in #5433 @ejona86:

• It's a bit hard to tell what implementations of Stream and ServerCall need overrides for setOnReadyThreshold(). Some guidance here would be helpful
• I'll work on adding unit tests and fixing up the documentation. If there are pointers on where to write the tests that'd be great.

[jduo]

Hey @ejona86 , any more feedback? Particularly about the server side.

[jduo]

Hi @ejona86 , checking in on if there's an update.

I'm hoping to get this in March, the earlier the better. The Arrow project has a 3-month release cycle and there's a code freeze coming up at the end of March, and I'll be away for a from March 23rd until April.

Thanks!

[ejona86]

gRPC's next release is April 2nd, so the release cycles may not align for you :frowning_face:. Even if we shifted that a few days, it sounds like that might not be enough for you.

[jduo]

gRPC's next release is April 2nd, so the release cycles may not align for you ☹️. Even if we shifted that a few days, it sounds like that might not be enough for you.

Thanks for the update. Yeah more realistically this will get into Arrow 17. We can still have Arrow Flight server developers use this before it officially makes it into Arrow though. Server developers can create the gRPC server themselves, configure this option, then apply the Arrow Flight stub.

[jduo]

I've added server tests now. I don't have tests for the TransmitStatusRuntimeExceptionInterceptor since I wasn't really sure how to use it. Hopefully this covers everything @ejona86

[jduo]

Hi @ejona86 , just wondering if you had a chance to look. I'll be away at the end of the week until the 3rd so it'd be great if we could get this wrapped up. Maybe this could get into 1.63 if it all looks OK.

[ejona86]

I'll be away at the end of the week until the 3rd

At this point is this "I'll be gone tomorrow"? If so (if you have no time), I'd accept this with the threshold checks in OkHttpClientStream and the like. But we do need to get some of the other easier things resolved.

[jduo]

I'll be away at the end of the week until the 3rd

At this point is this "I'll be gone tomorrow"? If so (if you have no time), I'd accept this with the threshold checks in OkHttpClientStream and the like. But we do need to get some of the other easier things resolved.

I've covered the javadoc-related ones mostly now. If you can give some more guidance around the AbstractClientStream constructor I can pick that up too.

Thanks!

[jduo]

The failure in JDK 8 testing looks unrelated. Is it flakey? io.grpc.servlet.jakarta.UndertowInteropTest > pingPong FAILED java.lang.AssertionError at Assert.java:89

[ejona86]

Yeah, UndertowInteropTest.pingPong is likely a flake. I've restarted it, but I wouldn't be concerned. We can still merge if it fails.

[jduo]

Looks like there are some Android classes that need to be updated (SingleMessageServerStream and MultiMessageServerStream).

These seem to use a different class hierarchy than using a Transport so I'm not sure what the implementation entails. For now I'm just going to make setOnReadyThreshold() a no-op.

Unfortunately my Android Studio installation isn't working right so I'm having difficulty seeing Android build issues locally and am only spotting them through Kokoro.

[ejona86]

No-op is fine. Since the Android build requires us to add the "kokoro:run" label for each change, it might make sense for us to fix it up instead of you. Do you have a change there in-flight, or is it safe to push to your branch?

[jduo]

No-op is fine. Since the Android build requires us to add the "kokoro:run" label for each change, it might make sense for us to fix it up instead of you. Do you have a change there in-flight, or is it safe to push to your branch?

I added no-ops for the two ServerStream implementations in binder. No other changes in-flight. It'd be great if you could finish up the Android build.

[ejona86]

I just built your "Fix Android build" commit locally and it worked fine. So the Android CI should turn green this time.

[ejona86]

@jduo, thank you! This made it in before the 1.63 branch cut, so it will be in the next release. (The branch cut was scheduled for Tuesday, but it had been forgotten about a bit so hasn't been done yet.)

[jduo]

@jduo, thank you! This made it in before the 1.63 branch cut, so it will be in the next release. (The branch cut was scheduled for Tuesday, but it had been forgotten about a bit so hasn't been done yet.)

Thanks for your help getting this in for our timeline @ejona86 @larry-safran !