grpc / grpc-go

The Go language implementation of gRPC. HTTP/2 based RPC
https://grpc.io
Apache License 2.0
21.08k stars 4.38k forks source link

How to check if server implements bidirectional stream method before sending data #5910

Closed ItalyPaleAle closed 1 year ago

ItalyPaleAle commented 1 year ago

We (Dapr) have a gRPC API that uses a bi-directional stream:

service ServiceInvocation {
  rpc CallLocalStream (stream InternalInvokeRequestStream) returns (stream InternalInvokeResponseStream) {}
}

This is a new API that we're adding, and we need to maintain backwards-compatibility so new clients (which implement CallLocalStream) can invoke old servers (which do not implement CallLocalStream), falling back to a unary RPC in that case.

The problem we have is that when we create the stream on the client, with:

stream, err := clientV1.CallLocalStream(ctx, opts...)
if err != nil {
    return nil, err
}

Even when the server does not implement CallLocalStream, err is always nil. We get an error (with code "Unimplemented") only after we try to receive a message from the stream.

This is a problem for us because by the time we (try to) receive a message from the stream, we have already sent data to the server, and that works without errors. That data comes from a readable stream, which we end up consuming along the way, so the data doesn't exist anymore if we then need to fall back to a unary RPC.

Is there a way to determine earlier if the server implements a bidirectional stream method? Ideally, without having to make a "ping" call which would add latency.

arvindbr8 commented 1 year ago

Hi @ItalyPaleAle,

No, in all implementations of gRPC, there is no way of knowing if a method is implemented on the server. In a non-homogenous service setup, the client conn (based on the load balancing config) will have subconns to servers with different versions. Now there is a possibility of the subconn just working the next time another RPC is made. Currently there is no way to know if the method is implemented without looking for the unimplemented error code.

Hope this helps

dfawley commented 1 year ago

This is a problem for us because by the time we (try to) receive a message from the stream, we have already sent data to the server, and that works without errors.

I think I see what you're saying here. The only way to avoid doing those sends would be to have the server either send an empty response message at the start, which you Recv before sending, or to have the server send headers (ServerStream.SendHeader()) and have the client receive them (ClientStream.Header()). In reality it's probably fine to have the client send its request messages at the start and avoid the extra round trip -- those sends will start to error once the server responds with UNAVAILABLE anyway.

But @arvindbr8 is right that if the streaming RPC fails and you are in a heterogeneous environment, you don't necessarily know where your next RPC will be routed -- it could actually be routed to a server that supports the streaming version. The unary RPC will always succeed as a fallback, but IMO it's better to wait for all your servers to be updated before using the streaming RPC in any clients.

ItalyPaleAle commented 1 year ago

Yes, I fully understand @arvindbr8's point and it makes sense (although, on the point on load balancing, that could be simplified by not making it persistent in the connection, so on each call the target would have to report whether the method is supported... but this would add latency).

In reality it's probably fine to have the client send its request messages at the start and avoid the extra round trip -- those sends will start to error once the server responds with UNAVAILABLE anyway.

The only time when this isn't fine is if the caller doesn't have a way to "replay" a request, for example to fall back to another gRPC method. This is the case for us in Dapr, for example, where the request's data is a stream and we can't replay it (unless we buffer it in memory).

We found a way around the problem that is based around doing a controlled rollout and other things that are specified to our project.

dfawley commented 1 year ago

on each call the target would have to report whether the method is supported

This would need to be part of your streaming RPC's protocol, as gRPC itself does not do this.

We found a way around the problem that is based around doing a controlled rollout and other things that are specified to our project.

That sounds like the best way to address this to me!