Protobuf: RPC service multiple parameter support, For existing services which uses proxy instead of stub

[supercharger]

Current protobuf RPC workflow is that, we start with writing .proto file, and then generate language specific stub and then build the client/service on top of stub. There is not much value in supporting multiple parameters for method calls as we can instead wrap multiple method parameters with another new type. But, it is required for following usecase.

Let's say you have existing interfaces and java classes ( return types, method parameters). Building stubs and IDL files, using the stubs ( for which you already have source) are unintuitive, and extra efforts to change the existing services.

For ex: Let's say you have following existing service.

public interface ChatService {
    public StatusResponse sendmessage(ChatMessage msg);
}
public class ChatMessage {
     private String msg; 
}
//ignored getters setters, other class  StatusResponse for brevity

Now, As we don't want to generate stub for which we already have code. We can mark them with annotations. And we call the RPC methods directly on the interface with the help of Proxy ( Java Proxy in JVM languages)

@GrpcService
public interface ChatService {
      @GrpcMethod
     public StatusResponse sendmessage(ChatMessage msg);
}

A similarly implemented on facebook thrift through project "Facebook swift" . Following are example similar to above code.
_Parameter or return type_ : https://github.com/facebook/fbthrift/blob/master/thrift/tutorial/java/swift/client/src/main/java/com/facebook/swift/exampleclient/ProcessedStringResults.java
_Service_: https://github.com/facebook/fbthrift/blob/master/thrift/tutorial/java/swift/client/src/main/java/com/facebook/swift/exampleclient/ExampleService.java

And with above ChatService, We would make calls as if they are local methods without changing the existing services. But, the calls are _routed through the proxy_.

ChatService chatService = grpcClientManager.get(ChatService.class).build();
chatService.sendmessage(msg);

Above implementation can be easily built on top protobuf and grpc (even with multiple parameters). But, the problem comes when we have multiple parameters, solution of _wrapping the input parameters_ with another class doesn't work because the whole point of going with proxy based implementation is we don't want to change the existing services by _introducing new types and wrapper methods_. While this limitation of single parameter can be worked around grpc-java, if you think from the future tools of protobuf this is very required feature. I did the java proxy implementation here https://github.com/supercharger/grpc-proxy ex: client call with proxy

In Future, We might have community built tools around protobuf, which can generate .proto files given the interface. For ex: https://github.com/facebook/swift/tree/master/swift2thrift-generator-cli for thrift , And then with generated .proto desired language stubs can be further generated. But for such thing, protobuf has to support multiple parameters.

To summarize, without multiple parameter support language specific tools ( like https://github.com/facebook/swift/ for thrift) doesn't make much sense !!!

[mitramkm]

I'm facing similar problem, where I need support for multiple parameters.

[supercharger]

I did the java proxy implementation here https://github.com/supercharger/grpc-proxy with the help of https://github.com/grpc/grpc-java/issues/1196 , But, if I want to do cross language calls, it is very difficult without native support of protobuf.

[supercharger]

Any thoughts/update on this ?

[xfxyjwf]

Am I right that this is about using grpc+protobuf to implement existing RPC interfaces written in a specific language (in this case, Java)? From what I can tell, this is not a typical use case of grpc+protobuf. We are unlikely to change protobuf syntax to support it.

[supercharger]

Am I right that this is about using grpc+protobuf to implement existing RPC interfaces

Yes, Just plain existing interfaces and POJOs (classes) that needs to be exposed as RPC instead of generating them. But, that wouldn't work because of restriction method parameters.

From what I can tell, this is not a typical use case of grpc+protobuf.

I am checking as our business needed this case. Thanks for responding, I will close the issue.

[supercharger]

Facebook Swift https://github.com/facebook/swift kind of projects for protobuf doesn't make much sense without this feature.

[saturnism]

Quick ping @supercharger to see the status - it was closed but never heard back :)

[Alvcohen]

Apparently it was decided against this feature: https://groups.google.com/forum/#!topic/protobuf/9JNzY1BMNI4

However this would be a needed feature for many reasons:

1. Use case where it might be needed: I define an interface with many methods. Each method has a unique parameter set but there is a small number of parameters. With current functionality I have to make many umbrella messages to support all parameter variations for my methods.
2. Most developers who use different languages are used to writing methods with multiple parameters. Thus all modern RPC frameworks have this functionality. So when one chooses an RPC framework, lack of this functionality is considered a disadvantage.
3. The fact that implementing this feature has difficulties should not be considered when deciding upon its functional usefulness.
4. As of 2016 API simplicity and its conformance to expected behaviour should be an important requirement for overall API quality.
5. Besides the obvious problem when API looks unexpected to a developer there is another problem of difficulty of properly naming those umbrella messages. One can name them the same as a methods in which they are used, but then the name will have no useful information for the implementor. And creating a thoughtful name for such umbrella message is not always possible.

[BenjaminSalah]

@supercharger could you re-open this?

To the devs: Please reconsider this feature as it is essential. The argument that it would complicate the syntax does not make sense to me. I am not going to create a new message which is another class just because i can only pass a single message. If i have to do this kind of logic of creating a new message to stuff my other reusable messages for every such scenario i'm going to end up with tons of messages that aren't reusable.

[carl-mastrangelo]

@BenjaminSalah (I'm from gRPC team) Probably the biggest reason is the one from 4 from @Alvcohen above. Adding in multiple parameter support adds a lot of complication:

1. What order should parameters be encoded in? What should their names be? What order should the parameters be in the generated code? What happens if the order changes in the proto IDL? Be mindful that any answer you give will make someone else unhappy.
2. APIs evolve. Parameters that were once required become optional, and then eventually deprecated and removed. There isn't a good way to support API evolution with multiple parameters.
3. Services have been single parameter for as long as I can remember, so every single implementation is going to expect it. Changing this now would create an unnecessary schism between implementations.

I am not going to create a new message which is another class just because i can only pass a single message.

Why not? Creating messages is easy. Most services don't have many methods, so the total number of these messages is likely going to be tiny.

If i have to do this kind of logic of creating a new message to stuff my other reusable messages for every such scenario i'm going to end up with tons of messages that aren't reusable.

If you are worried about lots of conversion code between generated protos and your existing classes, there are two ways to address it. Proto reflection can be used to map fields between the generated classes and your own. Keep the field names the same and you'll only have to write the conversion code once. Alternatively, you can very easily write a protoc plugin to generate the classes you want.

I think if you described your use case more clearly, it would become clear multi parameter services aren't actually the best solution.

[xfxyjwf]

+1 to what @carl-mastrangelo says.

Google has been using protobuf as its RPC IDL for more than a decade and I have not heard of any claim that supporting multiple parameters is essential. For complexity we weigh API complexity much heavier than implementation complexity and in this case the complexity it adds to protobuf is real. I can imagine people having a hard time to figure out how to map multiple parameters to the wire and as a library used pretty much by everyone in Google, any slight complexity in the API will cost us actual engineering resources. And as Carl mentioned, using multiple parameters makes it harder to evolve your RPC interface and just for that reason we can classify multiple parameters as a protobuf bad practice because protobufs are meant to evolve constantly.

Overall I do not see much value it brings to protobuf, and the complexity it adds and the potential cost to adopt it are way higher.

[supercharger]

If anyone wants to do RPC with grpc, and want to use existing interfaces/classes. May be this POC will be helpful https://github.com/supercharger/grpc-proxy

[MaerF0x0]

I came here hoping to basically do

  rpc PutRecords(int32 recordset_id, stream Record) returns bool

I can get around it by putting the recordset_id in the message Record{ ...} definition. But then i'm not assured they're all set, and it maybe inefficiently sending the int32 across the wire N times ? (maybe not with packed) . Or I can do some heuristic like the first record contains the recordset_id and just ignore the rest?

Maybe I'm not using grpc as expected.

[Julio-Guerra]

@MaerF0x0 Same case here. What we do is using SendMsg()/RecvMsg() before starting streaming. It's like sending/receiving metadata at the RPC level.

But it's a shame that this can't be part of the signature and is thus not documented (it is by commenting the RPC)...

This feature is definitely needed. Every data-streaming requires sending/receiving some context first.

Related SO question: https://stackoverflow.com/questions/47358316

[karllessard]

I did not find an equivalent of SendMsg()/RecvMsg in the Java API for sending a different kind of message on the stream than the one set in the method signature.

Since Context is more oriented on key/value for authentication and stuff, what I ended up to do is to add the metadata into the message payload but only initialize it in the first one and leave it empty on the others.

[sheepdreamofandroids]

I had a similar case where I needed to send a tiny bit of context before streaming. In my case it was just two bits so I ended up creating 4 methods but still, ugly....
Maybe a special case where the main parameter is a Stream it could be optionally be prefixed with a head and suffixed with a tail?

[maniankara]

+1 this is very much needed feature. For now going ahead with inefficient work-around by @MaerF0x0

[carl-mastrangelo]

@MaerF0x0 & @maniankara & @sheepdreamofandroids You are not using it as expected. This is the right way to do this in proto3:

service Foo {
  rpc PutRecords(stream RecordRequest) returns (RecordResponse);
}

message RecordRequest {
  oneof msgtype {
    int32 recordset_id = 1;
    Record record = 2;
  }
}

message RecordResponse {
  bool success = 1;
}

Your client sends the first request with the recordset_id field set. Proto knows how to distinguish between which of the "One Of" fields have been set. You need to make your client send the id first, and then the records later. The server needs to check the order as well. Your proto makes it clear that only one of those should be set.

I'll write up the reasons why at some point, but this structure is WAAAY more maintainable. @xfxyjwf and I haven't called as much attention to it, but I'm going to do so now: It is very difficult to change APIs after they are implemented. Proto messages are designed to be backwards compatible so it makes sense to put all the argument complexity in a single message parameter. This wins you the most maintainability.

As a quick example, suppose recordset_id needs to be an int64 and not an int32. How are you going to change it? If the type was in the service definition like rpc PutRecords(int32 recordset_id, stream Record) returns bool, changing it would break all the compiled code for typesafe languages. Also, as you role out this change to all your clients and servers, they would all silently truncate the larger ids as they talked to each other. As soon as you have 3 or more programs or users using this API, its going to get very hard to safely change it.

[sheepdreamofandroids]

@carl-mastrangelo You're right that a possible head or tail to a stream should be a message and not just one field.

But it would be nice to be explicit about the fact that that message is a head or a tail instead of defining it in comments somewhere. E.g.:

service Foo {
  rpc PutRecords(head Header, stream Record, tail Tail) returns (RecordResponse);
}

message Head {
    int32 recordset_id = 1;
}

message Tail {
    int32 total_sent = 1;
}

....

Or maybe a bit more terse:

service Foo {
  rpc PutRecords(stream Header Record Tail) returns (RecordResponse);
}
...

Of course it doesn't "do" anything that your method doesn't, it's just a bit more explicit about the intention.

[carl-mastrangelo]

@sheepdreamofandroids You can do something similar with MethodExtensions, and then modify the code generator to do that. The questions is: why? gRPC headers and trailers _already_ give you that ability. Now there are two ways to do things. Also, your proposal doesn't cover header-message-trailer responses, so now there are 6 messages minimum per rpc method? And are headers or trailers streaming too? The syntax makes it seem like they are.

This is adding complexity for no functionality gain. The API, semantics, generated code, and maintainability are all worse. And in return? Slightly clearer message ordering.

In any case, this discussion should probably be on a mailing list since Github isn't ideal for threaded conversation.