wasphin commented on code in PR #1806:
URL: https://github.com/apache/incubator-brpc/pull/1806#discussion_r901060350
##########
src/brpc/socket_map.h:
##########
@@ -47,17 +47,18 @@ inline bool operator!=(const ChannelSignature& s1, const
ChannelSignature& s2) {
// The following fields uniquely define a Socket. In other word,
// Socket can't be shared between 2 different SocketMapKeys
struct SocketMapKey {
- explicit SocketMapKey(const butil::EndPoint& pt)
- : peer(pt)
+ explicit SocketMapKey(const butil::EndPoint& pt, const std::string&
hc_path)
+ : peer(pt), health_check_path(hc_path)
{}
- SocketMapKey(const butil::EndPoint& pt, const ChannelSignature& cs)
- : peer(pt), channel_signature(cs)
+ SocketMapKey(const butil::EndPoint& pt, const std::string& hc_path, const
ChannelSignature& cs)
+ : peer(pt), health_check_path(hc_path), channel_signature(cs)
{}
- SocketMapKey(const ServerNode& sn, const ChannelSignature& cs)
- : peer(sn), channel_signature(cs)
+ SocketMapKey(const ServerNode& sn, const std::string& hc_path, const
ChannelSignature& cs)
+ : peer(sn), health_check_path(hc_path), channel_signature(cs)
{}
ServerNode peer;
+ std::string health_check_path;
Review Comment:
是否有添加到 key 里的必要?
##########
src/brpc/details/health_check.cpp:
##########
@@ -145,18 +146,20 @@ void OnAppHealthCheckDone::Run() {
class HealthCheckTask : public PeriodicTask {
public:
- explicit HealthCheckTask(SocketId id);
+ explicit HealthCheckTask(SocketId id, std::string health_check_path);
bool OnTriggeringTask(timespec* next_abstime) override;
void OnDestroyingTask() override;
private:
SocketId _id;
bool _first_time;
+ std::string _health_check_path;
};
-HealthCheckTask::HealthCheckTask(SocketId id)
+HealthCheckTask::HealthCheckTask(SocketId id, std::string hc_path)
: _id(id)
- , _first_time(true) {}
+ , _first_time(true)
+ , _health_check_path(hc_path) {}
Review Comment:
可以 `std::move()`
##########
src/brpc/channel.cpp:
##########
@@ -197,6 +198,12 @@ int Channel::InitChannelOptions(const ChannelOptions*
options) {
if (!cg.empty() && (::isspace(cg.front()) || ::isspace(cg.back()))) {
butil::TrimWhitespace(cg, butil::TRIM_ALL, &cg);
}
+
+ // health_check_path
+ if (_options.health_check_path.empty()) {
+ _options.health_check_path = FLAGS_health_check_path;
+ }
Review Comment:
还在 socket 里检查就可以.
##########
src/brpc/socket.h:
##########
@@ -286,6 +287,9 @@ friend class policy::H2GlobalStreamCreator;
// Initialized by SocketOptions.health_check_interval_s.
int health_check_interval() const { return _health_check_interval_s; }
+ // Initialized by SocketOptions.health_check_path.
+ std::string health_check_path() const { return _health_check_path; }
Review Comment:
可以返回 `const std::string&`
##########
src/brpc/channel.cpp:
##########
@@ -38,6 +38,7 @@ namespace brpc {
DECLARE_bool(enable_rpcz);
DECLARE_bool(usercode_in_pthread);
+DECLARE_string(health_check_path);
Review Comment:
默认配置不需要改到这里.
##########
src/brpc/details/health_check.cpp:
##########
@@ -103,7 +103,7 @@ void HealthCheckManager::StartCheck(SocketId id, int64_t
check_interval_s) {
void* HealthCheckManager::AppCheck(void* arg) {
OnAppHealthCheckDone* done = static_cast<OnAppHealthCheckDone*>(arg);
done->cntl.Reset();
- done->cntl.http_request().uri() = FLAGS_health_check_path;
+ done->cntl.http_request().uri() =
done->channel.options().health_check_path;
Review Comment:
path 可以直接存到 `OnAppHealthCheckDone` 里直接拿出来用?
##########
src/brpc/channel.cpp:
##########
@@ -136,7 +137,7 @@ Channel::Channel(ProfilerLinker)
Channel::~Channel() {
if (_server_id != INVALID_SOCKET_ID) {
const ChannelSignature sig = ComputeChannelSignature(_options);
- SocketMapRemove(SocketMapKey(_server_address, sig));
+ SocketMapRemove(SocketMapKey(_server_address,
_options.health_check_path, sig));
Review Comment:
不需要添加到 `SocketMapKey`
##########
src/brpc/details/naming_service_thread.cpp:
##########
@@ -71,7 +71,7 @@ NamingServiceThread::Actions::~Actions() {
// Remove all sockets from SocketMap
for (std::vector<ServerNode>::const_iterator it = _last_servers.begin();
it != _last_servers.end(); ++it) {
- const SocketMapKey key(*it, _owner->_options.channel_signature);
+ const SocketMapKey key(*it, _owner->_options.health_check_path,
_owner->_options.channel_signature);
Review Comment:
在 `SocketMapKey` 里存储 path 应该不是必要的.
##########
src/brpc/channel.cpp:
##########
@@ -311,7 +318,7 @@ int Channel::InitSingle(const butil::EndPoint&
server_addr_and_port,
if (CreateSocketSSLContext(_options, &ssl_ctx) != 0) {
return -1;
}
- if (SocketMapInsert(SocketMapKey(server_addr_and_port, sig),
+ if (SocketMapInsert(SocketMapKey(server_addr_and_port,
_options.health_check_path, sig),
Review Comment:
不需要在这里添加
##########
src/brpc/details/health_check.cpp:
##########
@@ -230,8 +233,8 @@ void HealthCheckTask::OnDestroyingTask() {
delete this;
}
-void StartHealthCheck(SocketId id, int64_t delay_ms) {
- PeriodicTaskManager::StartTaskAt(new HealthCheckTask(id),
+void StartHealthCheck(SocketId id, int64_t delay_ms, std::string hc_path) {
Review Comment:
这里可以检查为空时使用原来的默认配置, 这样适合传一个`const std::string&`
##########
src/brpc/channel.h:
##########
@@ -129,6 +129,14 @@ struct ChannelOptions {
// Default: ""
std::string connection_group;
+ // "Http path of health check call."
+ // "By default health check succeeds if the server is connectable."
+ // "If this health_check_path is not empty, 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)."
+ // Default: FLAGS_health_check_path
Review Comment:
copy 过来的时后可以适当调一下, 比如引号和`health_check_timeout_ms`
--
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.
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org
