You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brpc.apache.org by GitBox <gi...@apache.org> on 2022/06/23 02:36:28 UTC

[GitHub] [incubator-brpc] chenBright opened a new pull request, #1818: fix hostname2ip fails when aux_buf is not long enough

chenBright opened a new pull request, #1818:
URL: https://github.com/apache/incubator-brpc/pull/1818

   使用Channel::Init("http://hostname", options)时,会在hostname2ip中通过gethostbyname_r获取dns的节点信息。如果hostname在dns中配置了很多节点,hostname2ip会因为aux_buf长度不够导致调用失败,从而导致Channel::Init失败。
   
   解决方法:参考DomainNamingService获取节点信息的方法,当gethostbyname_r返回ERANGE,表示aux_buf长度不足时,将aux_buf长度增大一倍,重新调用gethostbyname_r获取节点信息,直到获取成功为止。


-- 
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-unsubscribe@brpc.apache.org

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


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


[GitHub] [incubator-brpc] chenBright commented on a diff in pull request #1818: fix hostname2ip fails when aux_buf is not long enough

Posted by GitBox <gi...@apache.org>.
chenBright commented on code in PR #1818:
URL: https://github.com/apache/incubator-brpc/pull/1818#discussion_r904489040


##########
src/butil/endpoint.cpp:
##########
@@ -193,12 +195,33 @@ int hostname2ip(const char* hostname, ip_t* ip) {
         return -1;
     }
 #else
-    char aux_buf[1024];
+    int aux_buf_len = 1024;
+    std::unique_ptr<char[]> aux_buf(new char[aux_buf_len]);
+    int ret = 0;
     int error = 0;
     struct hostent ent;
     struct hostent* result = NULL;
-    if (gethostbyname_r(hostname, &ent, aux_buf, sizeof(aux_buf),
-                        &result, &error) != 0 || result == NULL) {
+    do {
+        result = NULL;
+        error = 0;
+        ret = gethostbyname_r(hostname, &ent, aux_buf.get(), aux_buf_len,
+            &result, &error);
+        if (ret != ERANGE) { // aux_buf is not long enough
+            break;
+        }
+        aux_buf_len *= 2;
+        aux_buf.reset(new char[aux_buf_len]);
+        RPC_VLOG << "Resized aux_buf to " << aux_buf_len
+                 << ", hostname=" << hostname;
+    } while (1);
+    if (ret != 0) {
+        // `hstrerror' is thread safe under linux
+        LOG(WARNING) << "Can't resolve `" << hostname << "', return=`" << berror(ret)

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.

To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org

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


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


[GitHub] [incubator-brpc] chenBright commented on a diff in pull request #1818: fix hostname2ip fails when aux_buf is not long enough

Posted by GitBox <gi...@apache.org>.
chenBright commented on code in PR #1818:
URL: https://github.com/apache/incubator-brpc/pull/1818#discussion_r904503039


##########
src/butil/endpoint.cpp:
##########
@@ -193,12 +195,33 @@ int hostname2ip(const char* hostname, ip_t* ip) {
         return -1;
     }
 #else
-    char aux_buf[1024];
+    int aux_buf_len = 1024;
+    std::unique_ptr<char[]> aux_buf(new char[aux_buf_len]);
+    int ret = 0;
     int error = 0;
     struct hostent ent;
     struct hostent* result = NULL;
-    if (gethostbyname_r(hostname, &ent, aux_buf, sizeof(aux_buf),
-                        &result, &error) != 0 || result == NULL) {
+    do {
+        result = NULL;
+        error = 0;
+        ret = gethostbyname_r(hostname, &ent, aux_buf.get(), aux_buf_len,
+            &result, &error);
+        if (ret != ERANGE) { // aux_buf is not long enough
+            break;
+        }
+        aux_buf_len *= 2;
+        aux_buf.reset(new char[aux_buf_len]);
+        RPC_VLOG << "Resized aux_buf to " << aux_buf_len
+                 << ", hostname=" << hostname;
+    } while (1);
+    if (ret != 0) {
+        // `hstrerror' is thread safe under linux
+        LOG(WARNING) << "Can't resolve `" << hostname << "', return=`" << berror(ret)

Review Comment:
   get,已删除



-- 
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-unsubscribe@brpc.apache.org

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


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


[GitHub] [incubator-brpc] serverglen commented on a diff in pull request #1818: fix hostname2ip fails when aux_buf is not long enough

Posted by GitBox <gi...@apache.org>.
serverglen commented on code in PR #1818:
URL: https://github.com/apache/incubator-brpc/pull/1818#discussion_r906157418


##########
src/butil/endpoint.cpp:
##########
@@ -33,6 +33,7 @@
 #include "butil/memory/singleton_on_pthread_once.h"
 #include "butil/strings/string_piece.h"
 #include <sys/socket.h>                        // SO_REUSEADDR SO_REUSEPORT

Review Comment:
   这两个头文件建议移到28行后面,保持统一



-- 
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-unsubscribe@brpc.apache.org

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


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


[GitHub] [incubator-brpc] wwbmmm commented on pull request #1818: fix hostname2ip fails when aux_buf is not long enough

Posted by GitBox <gi...@apache.org>.
wwbmmm commented on PR #1818:
URL: https://github.com/apache/incubator-brpc/pull/1818#issuecomment-1163921230

   LGTM


-- 
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-unsubscribe@brpc.apache.org

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


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


[GitHub] [incubator-brpc] chenBright commented on a diff in pull request #1818: fix hostname2ip fails when aux_buf is not long enough

Posted by GitBox <gi...@apache.org>.
chenBright commented on code in PR #1818:
URL: https://github.com/apache/incubator-brpc/pull/1818#discussion_r904495898


##########
src/butil/endpoint.cpp:
##########
@@ -32,7 +32,9 @@
 #include "butil/logging.h"
 #include "butil/memory/singleton_on_pthread_once.h"
 #include "butil/strings/string_piece.h"
+#include "brpc/log.h"

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.

To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org

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


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


[GitHub] [incubator-brpc] serverglen commented on a diff in pull request #1818: fix hostname2ip fails when aux_buf is not long enough

Posted by GitBox <gi...@apache.org>.
serverglen commented on code in PR #1818:
URL: https://github.com/apache/incubator-brpc/pull/1818#discussion_r906157418


##########
src/butil/endpoint.cpp:
##########
@@ -33,6 +33,7 @@
 #include "butil/memory/singleton_on_pthread_once.h"
 #include "butil/strings/string_piece.h"
 #include <sys/socket.h>                        // SO_REUSEADDR SO_REUSEPORT

Review Comment:
   这两个头文件要不移到28行后面,保持统一



-- 
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-unsubscribe@brpc.apache.org

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


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


[GitHub] [incubator-brpc] chenBright commented on a diff in pull request #1818: fix hostname2ip fails when aux_buf is not long enough

Posted by GitBox <gi...@apache.org>.
chenBright commented on code in PR #1818:
URL: https://github.com/apache/incubator-brpc/pull/1818#discussion_r904621129


##########
src/butil/endpoint.cpp:
##########
@@ -193,12 +194,24 @@ int hostname2ip(const char* hostname, ip_t* ip) {
         return -1;
     }
 #else
-    char aux_buf[1024];
+    int aux_buf_len = 1024;
+    std::unique_ptr<char[]> aux_buf(new char[aux_buf_len]);
+    int ret = 0;
     int error = 0;
     struct hostent ent;
     struct hostent* result = NULL;
-    if (gethostbyname_r(hostname, &ent, aux_buf, sizeof(aux_buf),
-                        &result, &error) != 0 || result == NULL) {
+    do {
+        result = NULL;
+        error = 0;
+        ret = gethostbyname_r(hostname, &ent, aux_buf.get(), aux_buf_len,
+            &result, &error);

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.

To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org

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


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


[GitHub] [incubator-brpc] wwbmmm commented on a diff in pull request #1818: fix hostname2ip fails when aux_buf is not long enough

Posted by GitBox <gi...@apache.org>.
wwbmmm commented on code in PR #1818:
URL: https://github.com/apache/incubator-brpc/pull/1818#discussion_r904500998


##########
src/butil/endpoint.cpp:
##########
@@ -193,12 +195,33 @@ int hostname2ip(const char* hostname, ip_t* ip) {
         return -1;
     }
 #else
-    char aux_buf[1024];
+    int aux_buf_len = 1024;
+    std::unique_ptr<char[]> aux_buf(new char[aux_buf_len]);
+    int ret = 0;
     int error = 0;
     struct hostent ent;
     struct hostent* result = NULL;
-    if (gethostbyname_r(hostname, &ent, aux_buf, sizeof(aux_buf),
-                        &result, &error) != 0 || result == NULL) {
+    do {
+        result = NULL;
+        error = 0;
+        ret = gethostbyname_r(hostname, &ent, aux_buf.get(), aux_buf_len,
+            &result, &error);
+        if (ret != ERANGE) { // aux_buf is not long enough
+            break;
+        }
+        aux_buf_len *= 2;
+        aux_buf.reset(new char[aux_buf_len]);
+        RPC_VLOG << "Resized aux_buf to " << aux_buf_len
+                 << ", hostname=" << hostname;
+    } while (1);
+    if (ret != 0) {
+        // `hstrerror' is thread safe under linux
+        LOG(WARNING) << "Can't resolve `" << hostname << "', return=`" << berror(ret)

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.

To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org

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


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


[GitHub] [incubator-brpc] chenBright commented on a diff in pull request #1818: fix hostname2ip fails when aux_buf is not long enough

Posted by GitBox <gi...@apache.org>.
chenBright commented on code in PR #1818:
URL: https://github.com/apache/incubator-brpc/pull/1818#discussion_r906232911


##########
src/butil/endpoint.cpp:
##########
@@ -33,6 +33,7 @@
 #include "butil/memory/singleton_on_pthread_once.h"
 #include "butil/strings/string_piece.h"
 #include <sys/socket.h>                        // SO_REUSEADDR SO_REUSEPORT

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.

To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org

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


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


[GitHub] [incubator-brpc] wwbmmm commented on pull request #1818: fix hostname2ip fails when aux_buf is not long enough

Posted by GitBox <gi...@apache.org>.
wwbmmm commented on PR #1818:
URL: https://github.com/apache/incubator-brpc/pull/1818#issuecomment-1170876830

   > @serverglen 修改了头文件顺序就coredump了?这个要怎么debug呢?
   
   那个core是因为test_butil里面有的case是预期要出core的,是正常的。
   导致CI失败的原因是LoadBalancerTest.revived_from_all_failed_sanity 这个case,这个可能跟你的修改没有关系,可能是case不稳定。
   不知道为啥点不了CI重试了,你要不重新push一次,再触发一次CI试试


-- 
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-unsubscribe@brpc.apache.org

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


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


[GitHub] [incubator-brpc] serverglen commented on pull request #1818: fix hostname2ip fails when aux_buf is not long enough

Posted by GitBox <gi...@apache.org>.
serverglen commented on PR #1818:
URL: https://github.com/apache/incubator-brpc/pull/1818#issuecomment-1171888466

   LGTM


-- 
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-unsubscribe@brpc.apache.org

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


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


[GitHub] [incubator-brpc] chenBright commented on pull request #1818: fix hostname2ip fails when aux_buf is not long enough

Posted by GitBox <gi...@apache.org>.
chenBright commented on PR #1818:
URL: https://github.com/apache/incubator-brpc/pull/1818#issuecomment-1171874693

   @wwbmmm 好了


-- 
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-unsubscribe@brpc.apache.org

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


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


[GitHub] [incubator-brpc] wasphin commented on a diff in pull request #1818: fix hostname2ip fails when aux_buf is not long enough

Posted by GitBox <gi...@apache.org>.
wasphin commented on code in PR #1818:
URL: https://github.com/apache/incubator-brpc/pull/1818#discussion_r904520518


##########
src/butil/endpoint.cpp:
##########
@@ -193,12 +194,24 @@ int hostname2ip(const char* hostname, ip_t* ip) {
         return -1;
     }
 #else
-    char aux_buf[1024];
+    int aux_buf_len = 1024;
+    std::unique_ptr<char[]> aux_buf(new char[aux_buf_len]);
+    int ret = 0;
     int error = 0;
     struct hostent ent;
     struct hostent* result = NULL;
-    if (gethostbyname_r(hostname, &ent, aux_buf, sizeof(aux_buf),
-                        &result, &error) != 0 || result == NULL) {
+    do {
+        result = NULL;
+        error = 0;
+        ret = gethostbyname_r(hostname, &ent, aux_buf.get(), aux_buf_len,
+            &result, &error);

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.

To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org

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


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


[GitHub] [incubator-brpc] chenBright commented on pull request #1818: fix hostname2ip fails when aux_buf is not long enough

Posted by GitBox <gi...@apache.org>.
chenBright commented on PR #1818:
URL: https://github.com/apache/incubator-brpc/pull/1818#issuecomment-1169600910

   @serverglen 修改了头文件顺序就coredump了?这个要怎么debug呢?


-- 
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-unsubscribe@brpc.apache.org

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


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


[GitHub] [incubator-brpc] zyearn merged pull request #1818: fix hostname2ip fails when aux_buf is not long enough

Posted by GitBox <gi...@apache.org>.
zyearn merged PR #1818:
URL: https://github.com/apache/incubator-brpc/pull/1818


-- 
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-unsubscribe@brpc.apache.org

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


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


[GitHub] [incubator-brpc] serverglen commented on pull request #1818: fix hostname2ip fails when aux_buf is not long enough

Posted by GitBox <gi...@apache.org>.
serverglen commented on PR #1818:
URL: https://github.com/apache/incubator-brpc/pull/1818#issuecomment-1170649521

   > @serverglen 修改了头文件顺序就coredump了?这个要怎么debug呢?
   
   gdb看下堆栈呢?


-- 
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-unsubscribe@brpc.apache.org

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


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


[GitHub] [incubator-brpc] wwbmmm commented on a diff in pull request #1818: fix hostname2ip fails when aux_buf is not long enough

Posted by GitBox <gi...@apache.org>.
wwbmmm commented on code in PR #1818:
URL: https://github.com/apache/incubator-brpc/pull/1818#discussion_r904477754


##########
src/butil/endpoint.cpp:
##########
@@ -193,12 +195,33 @@ int hostname2ip(const char* hostname, ip_t* ip) {
         return -1;
     }
 #else
-    char aux_buf[1024];
+    int aux_buf_len = 1024;
+    std::unique_ptr<char[]> aux_buf(new char[aux_buf_len]);
+    int ret = 0;
     int error = 0;
     struct hostent ent;
     struct hostent* result = NULL;
-    if (gethostbyname_r(hostname, &ent, aux_buf, sizeof(aux_buf),
-                        &result, &error) != 0 || result == NULL) {
+    do {
+        result = NULL;
+        error = 0;
+        ret = gethostbyname_r(hostname, &ent, aux_buf.get(), aux_buf_len,
+            &result, &error);
+        if (ret != ERANGE) { // aux_buf is not long enough
+            break;
+        }
+        aux_buf_len *= 2;
+        aux_buf.reset(new char[aux_buf_len]);
+        RPC_VLOG << "Resized aux_buf to " << aux_buf_len
+                 << ", hostname=" << hostname;
+    } while (1);
+    if (ret != 0) {
+        // `hstrerror' is thread safe under linux
+        LOG(WARNING) << "Can't resolve `" << hostname << "', return=`" << berror(ret)

Review Comment:
   这个函数比较底层,可能不适合打WARNING,因为有些场景解析不到是符合预期的。



##########
src/butil/endpoint.cpp:
##########
@@ -32,7 +32,9 @@
 #include "butil/logging.h"
 #include "butil/memory/singleton_on_pthread_once.h"
 #include "butil/strings/string_piece.h"
+#include "brpc/log.h"

Review Comment:
   butil反过来依赖brpc目录的文件不太合适



-- 
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-unsubscribe@brpc.apache.org

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


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