support CompletionStage in SynchronizationContext (executeLater)

[pjfanning]

Is your feature request related to a problem?

Idea would be to add an executeLater(CompletionStage<?> future) method.
SynchronizationContext would store a CompletionStage variable and each new executeLater would be composed onto the existing CompletionStage so that they all run in order.

Describe the solution you'd like

It might be useful to keep the Queue of Runnables but after the first CompletionStage is added via executeLater, all subsequent executes (synchronous or asynchronous) would be added to the first CompletionStage so that these tasks continue to run in order (that they were added).

Describe alternatives you've considered

In #11662, we are being pushed towards using SynchronizationContext instead of using Scala asynchronous constructs like Future, ZIO, etc. It is feasible to wrap these Scala constructs as Java CompletionStages so that the API in grpc-java remains Java specific.

Additional context

The aim is not to force pekko-grpc and other non-Java frameworks to have to rewrite their async code.

[ejona86]

SynchronizationContext would store a CompletionStage variable and each new executeLater would be composed onto the existing CompletionStage so that they all run in order.

It fundamentally moves where all execution runs? No. That's a very scary change and counter to the API, which is closer to a lock in being foundational.

The aim is not to force pekko-grpc and other non-Java frameworks to have to rewrite their async code.

I don't understand what rewriting you are trying to avoid.

[pjfanning]

I said this #11662: we have existing non gRPC code for service discovery. It is Scala code that uses Scala Futures.
We also want to have a pekko-grpc framework that uses grpc-java but where we mix in some of our separate non-gRPC modules. If grpc-java starts throwing exceptions every time we use a Scala Future, then we are sort of stuck. Things worked up to grpc-java 1.67.1. I am stuggling to understand why you want to force everyone to use SynchronizationContext. Lots of developers like to write asynchronous code - they believe it helps them to solve real world problems.

I'm hoping that #11666 solves the immediate issues but what concerns me are the comments that we will see independent gRPC frameworks built on top of grpc-java being forced to rewrite their code to use SynchronizationContext in future.

[ejona86]

I don't understand. We're talking about the NameResolver specifically. It sounds like you're talking about gRPC in general. You can do async; that's fine. But calls into NameResolver.Listener would need to be from within SynchronizationContext. I don't see the problem you're describing.

The reason is simple: it reduces bugs. PekkoDiscoveryNameResolver.scala should be managing some state within refresh() to avoid looking up too frequently. Right now every call to refresh() starts a lookup; that means even if a lookup is already is in progress it will start another lookup. If you had the appropriate state, you'd need synchronization for that state.

Also, we can't easily run without the SynchronizationContext with the newer onResult2(). There's a return value of whether the results were accepted. That requires coordination with other components that use the SynchronizationContext. We looked at making that async-capable but 1) NameResolvers don't actually benefit from the capatibility (it is trivial to call within the SynchronizationContext) and 2) an async API would dramatically increase the complexity and likelihood of bugs in NameResolvers. NameResolvers must be relatively simple, because there will be many of them implemented outside gRPC.

[pjfanning]

I'll close this

[ejona86]

If there are conveniences we can add that convert to other APIs, we are interested. This one was about changing where synchronization context runs, which is not a convenience but fundamentally a new thing. For example, maybe it'd be useful to have Runnable and Consumer wrappers where they run in the synchronization context.

I'm also not very practiced with CompletionStage and don't know Scala, so I don't inherently understand how it helps things in this case. I thought part of the point of CompletionStage callbacks is they could happen on multiple threads simultaneously. And since you can add things after the stage has completed, it seems unlikely the implementation guarantees they are run serially (ListenableFuture for example doesn't guarantee this).