grpc-go icon indicating copy to clipboard operation
grpc-go copied to clipboard

codegen: Use Go generics for stream types

Open aarongable opened this issue 1 year ago • 8 comments
trafficstars

Use case(s)

Suppose you have the following service definitions:

message HelloRequest {
	string name = 1;
}

message HelloReply {
	string greeting = 1;
}

service Greeter {
	rpc StoreHello (HelloRequest) returns (google.protobuf.Empty) {}
	rpc SayHello (google.protobuf.Empty) returns (stream HelloReply) {}
}

service GreeterReadOnly {
	rpc SayHello (google.protobuf.Empty) returns (stream HelloReply) {}
}

The idea here being that someone with access to a Greeter client could call StoreHello multiple times to store many names that should be greeted, and then either a Greeter client or a GreeterReadOnly client can call SayHello to get a stream of all the stored replies. Obviously this is a toy example, but you can see a real example of this same pattern here. The salient feature here is this: we have two different services which define the exact same streaming RPC.

Now because both Greeter and GreeterReadOnly share functionality, we want to use a single implementation type to implement both of them. So we write something like this:

type GreeterImpl struct {
	names []string
}

func (g *GreeterImpl) StoreHello(ctx context.Context, req *pb.HelloRequest) (*emptypb.Empty, error) {
	...
}

func (g *GreeterImpl) SayHello(req *emptypb.Empty, stream pb.Greeter_SayHelloServer) error {
	...
}

But of course when we go to Register this implementation, it doesn't actually work. It satisfies the Greeter interface, but not the GreeterReadOnly interface.

func main() {
	grpcServer := grpc.NewServer()
	pb.RegisterGreeterServer(grpcServer, &GreeterImpl{}) // This succeeds
	pb.RegisterGreeterReadOnlyServer(grpcServer, &GreeterImpl{}) // This fails at compile time
}

This is because grpc-go has generated the following interfaces for us:

type GreeterServer interface {
	StoreHello(*HelloRequest) (*emptypb.Empty, error)
	SayHello(*HelloRequest, Greeter_SayHelloServer) error
}

type GreeterReadOnlyServer interface {
	SayHello(*HelloRequest, GreeterReadOnly_SayHelloServer) error
}

The SayHello methods in these two interfaces do not have the same signature, despite having the exact same definition in the original .proto file. This means that they can never both be implemented by the same Go struct.

Note: this is different from the case for Unary RPCs: if two different gRPC services define the same unary rpc, then the generated Go methods have the exact same signature and can both be implemented by the same type.

Proposed Solution

The obvious solution here is that the stream object could be named by the message type(s) it is capable of streaming, rather than by the service and rpc which define it. For example, instead of Greeter_SayHelloServer, the type could be named HelloReplyServerStreamServer, indicating that it a) is the Server half of the stream object, b) that it is for a Server-Streaming (i.e. one request, many replies) method, and c) that it streams HelloReply messages. Then this same exact type could be used in both versions of the SayHello method, giving them the same signature, and allowing them to be implemented by a single type.

But I propose going even further: rather than generating a unique type for every stream, I think that protoc-gen-go-grpc should instead take advantage of Go's generics support and define only six streaming types, parameterized by the message type they stream:

// ServerStreamClient represents the client side of a server-streaming (one request, many responses) RPC.
type ServerStreamClient[T] interface {
	Recv() (*T, error)
	grpc.ClientStream
}

type serverStreamClient struct {
	grpc.ClientStream
}

func (x *serverStreamClient[T]) Recv() (*T, error) {
	m := new(T)
	if err := x.ClientStream.RecvMsg(m); err != nil {
		return nil, err
	}
	return m, nil
}

// ServerStreamServer represents the server side of a server-streaming (one request, many responses) RPC.
type ServerStreamServer[T] interface {
	Send(*T) error
	grpc.ServerStream
}

type serverStreamServer[T] struct {
	grpc.ServerStream
}

func (x *serverStreamServer[T]) Send(m *T) error {
	return x.ServerStream.SendMsg(m)
}

// ClientStreamClient represents the client side of a client-streaming (many requests, one response) RPC.
type ClientStreamClient[T] interface { ... }

// ClientStreamServer represents the server side of a client-streaming (many requests, one response) RPC.
type ClientStreamServer[T] interface { ... }

// BidiStreamClient represents the client side of a bidirectional-streaming (many requests, many responses) RPC.
type BidiStreamClient[T, U] interface { ... }

// BidiStreamServer represents the server side of a bidirectional-streaming (many requests, many responses) RPC.
type BidiStreamServer[T, U] interface { ... }

Then the generated code for the service interfaces and their SayHello method would simply be reduced to:

type GreeterClient interface {
	StoreHello(ctx context.Context, in *HelloRequest, opts ...grpc.CallOption) (*emptypb.Empty, error)
	SayHello(ctx context.Context, in *emptypb.Empty, opts ...grpc.CallOption) (ServerStreamClient[HelloReply], error)
}

type GreeterReadOnlyClient interface {
	SayHello(ctx context.Context, in *emptypb.Empty, opts ...grpc.CallOption) (ServerStreamClient[HelloReply], error)
}

func (c *greeterClient) SayHello(ctx context.Context, in *emptypb.Empty, opts ...grpc.CallOption) (ServerStreamClient[HelloReply], error) {
	stream, err := c.cc.NewStream(ctx, &Greeter_ServiceDesc.Streams[2], "/pb.Greeter/SayHello", opts...)
	if err != nil {
		return nil, err
	}
	x := &serverStreamClient[HelloReply]{stream}
	if err := x.ClientStream.SendMsg(in); err != nil {
		return nil, err
	}
	if err := x.ClientStream.CloseSend(); err != nil {
		return nil, err
	}
	return x, nil
}

type GreeterServer interface {
	StoreHello(*HelloRequest) (*emptypb.Empty, error)
 	SayHello(*emptypb.Empty, ServerClientServer[HelloReply]) error
}

type GreeterReadOnlyServer interface {
	SayHello(*emptypb.Empty, ServerClientServer[HelloReply]) error
}

func _Greeter_SayHello_Handler(srv interface{}, stream grpc.ServerStream) error {
	m := new(HelloReply)
	if err := stream.RecvMsg(m); err != nil {
		return err
	}
	return srv.(GreeterServer).SayHello(m, &serverStreamServer[HelloReply]{stream})
}

Note first that this eliminates the need for there ever to be service- or method-specific types in the protoc-gen-go-grpc generated code. And in fact these [Server|Client|Bidi]Stream[Server|Client] generic types could be defined once in the gRPC package itself and then simply referenced in the generated code.

But note more importantly that this means that both GreeterServer.SayHello and GreeterReadOnlyServer.SayHello have the exact same function signature, meaning that they can both be implemented by a single type:

type GreeterImpl struct {
	names []string
}

func (g *GreeterImpl) StoreHello(ctx context.Context, req *pb.HelloRequest) (*emptypb.Empty, error) {
	...
}

func (g *GreeterImpl) SayHello(req *emptypb.Empty, stream grpc.ServerStreamServer[pb.HelloReply]) error {
	...
}

Additional Context

This proposal is obviously not backwards-compatible, and if it were adopted it would mean that the new version of protoc-gen-go-grpc cannot be used by versions of Go prior to when it got generics (1.18). But I think it is sufficiently elegant that it is worth considering anyway.

aarongable avatar Mar 08 '24 21:03 aarongable

Something like this could be really interesting. Unfortunately this is a very niche kind of thing and we likely won't have the bandwidth to work on it in the foreseeable future. If you are interested in fleshing this out and sending PRs, we can review them.

As for an implementation plan, I'd propose something like:

  1. Add these generic interfaces and implementations in the experimental package.
  2. Have the codegen produce them only if a flag is set.
  3. Give some time for users to try them out. If they prove to be good, move the definitions to the grpc package with type aliases in experimental for several releases.

Not being backward compatible would be the biggest problem with changing how the codegen works. We'd either need to generate both together with different names (the non-generic implementation could even call the generic one), or we'd need a flag to decide which one to generate. If we generated both, we'd need to choose good names to be ergonomic and to avoid conflicts.

I'm not concerned about supporting versions of Go older than 1.18, as this kind of thing is necessary from time to time anyway (e.g. we will soon be making changes to the codegen that will require the most recent release of grpc-go, which doesn't support 1.18 already).

dfawley avatar Mar 08 '24 22:03 dfawley

Thanks for taking the time to read! I'd be happy to contribute implementation, but of course also have a lot of other things on my plate so no guarantees.

Would you also be willing to accept an implementation plan like this:

  1. Add generic interfaces and implementations to the grpc package. Modify codegen to produce the same exact exported type names as today, but with (e.g.) GreeterReadOnly_SayHelloServer being a type alias for grpc.ServerStreamServer[HelloReply].
  2. Add a flag to codegen to stop producing and using the type aliases, and instead use the underlying generic types directly.
  3. Give some time for users to try them out. If people like it, flip the default value of the flag and eventually remove it entirely.

To my mind, the big change here is in the publicly-exposed interface, and that's the thing worth protecting behind a flag. But I definitely don't have insight into all of the things the gRPC team might be concerned about (do Go generics have meaningfully different runtime performance? I don't think so, but maybe) so I'm not sure if swapping out the underlying implementation is too risky to not put in experimental first.

Thanks for the note about go 1.18; I'll keep an eye on other compatibility stuff and just make sure any reliance on generics lands after the other changes you mentioned.

aarongable avatar Mar 09 '24 00:03 aarongable

I definitely would like to put the interfaces in experimental first, to prevent anyone from relying upon them until all of the work is done and we're sure they are going to work. The other option is to put them in the permanent location with an "experimental" tag on them in doc strings, but unfortunately, lots of our users don't heed those sorts of warnings and changing them later can be a major problem.

Does the type aliases prevent any backward compatibility breakage problems? If so, that might be the ideal final state? I really don't want to be in a situation where users have to choose between two different forms of codegen that are incompatible. Otherwise no old generated code library can ever migrate to the new style (without a v2 since it's a breaking change), and users of that generated code would be stuck implementing it that way forever.

dfawley avatar Mar 13 '24 23:03 dfawley

I've added the generic implementations to the experimental package, and added the new codegen behind a flag, in this PR: https://github.com/grpc/grpc-go/pull/7057 Let me know what you think!

aarongable avatar Mar 20 '24 20:03 aarongable

Let's leave this issue open to track:

  • [ ] Release of protoc-gen-go-grpc
  • [ ] Flag flip to default on
  • [ ] Release of protoc-gen-go-grpc
  • [ ] Remove flag and code when off
  • [ ] Release of protoc-gen-go-grpc

dfawley avatar May 03 '24 18:05 dfawley

Do we have a rough timeline for when a new version of protoc-gen-go-grpc (presumably v1.4.0?) will be released?

aarongable avatar May 22 '24 00:05 aarongable

Also, I'd just like to thank the gRPC maintainers (particularly @dfawley and @arvindbr8) for being willing to review and accept this contribution. Let's Encrypt is already using it, and the cleanup it enabled -- now that one implementation can satisfy two service interfaces that share some of the same methods -- is truly delightful. Thank you!

aarongable avatar May 24 '24 20:05 aarongable

I was hoping we could get it done this week, but it looks like that isn't going to happen. So, hopefully early next week. There is nothing blocking us at this point.

And thank you for the suggestion, contribution, and for sticking with it through all the changes during review!

dfawley avatar May 24 '24 21:05 dfawley

Released as part of https://github.com/grpc/grpc-go/releases/tag/cmd/protoc-gen-go-grpc/v1.5.x

arvindbr8 avatar Aug 07 '24 21:08 arvindbr8