fix: parse timestamp from the name of data dir for gc instead of the last update time

Author: empiredan

src/replica/disk_cleaner.cpp

@@ -77,17 +77,21 @@ const std::string kFolderSuffixTmp = ".tmp";
 
 namespace {
 
-bool get_expiration_seconds_by_last_write_time(const std::string &path,
-                                               uint64_t delay_seconds,
-                                               uint64_t &expiration_seconds)
+// TODO(wangdan): we could study later whether ctime (i.e. `st_ctime` within `struct stat`,
+// the time of last status change) could be used instead of mtime (i.e. `st_ctime` within
+// `struct stat`, the last write time), since ctime of the new directory would be updated
+// to the current time once rename() is called, while mtime would not be updated.
+bool get_expiration_timestamp_by_last_write_time(const std::string &path,
+                                                 uint64_t delay_seconds,
+                                                 uint64_t &expiration_timestamp_s)
 {
-    time_t last_write_seconds;
-    if (!dsn::utils::filesystem::last_write_time(path, last_write_seconds)) {
+    time_t last_write_time_s;
+    if (!dsn::utils::filesystem::last_write_time(path, last_write_time_s)) {
         LOG_WARNING("gc_disk: failed to get last write time of {}", path);
         return false;
     }
 
-    expiration_seconds = static_cast<uint64_t>(last_write_seconds) + delay_seconds;
+    expiration_timestamp_s = static_cast<uint64_t>(last_write_time_s) + delay_seconds;
     return true;
 }
 
@@ -97,12 +101,24 @@ bool get_expiration_seconds_by_last_write_time(const std::string &path,
 #define MIN_TIMESTAMP_US 1262275200000000
 #define MIN_TIMESTAMP_US_LENGTH (sizeof(STRINGIFY(MIN_TIMESTAMP_US)) - 1)
 
+// Parse timestamp from the directory name.
+//
+// There are only 2 kinds of directory names that could include timestamp: one is the faulty
+// replicas whose name has suffix ".err"; another is the dropped replicas whose name has
+// suffix ".gar". The examples for both kinds of directory names:
+//     1.1.pegasus.1698843209235962.err
+//     1.2.pegasus.1698843214240709.gar
+//
+// Specify the size of suffix by `suffix_size`. For both kinds of names (.err and .gar),
+// `suffix_size` is 4.
+//
+// The timestamp is the number the number just before the suffix, between the 2 dots. For
+// example, in 1.1.pegasus.1698843209235962.err, 1698843209235962 is the timestamp, in
+// microseconds, generated by dsn_now_us().
+//
+// `timestamp_us` is parsed result while returning true; otherwise, it would never be assigned.
 bool parse_timestamp_us(const std::string &name, size_t suffix_size, uint64_t &timestamp_us)
 {
-    // Examples of the directory names of faulty or dropped replicas could be:
-    // 1.1.pegasus.1698843209235962.err
-    // 2.1.pegasus.1698843214240709.gar
-
     CHECK_GE(name.size(), suffix_size);
 
     if (suffix_size == name.size()) {
@@ -137,11 +153,11 @@ bool parse_timestamp_us(const std::string &name, size_t suffix_size, uint64_t &t
     return ok ? timestamp_us > MIN_TIMESTAMP_US : false;
 }
 
-bool get_expiration_seconds_by_timestamp(const std::string &name,
-                                         const std::string &path,
-                                         size_t suffix_size,
-                                         uint64_t delay_seconds,
-                                         uint64_t &expiration_seconds)
+bool get_expiration_timestamp(const std::string &name,
+                              const std::string &path,
+                              size_t suffix_size,
+                              uint64_t delay_seconds,
+                              uint64_t &expiration_timestamp_s)
 {
     uint64_t timestamp_us = 0;
     if (!parse_timestamp_us(name, suffix_size, timestamp_us)) {
@@ -151,10 +167,11 @@ bool get_expiration_seconds_by_timestamp(const std::string &name,
                     "the last write time for {}",
                     name,
                     path);
-        return get_expiration_seconds_by_last_write_time(path, delay_seconds, expiration_seconds);
+        return get_expiration_timestamp_by_last_write_time(
+            path, delay_seconds, expiration_timestamp_s);
     }
 
-    expiration_seconds = timestamp_us / 1000000 + delay_seconds;
+    expiration_timestamp_s = timestamp_us / 1000000 + delay_seconds;
     return true;
 }
 
@@ -179,50 +196,50 @@ error_s disk_remove_useless_dirs(const std::vector<std::shared_ptr<dir_node>> &d
     }
 
     for (const auto &path : sub_list) {
-        uint64_t expiration_seconds = 0;
+        uint64_t expiration_timestamp_s = 0;
 
         // Note: don't delete ".bak" directory since it could be did by administrator.
         const auto name = dsn::utils::filesystem::get_file_name(path);
         if (boost::algorithm::ends_with(name, kFolderSuffixErr)) {
             report.error_replica_count++;
-            if (!get_expiration_seconds_by_timestamp(name,
-                                                     path,
-                                                     kFolderSuffixErr.size(),
-                                                     FLAGS_gc_disk_error_replica_interval_seconds,
-                                                     expiration_seconds)) {
+            if (!get_expiration_timestamp(name,
+                                          path,
+                                          kFolderSuffixErr.size(),
+                                          FLAGS_gc_disk_error_replica_interval_seconds,
+                                          expiration_timestamp_s)) {
                 continue;
             }
         } else if (boost::algorithm::ends_with(name, kFolderSuffixGar)) {
             report.garbage_replica_count++;
-            if (!get_expiration_seconds_by_timestamp(name,
-                                                     path,
-                                                     kFolderSuffixGar.size(),
-                                                     FLAGS_gc_disk_garbage_replica_interval_seconds,
-                                                     expiration_seconds)) {
+            if (!get_expiration_timestamp(name,
+                                          path,
+                                          kFolderSuffixGar.size(),
+                                          FLAGS_gc_disk_garbage_replica_interval_seconds,
+                                          expiration_timestamp_s)) {
                 continue;
             }
         } else if (boost::algorithm::ends_with(name, kFolderSuffixTmp)) {
             report.disk_migrate_tmp_count++;
-            if (!get_expiration_seconds_by_last_write_time(
+            if (!get_expiration_timestamp_by_last_write_time(
                     path,
                     FLAGS_gc_disk_migration_tmp_replica_interval_seconds,
-                    expiration_seconds)) {
+                    expiration_timestamp_s)) {
                 continue;
             }
         } else if (boost::algorithm::ends_with(name, kFolderSuffixOri)) {
             report.disk_migrate_origin_count++;
-            if (!get_expiration_seconds_by_last_write_time(
+            if (!get_expiration_timestamp_by_last_write_time(
                     path,
                     FLAGS_gc_disk_migration_origin_replica_interval_seconds,
-                    expiration_seconds)) {
+                    expiration_timestamp_s)) {
                 continue;
             }
         } else {
             continue;
         }
 
         const auto current_time_ms = dsn_now_ms();
-        if (expiration_seconds <= current_time_ms / 1000) {
+        if (expiration_timestamp_s <= current_time_ms / 1000) {
             if (dsn::utils::filesystem::remove_path(path)) {
                 LOG_WARNING("gc_disk: replica_dir_op succeed to delete directory '{}'"
                             ", time_used_ms = {}",
@@ -237,7 +254,7 @@ error_s disk_remove_useless_dirs(const std::vector<std::shared_ptr<dir_node>> &d
         } else {
             LOG_INFO("gc_disk: reserve directory '{}', wait_seconds = {}",
                      path,
-                     expiration_seconds - current_time_ms / 1000);
+                     expiration_timestamp_s - current_time_ms / 1000);
         }
     }
     return error_s::ok();

src/replica/test/replica_disk_test.cpp

@@ -200,6 +200,28 @@ TEST_P(replica_disk_test, on_query_disk_info_one_app)
     }
 }
 
+TEST_P(replica_disk_test, check_data_dir_removable)
+{
+    struct test_case
+    {
+        std::string path;
+        bool expected_removable;
+        bool expected_invalid;
+    } tests[] = {{"./replica.0.err", true, true},
+                 {"./replica.1.gar", true, true},
+                 {"./replica.2.tmp", true, true},
+                 {"./replica.3.ori", true, true},
+                 {"./replica.4.bak", false, true},
+                 {"./replica.5.abcde", false, false},
+                 {"./replica.6.x", false, false},
+                 {"./replica.7.8", false, false}};
+
+    for (const auto &test : tests) {
+        EXPECT_EQ(test.expected_removable, is_data_dir_removable(test.path));
+        EXPECT_EQ(test.expected_invalid, is_data_dir_invalid(test.path));
+    }
+}
+
 TEST_P(replica_disk_test, gc_disk_useless_dir)
 {
     PRESERVE_FLAG(gc_disk_error_replica_interval_seconds);