[Java] Ability to register additional grpc services with FlightServer

[aiguofer]

Describe the enhancement requested

We want to add the gRPC health endpoint to our server. It looks like the ability to do this was added to the go server with this PR.. It would be great to have something similar for the Java server.

Component(s)

Java

[lidavidm]

Use FlightGrpcUtil for full control; we are trying not to expose gRPC APIs directly from core APIs.

[aiguofer]

Use FlightGrpcUtil for full control; we are trying not to expose gRPC APIs directly from core APIs.

@lidavidm I've been looking at FlightServer and FlightGrpcUtil... I'm not sure how FlightGrpcUtil is helpful here? Are you saying we should re-implement our own entire FlightServer builder to add more services?

[lidavidm]

Yes, and FlightServer does not do all that much for you in the first place, so I don't really want it to be a giant wrapper around the gRPC server builders...just use the gRPC ones directly.

[aiguofer]

@lidavidm so FlightServer is not the recommended way to set up a Flight server? Maybe it should be deprecated and moved to examples instead. Seems like a simple change like this would add what we need, but if you prefer not supporting FlightServer we can just copy-pasta what we need.

[lidavidm]

It is still recommended, but if you want to control every gRPC knob we can't really support that in the core API. Again, the main constraint is not leaking gRPC implementation details (as best as possible) into the core API since we are still working on supporting non-gRPC transports.

[lidavidm]

Of course, the C++/Python APIs expose some generic hooks for this kind of customization in the core API. A proposal for a similar hook in Java would be welcome.

[aiguofer]

I believe the Go API does as well. Would the above commit seem like a reasonable proposal? I can make a PR and move the conversation there.

[lidavidm]

Not quite, directly exposing gRPC is specifically what we have tried to avoid.

I'm thinking something like (directly off the top of my head, not sure if this compiles)

<T> Builder withTransportHook(Consumer<T> hook) {
    if (grpcBuilder instanceof T) {
        hook.accept((T) grpcBuilder);
    } else {
        // raise exception
    }
}

// ...

FlightServer.builder().withTransportHook<NettyServerBuilder>((nsb) -> { nsb.registerService(...); })...;

(Go and Rust forego flexibility and directly tie themselves to the gRPC API: that means that a project like Flight-UCX is not possible in those languages, not without rewriting your Flight client/service. That's what we are trying to avoid.)

[aiguofer]

Gotcha, that makes sense! I'll take a stab at it!

[lidavidm]

@jiezhang would this sort of API solve your concern in #34612?

[aiguofer]

@lidavidm I've been struggling with generics and how to store the hook from withTransportHook(Consumer<T> hook) for later use in .build().

Is it safe to assume in the future all builders will be either AbstractServerImplBuilder or ServerBuilder? That would make it a lot easier to do something like:

    public Builder withTransportHook(Consumer<ServerBuilder<?>> hook) {
      this.transportHook = hook;
      return this;
    }

and in build():

      transportHook.accept(builder);

[lidavidm]

I think that's fine for this implementation. We can refactor when we have other implementations.

[aiguofer]

@lidavidm got an initial PR out. Let me know what you think about the API before I go on making the unit tests and docs changes. I tested this with our server and I can add both Health Check and Reflection services.