Adjust meta client ctor && implement heartbeat logic inside. #379
Conversation
dangleptr
requested review from
laura-ding,
dutor and
darionyaphet
and removed request for
laura-ding and
dutor
|
Jenkins go |
|
Unit testing passed. |
| mClient->init(); | ||
| auto r = mClient->addHosts({HostAddr(localIp, localDataPort)}).get(); | ||
|
|
||
| auto r = mClient->addHosts({HostAddr(localIp, localDataPort)}).get(); |
| LOG(ERROR) << "meta_server_addrs flag should be set!"; | ||
| return -1; | ||
| } | ||
| auto ioThreadPool = std::make_shared<folly::IOThreadPoolExecutor>(FLAGS_io_handlers); |
There was a problem hiding this comment.
Could we share the same pool with the hosting service?
There was a problem hiding this comment.
Hosting service? If it means the thrift service, yes of course. Please note line 143.
| @@ -114,7 +125,10 @@ int main(int argc, char *argv[]) { | |||
|
|
|||
| auto handler = std::make_shared<StorageServiceHandler>(kvstore.get(), std::move(schemaMan)); | |||
| gServer = std::make_unique<apache::thrift::ThriftServer>(); | |||
| CHECK(!!gServer) << "Failed to create the thrift server"; | |||
| if (!!gServer) { | |||
There was a problem hiding this comment.
By default, make_unique could never return a nullptr, instead it will throw bad_alloc on memory allocation failure.
There was a problem hiding this comment.
Thanks for your reminding. Good point. 👍
| DEFINE_string(meta_server_addrs, "", "list of meta server addresses," | ||
| "the format looks like ip1:port1, ip2:port2, ip3:port3"); | ||
| DEFINE_int32(meta_client_io_threads, 3, "meta client io threads"); | ||
| DEFINE_int32(heartbeat_interval_sec, 10, "Heartbeat interval, unit: second"); |
There was a problem hiding this comment.
secs or seconds? Maybe we should unify them, e.g. the above line.
| req.set_host(std::move(thriftHost)); | ||
| return getResponse(std::move(req), [] (auto client, auto request) { | ||
| return client->future_heartBeat(request); | ||
| }, [] (cpp2::HBResp&& resp) -> decltype(auto) { |
There was a problem hiding this comment.
FYI. I sincerely suggest not to use decltype(auto) if not necessary while auto suffices, because you may return a reference to local storage by accident. Consider this:
decltype(auto) get() {
int var = 0;
int &ref = var;
return ref;
// or return (var);
// or return (var &= 0xF)
}There was a problem hiding this comment.
You are right. We should be careful about it.
|
Please refer #383 |
Subtask of #283