-
Notifications
You must be signed in to change notification settings - Fork 809
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gRPC for cross DC traffic #4390
gRPC for cross DC traffic #4390
Conversation
1a04689
to
89406e7
Compare
57eab54
to
fb3f0d1
Compare
client/clientBean.go
Outdated
@@ -65,9 +66,10 @@ type ( | |||
SetRemoteFrontendClient(cluster string, client frontend.Client) | |||
} | |||
|
|||
// DispatcherProvider provides a diapatcher to a given address | |||
// DispatcherProvider provides a dispatcher to a given address | |||
DispatcherProvider interface { | |||
Get(name string, address string) (*yarpc.Dispatcher, error) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you rename it to GetTchannel to make it explicit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, updated.
@@ -358,3 +374,8 @@ func (cf *rpcClientFactory) newFrontendGRPCClient(hostAddress string) (frontend. | |||
apiv1.NewVisibilityAPIYARPCClient(config), | |||
), nil | |||
} | |||
|
|||
func isGRPCOutbound(config transport.ClientConfig) bool { | |||
namer, ok := config.GetUnaryOutbound().(transport.Namer) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we panic/log.fatal here if ok==false
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless old version of yarpc is used, this will not happen. But I guess explicit early panic is better. Updated.
What changed?
DispatcherProvider
interface to support creating dispatchers for gRPC. UpdateddnsDispatcherProvider
implementation to accordingly.ClusterGroupMetadata
to allow configuring cross DC transport. If not specified, it defaults totchannel
, as currently used.Why?
To support gRPC for cross DC traffic.
How did you test it?
Integration tests for cross DC replication.
Potential risks
TChannel is still used by default, and gRPC is currently opt in to minimize risk.
Release notes
Updated CHANGELOG.md
Documentation Changes