Re: [PR] [Enhancement](metrics) Avoid update_process_fd_num reporting no such file error indiscriminately. [doris]

[via GitHub]

JackDrogon commented on code in PR #26013:
URL: https://github.com/apache/doris/pull/26013#discussion_r1375726261


##########
be/src/util/doris_metrics.cpp:
##########
@@ -341,59 +341,51 @@ void DorisMetrics::_update() {
 }
 
 // get num of thread of doris_be process
-// from /proc/pid/task
+// from /proc/self/task
 void DorisMetrics::_update_process_thread_num() {
-    int64_t pid = getpid();
-    std::stringstream ss;
-    ss << "/proc/" << pid << "/task/";
-
-    int64_t count = 0;
-    auto cb = [&count](const io::FileInfo& file) -> bool {
-        count += 1;
-        return true;
-    };
-    Status st = io::global_local_filesystem()->iterate_directory(ss.str(), cb);
-    if (!st.ok()) {
-        LOG(WARNING) << "failed to count thread num: " << st;
-        process_thread_num->set_value(0);
+    std::error_code ec;
+    std::filesystem::directory_iterator dict_iter("/proc/self/task/", ec);
+    if (ec) {
+        LOG(WARNING) << "failed to count thread num: " << ec.message();
+        process_fd_num_used->set_value(0);
         return;
     }
+    int64_t count = std::count_if(dict_iter, 
std::filesystem::directory_iterator(), [](const auto& entry) {
+        std::error_code ec;
+        return entry.is_regular_file(ec) && !ec;
+    });
 
     process_thread_num->set_value(count);
 }
 
 // get num of file descriptor of doris_be process
 void DorisMetrics::_update_process_fd_num() {
-    int64_t pid = getpid();
-
     // fd used
-    std::stringstream ss;
-    ss << "/proc/" << pid << "/fd/";
     int64_t count = 0;
-    auto cb = [&count](const io::FileInfo& file) -> bool {
-        count += 1;
-        return true;
-    };
-    Status st = io::global_local_filesystem()->iterate_directory(ss.str(), cb);
-    if (!st.ok()) {
-        LOG(WARNING) << "failed to count fd: " << st;
+    std::error_code ec;

Review Comment:
   remove these unused shadow variable



##########
be/src/util/doris_metrics.cpp:
##########
@@ -341,59 +341,51 @@ void DorisMetrics::_update() {
 }
 
 // get num of thread of doris_be process
-// from /proc/pid/task
+// from /proc/self/task
 void DorisMetrics::_update_process_thread_num() {
-    int64_t pid = getpid();
-    std::stringstream ss;
-    ss << "/proc/" << pid << "/task/";
-
-    int64_t count = 0;
-    auto cb = [&count](const io::FileInfo& file) -> bool {
-        count += 1;
-        return true;
-    };
-    Status st = io::global_local_filesystem()->iterate_directory(ss.str(), cb);
-    if (!st.ok()) {
-        LOG(WARNING) << "failed to count thread num: " << st;
-        process_thread_num->set_value(0);
+    std::error_code ec;
+    std::filesystem::directory_iterator dict_iter("/proc/self/task/", ec);
+    if (ec) {
+        LOG(WARNING) << "failed to count thread num: " << ec.message();
+        process_fd_num_used->set_value(0);
         return;
     }
+    int64_t count = std::count_if(dict_iter, 
std::filesystem::directory_iterator(), [](const auto& entry) {

Review Comment:
   use `std::filesystem::end(dict_iter)` to replace 
`std::filesystem::directory_iterator()`, 
`std::filesystem::directory_iterator()` is ub in end



##########
be/src/util/doris_metrics.cpp:
##########
@@ -341,59 +341,51 @@ void DorisMetrics::_update() {
 }
 
 // get num of thread of doris_be process
-// from /proc/pid/task
+// from /proc/self/task
 void DorisMetrics::_update_process_thread_num() {
-    int64_t pid = getpid();
-    std::stringstream ss;
-    ss << "/proc/" << pid << "/task/";
-
-    int64_t count = 0;
-    auto cb = [&count](const io::FileInfo& file) -> bool {
-        count += 1;
-        return true;
-    };
-    Status st = io::global_local_filesystem()->iterate_directory(ss.str(), cb);
-    if (!st.ok()) {
-        LOG(WARNING) << "failed to count thread num: " << st;
-        process_thread_num->set_value(0);
+    std::error_code ec;
+    std::filesystem::directory_iterator dict_iter("/proc/self/task/", ec);
+    if (ec) {
+        LOG(WARNING) << "failed to count thread num: " << ec.message();
+        process_fd_num_used->set_value(0);
         return;
     }
+    int64_t count = std::count_if(dict_iter, 
std::filesystem::directory_iterator(), [](const auto& entry) {
+        std::error_code ec;
+        return entry.is_regular_file(ec) && !ec;
+    });
 
     process_thread_num->set_value(count);
 }
 
 // get num of file descriptor of doris_be process
 void DorisMetrics::_update_process_fd_num() {
-    int64_t pid = getpid();
-
     // fd used
-    std::stringstream ss;
-    ss << "/proc/" << pid << "/fd/";
     int64_t count = 0;
-    auto cb = [&count](const io::FileInfo& file) -> bool {
-        count += 1;
-        return true;
-    };
-    Status st = io::global_local_filesystem()->iterate_directory(ss.str(), cb);
-    if (!st.ok()) {
-        LOG(WARNING) << "failed to count fd: " << st;
+    std::error_code ec;
+    std::filesystem::directory_iterator dict_iter("/proc/self/fd/", ec);
+    if (ec) {
+        LOG(WARNING) << "failed to count fd: " << ec.message();
         process_fd_num_used->set_value(0);
         return;
     }
+    int64_t count = std::count_if(dict_iter, 
std::filesystem::fs::directory_iterator(), [](const auto& entry) {
+        std::error_code ec;

Review Comment:
   not use shadow variable, just use dec or something else



-- 
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