[GitHub] [incubator-brpc] zyearn commented on a change in pull request #694: Health check by rpc call

2019-03-28 Thread GitBox
zyearn commented on a change in pull request #694: Health check by rpc call
URL: https://github.com/apache/incubator-brpc/pull/694#discussion_r270268975
 
 

 ##
 File path: src/brpc/details/health_check.cpp
 ##
 @@ -0,0 +1,233 @@
+// Copyright (c) 2014 Baidu, Inc.
+// 
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+// 
+// http://www.apache.org/licenses/LICENSE-2.0
+// 
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Authors: Ge,Jun (ge...@baidu.com)
+//  Jiashun Zhu(zhujias...@baidu.com)
+
+#include "brpc/details/health_check.h"
+#include "brpc/socket.h"
+#include "brpc/channel.h"
+#include "brpc/controller.h"
+#include "brpc/details/controller_private_accessor.h"
+#include "brpc/global.h"
+#include "brpc/log.h"
+#include "bthread/unstable.h"
+#include "bthread/bthread.h"
+
+namespace brpc {
+
+// declared at socket.cpp
+extern SocketVarsCollector* g_vars;
+
+DEFINE_string(health_check_path, "", "Http path of health check call."
+"By default health check succeeds if the server is connectable."
+"If this flag is set, health check is not completed until a http "
+"call to the path succeeds within -health_check_timeout_ms(to make "
+"sure the server functions well).");
+DEFINE_int32(health_check_timeout_ms, 500, "The timeout for both establishing "
+"the connection and the http call to -health_check_path over the 
connection");
+
+class HealthCheckChannel : public brpc::Channel {
+public:
+HealthCheckChannel() {}
+~HealthCheckChannel() {}
+
+int Init(SocketId id, const ChannelOptions* options);
+};
+
+int HealthCheckChannel::Init(SocketId id, const ChannelOptions* options) {
+brpc::GlobalInitializeOrDie();
+if (InitChannelOptions(options) != 0) {
+return -1;
+}
+_server_id = id;
+return 0;
+}
+
+class OnAppHealthCheckDone : public google::protobuf::Closure {
 
 Review comment:
   这个当时讨论过,用AppCheck的原因是Http只是实现方式。目前client打开这个功能的话,需要由用户来保证这个接口server是支持的


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org



[GitHub] [incubator-brpc] zyearn commented on a change in pull request #694: Health check by rpc call

2019-03-28 Thread GitBox
zyearn commented on a change in pull request #694: Health check by rpc call
URL: https://github.com/apache/incubator-brpc/pull/694#discussion_r270268750
 
 

 ##
 File path: src/brpc/details/health_check.cpp
 ##
 @@ -0,0 +1,233 @@
+// Copyright (c) 2014 Baidu, Inc.
+// 
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+// 
+// http://www.apache.org/licenses/LICENSE-2.0
+// 
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Authors: Ge,Jun (ge...@baidu.com)
+//  Jiashun Zhu(zhujias...@baidu.com)
+
+#include "brpc/details/health_check.h"
+#include "brpc/socket.h"
+#include "brpc/channel.h"
+#include "brpc/controller.h"
+#include "brpc/details/controller_private_accessor.h"
+#include "brpc/global.h"
+#include "brpc/log.h"
+#include "bthread/unstable.h"
+#include "bthread/bthread.h"
+
+namespace brpc {
+
+// declared at socket.cpp
+extern SocketVarsCollector* g_vars;
+
+DEFINE_string(health_check_path, "", "Http path of health check call."
+"By default health check succeeds if the server is connectable."
+"If this flag is set, health check is not completed until a http "
+"call to the path succeeds within -health_check_timeout_ms(to make "
+"sure the server functions well).");
+DEFINE_int32(health_check_timeout_ms, 500, "The timeout for both establishing "
+"the connection and the http call to -health_check_path over the 
connection");
+
+class HealthCheckChannel : public brpc::Channel {
+public:
+HealthCheckChannel() {}
+~HealthCheckChannel() {}
+
+int Init(SocketId id, const ChannelOptions* options);
+};
+
+int HealthCheckChannel::Init(SocketId id, const ChannelOptions* options) {
+brpc::GlobalInitializeOrDie();
+if (InitChannelOptions(options) != 0) {
+return -1;
+}
+_server_id = id;
+return 0;
+}
+
+class OnAppHealthCheckDone : public google::protobuf::Closure {
+public:
+virtual void Run();
+
+HealthCheckChannel channel;
+brpc::Controller cntl;
+SocketId id;
+int64_t interval_s;
+int64_t last_check_time_ms;
+};
+
+class HealthCheckManager {
+public:
+static void StartCheck(SocketId id, int64_t check_interval_s);
+static void* AppCheck(void* arg);
+};
+
+void HealthCheckManager::StartCheck(SocketId id, int64_t check_interval_s) {
+SocketUniquePtr ptr;
+const int rc = Socket::AddressFailedAsWell(id, );
+if (rc < 0) {
+RPC_VLOG << "SocketId=" << id
+ << " was abandoned during health checking";
+return;
+}
+LOG(INFO) << "Checking path=" << ptr->remote_side() << 
FLAGS_health_check_path;
+OnAppHealthCheckDone* done = new OnAppHealthCheckDone;
+done->id = id;
+done->interval_s = check_interval_s;
+brpc::ChannelOptions options;
+options.protocol = PROTOCOL_HTTP;
+options.max_retry = 0;
+options.timeout_ms =
+std::min((int64_t)FLAGS_health_check_timeout_ms, check_interval_s * 
1000);
+if (done->channel.Init(id, ) != 0) {
+LOG(WARNING) << "Fail to init health check channel to SocketId=" << id;
+ptr->_ninflight_app_health_check.fetch_sub(
+1, butil::memory_order_relaxed);
+delete done;
+return;
+}
+AppCheck(done);
+}
+
+void* HealthCheckManager::AppCheck(void* arg) {
+OnAppHealthCheckDone* done = static_cast(arg);
+done->cntl.Reset();
+done->cntl.http_request().uri() = FLAGS_health_check_path;
+ControllerPrivateAccessor(>cntl).set_health_check_call();
+done->last_check_time_ms = butil::gettimeofday_ms();
+done->channel.CallMethod(NULL, >cntl, NULL, NULL, done);
+return NULL;
+}
+
+void OnAppHealthCheckDone::Run() {
+std::unique_ptr self_guard(this);
+SocketUniquePtr ptr;
+const int rc = Socket::AddressFailedAsWell(id, );
+if (rc < 0) {
+RPC_VLOG << "SocketId=" << id
+<< " was abandoned during health checking";
+return;
+}
+if (!cntl.Failed() || ptr->Failed()) {
+LOG_IF(INFO, !cntl.Failed()) << "Succeeded to call "
+<< ptr->remote_side() << FLAGS_health_check_path;
+// if ptr->Failed(), previous SetFailed would trigger next round
+// of hc, just return here.
+ptr->_ninflight_app_health_check.fetch_sub(
+1, butil::memory_order_relaxed);
+return;
+}
+RPC_VLOG << "Fail to check path=" << FLAGS_health_check_path
+<< ", " << cntl.ErrorText();
+
+int64_t sleep_time_ms =
+last_check_time_ms + interval_s * 1000 - butil::gettimeofday_ms();
+if 

[GitHub] [incubator-brpc] zyearn commented on a change in pull request #694: Health check by rpc call

2019-03-28 Thread GitBox
zyearn commented on a change in pull request #694: Health check by rpc call
URL: https://github.com/apache/incubator-brpc/pull/694#discussion_r269896275
 
 

 ##
 File path: src/brpc/details/health_check.cpp
 ##
 @@ -0,0 +1,246 @@
+// Copyright (c) 2014 Baidu, Inc.
+// 
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+// 
+// http://www.apache.org/licenses/LICENSE-2.0
+// 
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Authors: Ge,Jun (ge...@baidu.com)
+//  Jiashun Zhu(zhujias...@baidu.com)
+
+#include "brpc/details/health_check.h"
+#include "brpc/socket.h"
+#include "brpc/channel.h"
+#include "brpc/controller.h"
+#include "brpc/details/controller_private_accessor.h"
+#include "brpc/global.h"
+#include "brpc/log.h"
+#include "bthread/unstable.h"
+#include "bthread/bthread.h"
+
+namespace brpc {
+
+// declared at socket.cpp
+extern SocketVarsCollector* g_vars;
+
+DEFINE_string(health_check_path, "", "Http path of health check call."
+"By default health check succeeds if the server is connectable."
+"If this flag is set, health check is not completed until a http "
+"call to the path succeeds within -health_check_timeout_ms(to make "
+"sure the server functions well).");
+DEFINE_int32(health_check_timeout_ms, 500, "The timeout for both establishing "
+"the connection and the http call to -health_check_path over the 
connection");
+
+class HealthCheckChannel : public brpc::Channel {
+public:
+HealthCheckChannel() {}
+~HealthCheckChannel() {}
+
+int Init(SocketId id, const ChannelOptions* options);
+};
+
+int HealthCheckChannel::Init(SocketId id, const ChannelOptions* options) {
+brpc::GlobalInitializeOrDie();
+if (InitChannelOptions(options) != 0) {
+return -1;
+}
+_server_id = id;
+return 0;
+}
+
+class OnAppHealthCheckDone : public google::protobuf::Closure {
+public:
+virtual void Run();
+
+HealthCheckChannel channel;
+brpc::Controller cntl;
+SocketId id;
+int64_t interval_s;
+int64_t last_check_time_ms;
+};
+
+class HealthCheckManager {
+public:
+static void StartCheck(SocketId id, int64_t check_interval_s) {
+SocketUniquePtr ptr;
+const int rc = Socket::AddressFailedAsWell(id, );
+if (rc < 0) {
+RPC_VLOG << "SocketId=" << id
+ << " was abandoned during health checking";
+return;
+}
+LOG(INFO) << "Checking path=" << ptr->remote_side() << 
FLAGS_health_check_path;
+OnAppHealthCheckDone* done = new OnAppHealthCheckDone;
+done->id = id;
+done->interval_s = check_interval_s;
+brpc::ChannelOptions options;
+options.protocol = PROTOCOL_HTTP;
+options.max_retry = 0;
+options.timeout_ms =
+std::min((int64_t)FLAGS_health_check_timeout_ms, check_interval_s 
* 1000);
+if (done->channel.Init(id, ) != 0) {
+LOG(WARNING) << "Fail to init health check channel to SocketId=" 
<< id;
+ptr->_ninflight_app_health_check.fetch_sub(
+1, butil::memory_order_relaxed);
+delete done;
+return;
+}
+AppCheck(done);
+}
+
+static void* AppCheck(void* arg) {
+OnAppHealthCheckDone* done = static_cast(arg);
+done->cntl.Reset();
+done->cntl.http_request().uri() = FLAGS_health_check_path;
+ControllerPrivateAccessor(>cntl).set_health_check_call();
+done->last_check_time_ms = butil::gettimeofday_ms();
+done->channel.CallMethod(NULL, >cntl, NULL, NULL, done);
+return NULL;
+}
+
+static void RunAppCheck(void* arg) {
+bthread_t th = 0;
+int rc = bthread_start_background(
+, _ATTR_NORMAL, AppCheck, arg);
+if (rc != 0) {
+LOG(ERROR) << "Fail to start AppCheck";
+AppCheck(arg);
+return;
+}
+}
+};
+
+void OnAppHealthCheckDone::Run() {
+std::unique_ptr self_guard(this);
+SocketUniquePtr ptr;
+const int rc = Socket::AddressFailedAsWell(id, );
+if (rc < 0) {
+RPC_VLOG << "SocketId=" << id
+<< " was abandoned during health checking";
+return;
+}
+if (!cntl.Failed() || ptr->Failed()) {
 
 Review comment:
   OK,已加


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about 

[GitHub] [incubator-brpc] zyearn commented on a change in pull request #694: Health check by rpc call

2019-03-26 Thread GitBox
zyearn commented on a change in pull request #694: Health check by rpc call
URL: https://github.com/apache/incubator-brpc/pull/694#discussion_r268964773
 
 

 ##
 File path: docs/cn/client.md
 ##
 @@ -242,7 +242,7 @@ locality-aware,优先选择延时低的下游,直到其延时高于其他机
 | - | - |  
| --- |
 | health_check_interval (R) | 3 | seconds between consecutive 
health-checkings | src/brpc/socket_map.cpp |
 
-一旦server被连接上,它会恢复为可用状态。如果在隔离过程中,server从命名服务中删除了,brpc也会停止连接尝试。
+在默认的配置下,一旦server被连接上,它会恢复为可用状态;brpc还提供了应用层健康检查的机制,协议是Http,只有当Server返回200时,这个server才算恢复,可以通过把-health\_check\_path设置成被检查的路径来打开这个功能(如果下游也是brpc,推荐设置成/health,服务健康的话会返回200),-health\_check\_timeout\_ms设置超时(默认500ms)。如果在隔离过程中,server从命名服务中删除了,brpc也会停止连接尝试。
 
 Review comment:
   应该是要的


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org



[GitHub] [incubator-brpc] zyearn commented on a change in pull request #694: Health check by rpc call

2019-03-24 Thread GitBox
zyearn commented on a change in pull request #694: Health check by rpc call
URL: https://github.com/apache/incubator-brpc/pull/694#discussion_r268477608
 
 

 ##
 File path: src/brpc/socket.cpp
 ##
 @@ -979,6 +995,89 @@ void HealthCheckTask::OnDestroyingTask() {
 delete this;
 }
 
+class HealthCheckChannel : public brpc::Channel {
+public:
+HealthCheckChannel() {}
+~HealthCheckChannel() {}
+
+int Init(SocketId id, const ChannelOptions* options);
+};
+
+int HealthCheckChannel::Init(SocketId id, const ChannelOptions* options) {
+brpc::GlobalInitializeOrDie();
+if (InitChannelOptions(options) != 0) {
+return -1;
+}
+_server_id = id;
+return 0;
+}
+
+class OnHealthCheckRPCDone : public google::protobuf::Closure {
+public:
+void Run() {
+std::unique_ptr self_guard(this);
+SocketUniquePtr ptr;
+const int rc = Socket::AddressFailedAsWell(id, );
+if (rc < 0) {
+RPC_VLOG << "SocketId=" << id
+ << " was abandoned during health checking";
+return;
+}
+if (!cntl.Failed()) {
+ptr->ResetHealthCheckingUsingRPC();
+return;
+}
+
+// Socket::SetFailed() will trigger next round of hc, just
+// return here.
+if (cntl.Failed() && ptr->Failed()) {
+return;
+}
+// the left case is cntl.Failed() && !ptr->Failed(),
+// in which we should retry hc rpc.
+RPC_VLOG << "Fail to health check using rpc, error="
+<< cntl.ErrorText();
+bthread_usleep(interval_s * 100);
+cntl.Reset();
+cntl.http_request().uri() = FLAGS_health_check_path;
+cntl.set_health_check_call(true);
+channel.CallMethod(NULL, , NULL, NULL, self_guard.release());
+}
+
+HealthCheckChannel channel;
+brpc::Controller cntl;
+SocketId id;
+int64_t interval_s;
+};
+
+class HealthCheckManager {
+public:
+static void StartCheck(SocketId id, int64_t check_interval_s) {
+SocketUniquePtr ptr;
+const int rc = Socket::AddressFailedAsWell(id, );
+if (rc < 0) {
+RPC_VLOG << "SocketId=" << id
+ << " was abandoned during health checking";
+return;
+}
+OnHealthCheckRPCDone* done = new OnHealthCheckRPCDone;
+done->id = id;
+done->interval_s = check_interval_s;
+brpc::ChannelOptions options;
+options.protocol = "http";
+options.max_retry = 0;
+options.timeout_ms = FLAGS_health_check_timeout_ms;
+if (done->channel.Init(id, ) != 0) {
+LOG(WARNING) << "Fail to init health check channel to SocketId=" 
<< id;
+ptr->ResetHealthCheckingUsingRPC();
+return;
 
 Review comment:
   嗯你说得对,不过这里逻辑简单,而且下面CallMethod的 
>cntl和done.release()是存在竞争的,done.release()需要单独赋给一个额外的局部变量,我就直接delete了


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org



[GitHub] [incubator-brpc] zyearn commented on a change in pull request #694: Health check by rpc call

2019-03-24 Thread GitBox
zyearn commented on a change in pull request #694: Health check by rpc call
URL: https://github.com/apache/incubator-brpc/pull/694#discussion_r268420430
 
 

 ##
 File path: src/brpc/socket.cpp
 ##
 @@ -1026,6 +1125,9 @@ bool HealthCheckTask::OnTriggeringTask(timespec* 
next_abstime) {
 }
 ptr->Revive();
 ptr->_hc_count = 0;
+if (ptr->IsHealthCheckingUsingRPC()) {
 
 Review comment:
   
不在revive函数中直接StartCheck的原因是Revive就应该做好自己的事情,让StartCheck交给更上层控制逻辑做。从这个角度来说,Revive这个函数是不应该动的,把_health_checking_using_rpc的赋值放到WaitAndReset中更合适。上面提到的熔断的Reset可能也应该放到WaitAndReset中
 @TousakaRin 


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org



[GitHub] [incubator-brpc] zyearn commented on a change in pull request #694: Health check by rpc call

2019-03-24 Thread GitBox
zyearn commented on a change in pull request #694: Health check by rpc call
URL: https://github.com/apache/incubator-brpc/pull/694#discussion_r268420430
 
 

 ##
 File path: src/brpc/socket.cpp
 ##
 @@ -1026,6 +1125,9 @@ bool HealthCheckTask::OnTriggeringTask(timespec* 
next_abstime) {
 }
 ptr->Revive();
 ptr->_hc_count = 0;
+if (ptr->IsHealthCheckingUsingRPC()) {
 
 Review comment:
   
不在revive函数中直接StartCheck的原因是Revive就应该做好自己的事情,让StartCheck交给更上层控制逻辑做。从这个角度来说,Revive这个函数是不应该动的,用该把_health_checking_using_rpc的赋值放到WaitAndReset中更合适。上面提到的熔断的Reset可能也应该放到WaitAndReset中
 @TousakaRin 


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org



[GitHub] [incubator-brpc] zyearn commented on a change in pull request #694: Health check by rpc call

2019-03-24 Thread GitBox
zyearn commented on a change in pull request #694: Health check by rpc call
URL: https://github.com/apache/incubator-brpc/pull/694#discussion_r268419761
 
 

 ##
 File path: src/brpc/controller.cpp
 ##
 @@ -986,7 +986,8 @@ void Controller::IssueRPC(int64_t start_realtime_us) {
 // Don't use _current_call.peer_id which is set to -1 after 
construction
 // of the backup call.
 const int rc = Socket::Address(_single_server_id, _sock);
-if (rc != 0 || tmp_sock->IsLogOff()) {
+if (rc != 0 || tmp_sock->IsLogOff() ||
+(!has_flag(FLAGS_HEALTH_CHECK_CALL) && 
tmp_sock->IsHealthCheckingUsingRPC())) {
 
 Review comment:
   封装了一个is_health_check_call


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org



[GitHub] [incubator-brpc] zyearn commented on a change in pull request #694: Health check by rpc call

2019-03-24 Thread GitBox
zyearn commented on a change in pull request #694: Health check by rpc call
URL: https://github.com/apache/incubator-brpc/pull/694#discussion_r268419752
 
 

 ##
 File path: src/brpc/socket.cpp
 ##
 @@ -979,6 +995,89 @@ void HealthCheckTask::OnDestroyingTask() {
 delete this;
 }
 
+class HealthCheckChannel : public brpc::Channel {
+public:
+HealthCheckChannel() {}
+~HealthCheckChannel() {}
+
+int Init(SocketId id, const ChannelOptions* options);
+};
+
+int HealthCheckChannel::Init(SocketId id, const ChannelOptions* options) {
+brpc::GlobalInitializeOrDie();
+if (InitChannelOptions(options) != 0) {
+return -1;
+}
+_server_id = id;
+return 0;
+}
+
+class OnHealthCheckRPCDone : public google::protobuf::Closure {
+public:
+void Run() {
+std::unique_ptr self_guard(this);
+SocketUniquePtr ptr;
+const int rc = Socket::AddressFailedAsWell(id, );
+if (rc < 0) {
+RPC_VLOG << "SocketId=" << id
+ << " was abandoned during health checking";
+return;
+}
+if (!cntl.Failed()) {
+ptr->ResetHealthCheckingUsingRPC();
+return;
+}
+
+// Socket::SetFailed() will trigger next round of hc, just
+// return here.
+if (cntl.Failed() && ptr->Failed()) {
+return;
+}
+// the left case is cntl.Failed() && !ptr->Failed(),
+// in which we should retry hc rpc.
+RPC_VLOG << "Fail to health check using rpc, error="
+<< cntl.ErrorText();
+bthread_usleep(interval_s * 100);
+cntl.Reset();
+cntl.http_request().uri() = FLAGS_health_check_path;
+cntl.set_health_check_call(true);
+channel.CallMethod(NULL, , NULL, NULL, self_guard.release());
+}
+
+HealthCheckChannel channel;
+brpc::Controller cntl;
+SocketId id;
+int64_t interval_s;
+};
+
+class HealthCheckManager {
+public:
+static void StartCheck(SocketId id, int64_t check_interval_s) {
+SocketUniquePtr ptr;
+const int rc = Socket::AddressFailedAsWell(id, );
+if (rc < 0) {
+RPC_VLOG << "SocketId=" << id
+ << " was abandoned during health checking";
+return;
+}
+OnHealthCheckRPCDone* done = new OnHealthCheckRPCDone;
+done->id = id;
+done->interval_s = check_interval_s;
+brpc::ChannelOptions options;
+options.protocol = "http";
+options.max_retry = 0;
+options.timeout_ms = FLAGS_health_check_timeout_ms;
+if (done->channel.Init(id, ) != 0) {
+LOG(WARNING) << "Fail to init health check channel to SocketId=" 
<< id;
+ptr->ResetHealthCheckingUsingRPC();
+return;
 
 Review comment:
   嗯Fixed


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org



[GitHub] [incubator-brpc] zyearn commented on a change in pull request #694: Health check by rpc call

2019-03-22 Thread GitBox
zyearn commented on a change in pull request #694: Health check by rpc call
URL: https://github.com/apache/incubator-brpc/pull/694#discussion_r268383593
 
 

 ##
 File path: src/brpc/socket.cpp
 ##
 @@ -780,6 +793,9 @@ void Socket::Revive() {
 } else {
 LOG(INFO) << "Revived " << *this;
 }
+if (FLAGS_health_check_using_rpc) {
 
 Review comment:
   这个问题之后单独开一个issue解决吧,混在一起解决有点乱


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org



[GitHub] [incubator-brpc] zyearn commented on a change in pull request #694: Health check by rpc call

2019-03-22 Thread GitBox
zyearn commented on a change in pull request #694: Health check by rpc call
URL: https://github.com/apache/incubator-brpc/pull/694#discussion_r268381737
 
 

 ##
 File path: src/brpc/socket.cpp
 ##
 @@ -780,6 +793,9 @@ void Socket::Revive() {
 } else {
 LOG(INFO) << "Revived " << *this;
 }
+if (FLAGS_health_check_using_rpc) {
 
 Review comment:
   
revive这个函数似乎是不存在竞争的,compare_exchange_weak是必成功的,那把这些操作放到compare_exchange_weak就可以了


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org



[GitHub] [incubator-brpc] zyearn commented on a change in pull request #694: Health check by rpc call

2019-03-22 Thread GitBox
zyearn commented on a change in pull request #694: Health check by rpc call
URL: https://github.com/apache/incubator-brpc/pull/694#discussion_r268103165
 
 

 ##
 File path: src/brpc/socket.cpp
 ##
 @@ -780,6 +793,9 @@ void Socket::Revive() {
 } else {
 LOG(INFO) << "Revived " << *this;
 }
+if (FLAGS_health_check_using_rpc) {
 
 Review comment:
   对的,不过这个问题一直存在。放while循环之前也不行,没有compare_exchange_weak成功还得恢复回来,这里有挺多竞争问题的


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org



[GitHub] [incubator-brpc] zyearn commented on a change in pull request #694: Health check by rpc call

2019-03-22 Thread GitBox
zyearn commented on a change in pull request #694: Health check by rpc call
URL: https://github.com/apache/incubator-brpc/pull/694#discussion_r268067952
 
 

 ##
 File path: src/brpc/channel.cpp
 ##
 @@ -281,6 +281,15 @@ int Channel::Init(butil::EndPoint server_addr_and_port,
 return InitSingle(server_addr_and_port, "", options);
 }
 
+int Channel::Init(SocketId id, const ChannelOptions* options) {
 
 Review comment:
   嗯,改成class HealthCheckChannel : public brpc::Channel了


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org



[GitHub] [incubator-brpc] zyearn commented on a change in pull request #694: Health check by rpc call

2019-03-20 Thread GitBox
zyearn commented on a change in pull request #694: Health check by rpc call
URL: https://github.com/apache/incubator-brpc/pull/694#discussion_r267273523
 
 

 ##
 File path: src/brpc/channel.cpp
 ##
 @@ -281,6 +281,15 @@ int Channel::Init(butil::EndPoint server_addr_and_port,
 return InitSingle(server_addr_and_port, "", options);
 }
 
+int Channel::Init(SocketId id, const ChannelOptions* options) {
 
 Review comment:
   TODO:这个接口暴露给用户有点怪怪的,有没有更好的workaround


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org