envoyproxy / java-control-plane

Java implementation of an Envoy gRPC control plane
Apache License 2.0
293 stars 136 forks source link

Code generation with Reactive gRPC #100

Closed linux-china closed 3 years ago

linux-china commented 5 years ago

Reactive gRPC makes gRPC reactive and easy to implement logic. Anybody agrees with this feature? It's easy, and just add reactive-grpc protocPlugin in api module. Details link: https://github.com/salesforce/reactive-grpc

joeyb commented 5 years ago

Reactive gRPC is definitely a cool project, but I'm not sure that we want to force consumers of this project to pull in the additional dependencies. If we can figure out a clean way to produce separate builds (one with reactive-grpc and one without) then I think it'd be fine to add, but I suspect the maven config needed for that might get pretty messy.

linux-china commented 5 years ago

How about a demo to help developers to implement Java Envoy control plane server with https://github.com/LogNet/grpc-spring-boot-starter, reactive-grpc and spring boot? Most developers choose Spring Boot and grpc-spring-boot-starter to implement gRPC services.

jakubdyszkiewicz commented 5 years ago

@linux-china I have a strong feeling that introducing reactive-grpc would require a complete rewrite of java-control-plane.

The flow of the project is to receive an xDS request, wait for Snapshot to change, respond. reactive-grpc in the server module will add no value itself because Snapshot changes are not reactive, so then the only thing that would change is semantic of DiscoveryServer. To make Snapshot changes reactive you would have to rethink Watches concept (most likely they will become irrelevant). To have backpressure that can hold back your source of Snapshot changes you probably want to have Snapshot#setSnapshot to return Flux. Overall, Fluxes all over the place. The code between reactive and non-reactive version would be hard to share.

That's why I think this is an interesting idea, but maybe for another project? This way you also won't be constrained by keeping the compatibility with non-reactive components.

slonka commented 3 years ago

I agree with Jakub, this is not something we will use in java-control-plane.