[vtctldclient-codegen] Add support in codegen for streaming RPCs #9064
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Andrew Mason <amason@slack-corp.com>
(and also the individual message channels, because i don't actually understand why this can't be unbuffered and so i'd rather not think about it) Signed-off-by: Andrew Mason <amason@slack-corp.com>
Signed-off-by: Andrew Mason <amason@slack-corp.com>
setassociative
approved these changes
Oct 26, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR finally pays down the TODO I left myself here:
How exactly this works
The grpc-client case is simplest, because the streaming client code has the same shape as the unary case with a slightly different signature (the return value is an interface type instead of a pointer type like in the unary case). You can see an example of the code it generates here (which I am keeping as a draft until (1) I merge this one and (2) I have time to write tests for it).
In the local, in-process client case, things get significantly more complex. We need to munge the types a bit to produce something that both the client and server implementations can use transparently without realizing there's no actual grpc-core code being invoked beneath them. To do this, we introduce a new type,
bidiStreamwhich provides no-op implementations of both thegrpc.ClientStreamandgrpc.ServerStreaminterfaces. A key detail here is that we know that the rpc implementations will never call these functions. As long as our servers only ever callstream.Send()and our clients only ever callstream.Recv(), this will work, but if we ever need to reach into other methods, we'll need to reevaluate this approach.bidiStreamalso provides some common fields that individual streaming rpcs will use as well as a privateclosemethod to handle the cleanup signaling.Armed with our new type that looks like a
ServerStreamto the server implementation and aClientStreamto the client, we can then create a stream adapter struct for each streaming RPC and embed a*bidiStreamin it. Each stream adapter type will also include achan *Tfield for the actual message type sent over the stream for that RPC. We addRecv() (*T, error)andSend(*T) error)methods to complete the<RPCName>_Serverand<RPCName>_Clientinterfaces, and then implement the client wrapper to do the following:err == nil,stream.closewill sendio.EOFover theerrchon a later call toRecvRecvin the standard "streaming client" pattern until error orio.EOFTo see a concrete example of this, see the
backupStreamAdapterthis generated.Does the code work though?
Seems to! Trying this out on my backups PR:
grpcvtctldclient case
localvtctldclient case
Kudos
Kudos to @setassociative for rubber ducking with me to figure out the fix in 07c1fe6.
Related Issue(s)
Closes #9063
Checklist
Deployment Notes