Add rpc_client interface. #11154
Merged
Add rpc_client interface. #11154
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
gongweibao
requested review from
Yancey1989,
jacquesqiao and
typhoonzero
and removed request for
jacquesqiao
Yancey1989
reviewed
Jun 4, 2018
| @@ -205,9 +207,9 @@ class RPCClient { | |||
| std::map<std::string, std::shared_ptr<grpc::Channel>> channels_; | |||
| std::atomic<int64_t> req_count_{0}; | |||
| std::mutex mutex_; | |||
| static std::unique_ptr<RPCClient> rpc_client_; | |||
| static std::unique_ptr<GRPCClient> rpc_client_; | |||
There was a problem hiding this comment.
rpc_client_
=>
grpc_client_
typhoonzero
reviewed
Jun 5, 2018
|
|
||
| virtual bool Wait() = 0; | ||
|
|
||
| static const int64_t rpc_time_out = 600 * 1000; |
|
|
||
| class RPCClient { | ||
| public: | ||
| virtual bool AsyncSendVariable(const std::string& ep, |
There was a problem hiding this comment.
We can make it shorter like ASendVar, SendVar, AGetVar, GetVar etc.
There was a problem hiding this comment.
Async不建议简写。 改成了类似AsyncSendVar。
| @@ -43,7 +43,7 @@ class FetchBarrierOp : public framework::OperatorBase { | |||
| // For profiling | |||
| platform::RecordEvent record_event(Type(), &ctx); | |||
|
|
|||
| auto rpc_client = detail::RPCClient::GetInstance(); | |||
| auto rpc_client = detail::GRPCClient::GetInstance(); | |||
There was a problem hiding this comment.
To use the client, better to use the type of base class, since we use a singleton, you can make the base class RPCClient as a singleton, and other clients can inherit from it.
typhoonzero
reviewed
Jun 6, 2018
|
|
||
| virtual void Wait() = 0; | ||
|
|
||
| static constexpr int64_t rpc_time_out = 600 * 1000; |
| } | ||
|
|
||
| protected: | ||
| virtual void InitImpl() = 0; |
There was a problem hiding this comment.
This may not need to be pure virtual, not all implementations need to start an event loop for a client.
typhoonzero
approved these changes
Jun 7, 2018
|
Will do in next PR. |
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.
Fix part of #10804
The old PR is #10805