fix: parse timestamp from the name of data dir for gc instead of the …

…last update time

src/replica/disk_cleaner.cpp

@@ -34,7 +34,9 @@
 #include "utils/filesystem.h"
 #include "utils/flags.h"
 #include "utils/fmt_logging.h"
+#include "utils/macros.h"
 #include "utils/string_conv.h"
+#include "utils/string_view.h"
 
 namespace dsn {
 namespace replication {
@@ -75,6 +77,24 @@ 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)
+{
+    time_t last_write_seconds;
+    if (!dsn::utils::filesystem::last_write_time(path, last_write_seconds)) {
+        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;
+    return true;
+}
+
+// Unix timestamp in microseconds for 2010-01-01 00:00:00.
+#define MIN_TIMESTAMP_US 1262275200000000
+#define MIN_TIMESTAMP_US_LENGTH (sizeof(STRINGIFY(MIN_TIMESTAMP_US)) - 1)
+
 bool parse_timestamp_us(const std::string &name, size_t suffix_size, uint64_t &timestamp_us)
 {
     CHECK_GE(name.size(), suffix_size);
@@ -89,38 +109,40 @@ bool parse_timestamp_us(const std::string &name, size_t suffix_size, uint64_t &t
         return false;
     }
 
-    return dsn::buf2uint64(dsn::string_view(name.data() + begin_idx, end_idx - begin_idx),
-                           timestamp_us);
+    const auto length = end_idx - begin_idx;
+    if (length < MIN_TIMESTAMP_US_LENGTH) {
+        return false;
+    }
+
+    const auto begin_itr = name.cbegin() + begin_idx;
+    if (!std::all_of(begin_itr, begin_itr + length, ::isdigit)) {
+        return false;
+    }
+
+    const auto ok =
+        dsn::buf2uint64(dsn::string_view(name.data() + begin_idx, length), timestamp_us);
+    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)
 {
     uint64_t timestamp_us = 0;
     if (!parse_timestamp_us(name, suffix_size, timestamp_us)) {
-        return false;
+        LOG_WARNING("gc_disk: failed to parse timestamp from {}, turn to "
+                    "the last write time for {}",
+                    name,
+                    path);
+        return get_expiration_seconds_by_last_write_time(path, delay_seconds, expiration_seconds);
     }
 
     expiration_seconds = timestamp_us / 1000000 + delay_seconds;
     return true;
 }
 
-bool get_expiration_seconds_by_last_write_time(const std::string &path,
-                                               uint64_t delay_seconds,
-                                               uint64_t &expiration_seconds)
-{
-    time_t last_write_seconds;
-    if (!dsn::utils::filesystem::last_write_time(path, last_write_seconds)) {
-        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;
-    return true;
-}
-
 } // anonymous namespace
 
 error_s disk_remove_useless_dirs(const std::vector<std::shared_ptr<dir_node>> &dir_nodes,
@@ -141,43 +163,41 @@ error_s disk_remove_useless_dirs(const std::vector<std::shared_ptr<dir_node>> &d
         sub_list.insert(sub_list.end(), tmp_list.begin(), tmp_list.end());
     }
 
-    for (const auto &fpath : sub_list) {
-        auto name = dsn::utils::filesystem::get_file_name(fpath);
-        if (!is_data_dir_removable(name)) {
-            continue;
-        }
-
+    for (const auto &path : sub_list) {
         uint64_t expiration_seconds = 0;
 
-        // don't delete ".bak" directory because it is backed by administrator.
+        // 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)) {
                 continue;
             }
         } else if (boost::algorithm::ends_with(name, kFolderSuffixGar)) {
             report.garbage_replica_count++;
-            if (get_expiration_seconds_by_timestamp(name,
-                                                    kFolderSuffixGar.size(),
-                                                    FLAGS_gc_disk_garbage_replica_interval_seconds,
-                                                    expiration_seconds)) {
+            if (!get_expiration_seconds_by_timestamp(name,
+                                                     path,
+                                                     kFolderSuffixGar.size(),
+                                                     FLAGS_gc_disk_garbage_replica_interval_seconds,
+                                                     expiration_seconds)) {
                 continue;
             }
         } else if (boost::algorithm::ends_with(name, kFolderSuffixTmp)) {
             report.disk_migrate_tmp_count++;
             if (!get_expiration_seconds_by_last_write_time(
-                    fpath,
+                    path,
                     FLAGS_gc_disk_migration_tmp_replica_interval_seconds,
                     expiration_seconds)) {
                 continue;
             }
         } else if (boost::algorithm::ends_with(name, kFolderSuffixOri)) {
             report.disk_migrate_origin_count++;
             if (!get_expiration_seconds_by_last_write_time(
-                    fpath,
+                    path,
                     FLAGS_gc_disk_migration_origin_replica_interval_seconds,
                     expiration_seconds)) {
                 continue;
@@ -186,22 +206,22 @@ error_s disk_remove_useless_dirs(const std::vector<std::shared_ptr<dir_node>> &d
             continue;
         }
 
-        auto current_time_ms = dsn_now_ms();
+        const auto current_time_ms = dsn_now_ms();
         if (expiration_seconds <= current_time_ms / 1000) {
-            if (dsn::utils::filesystem::remove_path(fpath)) {
+            if (dsn::utils::filesystem::remove_path(path)) {
                 LOG_WARNING("gc_disk: replica_dir_op succeed to delete directory '{}'"
                             ", time_used_ms = {}",
-                            fpath,
+                            path,
                             dsn_now_ms() - current_time_ms);
                 report.remove_dir_count++;
             } else {
                 LOG_WARNING("gc_disk: failed to delete directory '{}', time_used_ms = {}",
-                            fpath,
+                            path,
                             dsn_now_ms() - current_time_ms);
             }
         } else {
             LOG_INFO("gc_disk: reserve directory '{}', wait_seconds = {}",
-                     fpath,
+                     path,
                      expiration_seconds - current_time_ms / 1000);
         }
     }

src/replica/replica_stub.cpp

@@ -1754,7 +1754,7 @@ void replica_stub::on_gc_replica(replica_stub_ptr this_, gpid id)
     CHECK(
         dsn::utils::filesystem::directory_exists(replica_path), "dir({}) not exist", replica_path);
     LOG_INFO("start to move replica({}) as garbage, path: {}", id, replica_path);
-    const auto rename_path = fmt::format("{}.{}.gar", replica_path, dsn_now_us());
+    const auto rename_path = fmt::format("{}.{}{}", replica_path, dsn_now_us(), kFolderSuffixGar);
     if (!dsn::utils::filesystem::rename_path(replica_path, rename_path)) {
         LOG_WARNING("gc_replica: failed to move directory '{}' to '{}'", replica_path, rename_path);
 

src/utils/macros.h

@@ -0,0 +1,21 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#define STRINGIFY_HELPER(x) #x
+#define STRINGIFY(x) STRINGIFY_HELPER(x)

src/utils/metrics.h

@@ -46,6 +46,7 @@
 #include "utils/enum_helper.h"
 #include "utils/fmt_logging.h"
 #include "utils/long_adder.h"
+#include "utils/macros.h"
 #include "utils/nth_element.h"
 #include "utils/ports.h"
 #include "utils/singleton.h"
@@ -973,8 +974,6 @@ struct kth_percentile_property
     double decimal;
 };
 
-#define STRINGIFY_HELPER(x) #x
-#define STRINGIFY(x) STRINGIFY_HELPER(x)
 #define STRINGIFY_KTH_PERCENTILE_NAME(kth) STRINGIFY(KTH_PERCENTILE_NAME(kth))
 #define KTH_TO_DECIMAL(kth) 0.##kth
 #define KTH_PERCENTILE_PROPERTY_LIST(kth)                                                          \