levy5307 commented on code in PR #1487:
URL: 
https://github.com/apache/incubator-pegasus/pull/1487#discussion_r1206173178


##########
src/common/fs_manager.cpp:
##########
@@ -110,29 +110,26 @@ uint64_t dir_node::remove(const gpid &pid)
     return iter->second.erase(pid);
 }
 
-bool dir_node::update_disk_stat(const bool update_disk_status)
+void dir_node::update_disk_stat()
 {
-    FAIL_POINT_INJECT_F("update_disk_stat", [](string_view) { return false; });
-    dsn::utils::filesystem::disk_space_info info;
-    if (!dsn::utils::filesystem::get_disk_space_info(full_dir, info)) {
-        LOG_ERROR("update disk space failed: dir = {}", full_dir);
-        return false;
+    FAIL_POINT_INJECT_F("update_disk_stat", [](string_view) { return; });
+
+    // Get disk space info.
+    dsn::utils::filesystem::disk_space_info dsi;
+    if (!dsn::utils::filesystem::get_disk_space_info(full_dir, dsi)) {
+        // TODO(yingchun): it may encounter some IO errors when 
get_disk_space_info() failed, deal
+        //  with it.
+        LOG_ERROR("update disk space failed, dir = {}", full_dir);

Review Comment:
   LOG_ERROR("get disk space info failed, dir = {}", full_dir);



##########
src/common/fs_manager.cpp:
##########
@@ -110,29 +110,26 @@ uint64_t dir_node::remove(const gpid &pid)
     return iter->second.erase(pid);
 }
 
-bool dir_node::update_disk_stat(const bool update_disk_status)
+void dir_node::update_disk_stat()
 {
-    FAIL_POINT_INJECT_F("update_disk_stat", [](string_view) { return false; });
-    dsn::utils::filesystem::disk_space_info info;
-    if (!dsn::utils::filesystem::get_disk_space_info(full_dir, info)) {
-        LOG_ERROR("update disk space failed: dir = {}", full_dir);
-        return false;
+    FAIL_POINT_INJECT_F("update_disk_stat", [](string_view) { return; });
+
+    // Get disk space info.

Review Comment:
   remove this comment



##########
src/common/fs_manager.cpp:
##########
@@ -341,17 +337,16 @@ bool fs_manager::for_each_dir_node(const 
std::function<bool(const dir_node &)> &
     return true;
 }
 
-void fs_manager::update_disk_stat(bool check_status_changed)
+void fs_manager::update_disk_stat()
 {
+    zauto_read_lock l(_lock);

Review Comment:
   Why did you add a read lock here?



##########
src/common/fs_manager.cpp:
##########
@@ -110,29 +110,26 @@ uint64_t dir_node::remove(const gpid &pid)
     return iter->second.erase(pid);
 }
 
-bool dir_node::update_disk_stat(const bool update_disk_status)
+void dir_node::update_disk_stat()
 {
-    FAIL_POINT_INJECT_F("update_disk_stat", [](string_view) { return false; });
-    dsn::utils::filesystem::disk_space_info info;
-    if (!dsn::utils::filesystem::get_disk_space_info(full_dir, info)) {
-        LOG_ERROR("update disk space failed: dir = {}", full_dir);
-        return false;
+    FAIL_POINT_INJECT_F("update_disk_stat", [](string_view) { return; });
+
+    // Get disk space info.
+    dsn::utils::filesystem::disk_space_info dsi;
+    if (!dsn::utils::filesystem::get_disk_space_info(full_dir, dsi)) {
+        // TODO(yingchun): it may encounter some IO errors when 
get_disk_space_info() failed, deal
+        //  with it.
+        LOG_ERROR("update disk space failed, dir = {}", full_dir);
+        return;
     }
-    // update disk space info
-    disk_capacity_mb = info.capacity / 1024 / 1024;
-    disk_available_mb = info.available / 1024 / 1024;
+
+    // Update in-memory disk space info.
+    disk_capacity_mb = dsi.capacity >> 20;
+    disk_available_mb = dsi.available >> 20;
     disk_available_ratio = static_cast<int>(
         disk_capacity_mb == 0 ? 0 : std::round(disk_available_mb * 100.0 / 
disk_capacity_mb));
 
-    if (!update_disk_status) {
-        LOG_INFO("update disk space succeed: dir = {}, capacity_mb = {}, 
available_mb = {}, "
-                 "available_ratio = {}%",
-                 full_dir,
-                 disk_capacity_mb,
-                 disk_available_mb,
-                 disk_available_ratio);
-        return false;
-    }
+    // Update status.

Review Comment:
   In my opinion, the comment is useless



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to