morningman commented on code in PR #19123:
URL: https://github.com/apache/doris/pull/19123#discussion_r1185223106


##########
be/src/util/network_util.cpp:
##########
@@ -104,6 +104,26 @@ Status hostname_to_ip_addrs(const std::string& name, 
std::vector<std::string>* a
     return Status::OK();
 }
 
+bool is_valid_ip(const std::string& ip) {
+    unsigned char buf[sizeof(struct in6_addr)];
+    return inet_pton(AF_INET, ip.data(), buf) > 0;
+}
+
+std::string hostname_to_ip(const std::string& host) {
+    std::vector<std::string> addresses;
+    Status status = hostname_to_ip_addrs(host, &addresses);
+    if (!status.ok()) {
+        LOG(WARNING) << "status of hostname_to_ip_addrs was not ok, err is " 
<< status.to_string();
+        return "";
+    }
+    if (addresses.size() != 1) {
+        LOG(WARNING)
+                << "the number of addresses could only be equal to 1, failed 
to get ip from host";

Review Comment:
   Better print ip in `addresses` for debug



##########
fe/fe-core/src/main/java/org/apache/doris/service/FrontendOptions.java:
##########
@@ -53,64 +55,131 @@ public static void init() throws UnknownHostException {
             return;
         }
 
-        analyzePriorityCidrs();
-
-        // if not set frontend_address, get a non-loopback ip
         List<InetAddress> hosts = new ArrayList<>();
         NetUtils.getHosts(hosts);
         if (hosts.isEmpty()) {
             LOG.error("fail to get localhost");
             System.exit(-1);
         }
+        if (Config.enable_fqdn_mode) {
+            initAddrUseFqdn(hosts);

Review Comment:
   ```suggestion
               initAddrUsingFqdn(hosts);
   ```



##########
be/src/util/network_util.cpp:
##########
@@ -104,6 +104,26 @@ Status hostname_to_ip_addrs(const std::string& name, 
std::vector<std::string>* a
     return Status::OK();
 }
 
+bool is_valid_ip(const std::string& ip) {
+    unsigned char buf[sizeof(struct in6_addr)];
+    return inet_pton(AF_INET, ip.data(), buf) > 0;
+}
+
+std::string hostname_to_ip(const std::string& host) {

Review Comment:
   And better to cache the result so that we don't need to convert hostname  to 
ip every time



##########
be/src/util/network_util.cpp:
##########
@@ -104,6 +104,26 @@ Status hostname_to_ip_addrs(const std::string& name, 
std::vector<std::string>* a
     return Status::OK();
 }
 
+bool is_valid_ip(const std::string& ip) {
+    unsigned char buf[sizeof(struct in6_addr)];
+    return inet_pton(AF_INET, ip.data(), buf) > 0;

Review Comment:
   Is it suitable for ipv6?



##########
be/src/agent/heartbeat_server.cpp:
##########
@@ -83,11 +84,56 @@ Status HeartbeatServer::_heartbeat(const TMasterInfo& 
master_info) {
 
     if (master_info.__isset.backend_ip) {
         if (master_info.backend_ip != BackendOptions::get_localhost()) {
-            LOG(WARNING) << "backend ip saved in master does not equal to 
backend local ip"
-                         << master_info.backend_ip << " vs. " << 
BackendOptions::get_localhost();
-            std::stringstream ss;
-            ss << "actual backend local ip: " << 
BackendOptions::get_localhost();
-            return Status::InternalError(ss.str());
+            LOG(INFO) << master_info.backend_ip << " not equal to to backend 
localhost "
+                      << BackendOptions::get_localhost();
+            if (is_valid_ip(master_info.backend_ip)) {
+                LOG(WARNING) << "backend ip saved in master does not equal to 
backend local ip"
+                             << master_info.backend_ip << " vs. "
+                             << BackendOptions::get_localhost();
+                std::stringstream ss;
+                ss << "actual backend local ip: " << 
BackendOptions::get_localhost();
+                return Status::InternalError(ss.str());
+            }
+
+            std::string ip = hostname_to_ip(master_info.backend_ip);
+            if (ip.empty()) {
+                std::stringstream ss;
+                ss << "can not get ip from fqdn: " << master_info.backend_ip;
+                LOG(WARNING) << ss.str();
+                return Status::InternalError(ss.str());
+            }
+
+            std::vector<InetAddress> hosts;
+            Status status = get_hosts(&hosts);
+            if (!status.ok() || hosts.empty()) {
+                std::stringstream ss;
+                ss << "the status was not ok when get_hosts_v4, error is " << 
status.to_string();
+                LOG(WARNING) << ss.str();
+                return Status::InternalError(ss.str());
+            }
+
+            bool set_new_localhost = false;
+            BackendOptions::set_localhost(master_info.backend_ip);
+            for (std::vector<InetAddress>::iterator addr_it = hosts.begin(); 
addr_it != hosts.end();

Review Comment:
   for (auto& addr : hosts) {
   
   }



##########
be/src/util/network_util.cpp:
##########
@@ -104,6 +104,26 @@ Status hostname_to_ip_addrs(const std::string& name, 
std::vector<std::string>* a
     return Status::OK();
 }
 
+bool is_valid_ip(const std::string& ip) {
+    unsigned char buf[sizeof(struct in6_addr)];
+    return inet_pton(AF_INET, ip.data(), buf) > 0;
+}
+
+std::string hostname_to_ip(const std::string& host) {

Review Comment:
   Better return `Status`. And pass out ip addr as a parameter.



##########
be/src/agent/heartbeat_server.cpp:
##########
@@ -83,11 +84,56 @@ Status HeartbeatServer::_heartbeat(const TMasterInfo& 
master_info) {
 
     if (master_info.__isset.backend_ip) {
         if (master_info.backend_ip != BackendOptions::get_localhost()) {
-            LOG(WARNING) << "backend ip saved in master does not equal to 
backend local ip"
-                         << master_info.backend_ip << " vs. " << 
BackendOptions::get_localhost();
-            std::stringstream ss;
-            ss << "actual backend local ip: " << 
BackendOptions::get_localhost();
-            return Status::InternalError(ss.str());
+            LOG(INFO) << master_info.backend_ip << " not equal to to backend 
localhost "

Review Comment:
   Please add comment in code to explain following logic, in format like:
   ```
   step 1:
   step 2:
   ...
   ``



##########
be/src/service/backend_options.cpp:
##########
@@ -91,6 +92,17 @@ const std::string& BackendOptions::get_localhost() {
     return _s_localhost;
 }
 
+TBackend BackendOptions::get_localBackend() {

Review Comment:
   ```suggestion
   TBackend BackendOptions::get_local_backend() {
   ```



-- 
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: commits-unsubscr...@doris.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


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

Reply via email to