feat(config): add compaction-checker-cron to extend original compacti…

…on-checker-range (#2383)

kvrocks.conf

@@ -492,15 +492,23 @@ profiling-sample-record-threshold-ms 100
 ################################## CRON ###################################
 
 # Compact Scheduler, auto compact at schedule time
-# Time expression format is the same as crontab (currently only support *, int and */int)
-# e.g. compact-cron 0 3 * * * 0 4 * * *
+# Time expression format is the same as crontab (supported cron syntax: *, n, */n, `1,3-6,9,11`)
+# e.g. compact-cron 0 3,4 * * *
 # would compact the db at 3am and 4am everyday
 # compact-cron 0 3 * * *
 
 # The hour range that compaction checker would be active
 # e.g. compaction-checker-range 0-7 means compaction checker would be worker between
 # 0-7am every day.
-compaction-checker-range 0-7
+# WARNING: this config option is deprecated and will be removed,
+# please use compaction-checker-cron instead
+# compaction-checker-range 0-7
+
+# The time pattern that compaction checker would be active
+# Time expression format is the same as crontab (supported cron syntax: *, n, */n, `1,3-6,9,11`)
+# e.g. compaction-checker-cron * 0-7 * * * means compaction checker would be worker between
+# 0-7am every day.
+compaction-checker-cron * 0-7 * * *
 
 # When the compaction checker is triggered, the db will periodically pick the SST file
 # with the highest "deleted percentage" (i.e. the percentage of deleted keys in the SST 
@@ -515,14 +523,14 @@ compaction-checker-range 0-7
 # force-compact-file-min-deleted-percentage 10
 
 # Bgsave scheduler, auto bgsave at scheduled time
-# Time expression format is the same as crontab (currently only support *, int and */int)
-# e.g. bgsave-cron 0 3 * * * 0 4 * * *
+# Time expression format is the same as crontab (supported cron syntax: *, n, */n, `1,3-6,9,11`)
+# e.g. bgsave-cron 0 3,4 * * *
 # would bgsave the db at 3am and 4am every day
 
 # Kvrocks doesn't store the key number directly. It needs to scan the DB and
 # then retrieve the key number by using the dbsize scan command.
 # The Dbsize scan scheduler auto-recalculates the estimated keys at scheduled time.
-# Time expression format is the same as crontab (currently only support *, int and */int)
+# Time expression format is the same as crontab (supported cron syntax: *, n, */n, `1,3-6,9,11`)
 # e.g. dbsize-scan-cron 0 * * * *
 # would recalculate the keyspace infos of the db every hour.
 

src/common/cron.cc

@@ -55,6 +55,8 @@ StatusOr<CronScheduler> CronScheduler::Parse(std::string_view minute, std::strin
   return st;
 }
 
+void Cron::Clear() { schedulers_.clear(); }
+
 Status Cron::SetScheduleTime(const std::vector<std::string> &args) {
   if (args.empty()) {
     schedulers_.clear();

src/common/cron.h

@@ -40,7 +40,7 @@ struct CronPattern {
   struct Any {};                                             // *
   using Numbers = std::vector<std::variant<Number, Range>>;  // 1,2,3-6,7
 
-  std::variant<Numbers, Interval, Any> val;
+  std::variant<Any, Numbers, Interval> val;
 
   static StatusOr<CronPattern> Parse(std::string_view str, std::tuple<int, int> minmax) {
     if (str == "*") {
@@ -63,10 +63,10 @@ struct CronPattern {
         if (auto pos = num_str.find('-'); pos != num_str.npos) {
           auto l_str = num_str.substr(0, pos);
           auto r_str = num_str.substr(pos + 1);
-          auto l = GET_OR_RET(ParseInt<int>(std::string(num_str.begin(), num_str.end()), minmax)
-                                  .Prefixed("an integer is expected before `-` in a cron expression"));
-          auto r = GET_OR_RET(ParseInt<int>(std::string(num_str.begin(), num_str.end()), minmax)
-                                  .Prefixed("an integer is expected after `-` in a cron expression"));
+          auto l = GET_OR_RET(
+              ParseInt<int>(l_str, minmax).Prefixed("an integer is expected before `-` in a cron expression"));
+          auto r = GET_OR_RET(
+              ParseInt<int>(r_str, minmax).Prefixed("an integer is expected after `-` in a cron expression"));
 
           if (l >= r) {
             return {Status::NotOK, "for pattern `l-r` in cron expression, r should be larger than l"};
@@ -160,6 +160,8 @@ class Cron {
   ~Cron() = default;
 
   Status SetScheduleTime(const std::vector<std::string> &args);
+  void Clear();
+
   bool IsTimeMatch(const tm *tm);
   std::string ToString() const;
   bool IsEnabled() const;

src/config/config.cc

@@ -141,6 +141,7 @@ Config::Config() {
       {"replica-announce-ip", false, new StringField(&replica_announce_ip, "")},
       {"replica-announce-port", false, new UInt32Field(&replica_announce_port, 0, 0, PORT_LIMIT)},
       {"compaction-checker-range", false, new StringField(&compaction_checker_range_str_, "")},
+      {"compaction-checker-cron", false, new StringField(&compaction_checker_cron_str_, "")},
       {"force-compact-file-age", false, new Int64Field(&force_compact_file_age, 2 * 24 * 3600, 60, INT64_MAX)},
       {"force-compact-file-min-deleted-percentage", false,
        new IntField(&force_compact_file_min_deleted_percentage, 10, 1, 100)},
@@ -300,21 +301,19 @@ void Config::initFieldValidator() {
        }},
       {"compaction-checker-range",
        [this](const std::string &k, const std::string &v) -> Status {
+         if (!compaction_checker_cron_str_.empty()) {
+           return {Status::NotOK, "compaction-checker-range cannot be set while compaction-checker-cron is set"};
+         }
          if (v.empty()) {
-           compaction_checker_range.start = -1;
-           compaction_checker_range.stop = -1;
+           compaction_checker_cron.Clear();
            return Status::OK();
          }
-         std::vector<std::string> args = util::Split(v, "-");
-         if (args.size() != 2) {
-           return {Status::NotOK, "invalid range format, the range should be between 0 and 24"};
-         }
-         auto start = GET_OR_RET(ParseInt<int>(args[0], {0, 24}, 10)),
-              stop = GET_OR_RET(ParseInt<int>(args[1], {0, 24}, 10));
-         if (start > stop) return {Status::NotOK, "invalid range format, start should be smaller than stop"};
-         compaction_checker_range.start = start;
-         compaction_checker_range.stop = stop;
-         return Status::OK();
+         return compaction_checker_cron.SetScheduleTime({"*", v, "*", "*", "*"});
+       }},
+      {"compaction-checker-cron",
+       [this](const std::string &k, const std::string &v) -> Status {
+         std::vector<std::string> args = util::Split(v, " \t");
+         return compaction_checker_cron.SetScheduleTime(args);
        }},
       {"rename-command",
        [](const std::string &k, const std::string &v) -> Status {

src/config/config.h

@@ -57,14 +57,6 @@ constexpr const char *kDefaultNamespace = "__namespace";
 
 enum class BlockCacheType { kCacheTypeLRU = 0, kCacheTypeHCC };
 
-struct CompactionCheckerRange {
- public:
-  int start;
-  int stop;
-
-  bool Enabled() const { return start != -1 || stop != -1; }
-};
-
 struct CLIOptions {
   std::string conf_file;
   std::vector<std::pair<std::string, std::string>> cli_options;
@@ -138,7 +130,7 @@ struct Config {
   Cron compact_cron;
   Cron bgsave_cron;
   Cron dbsize_scan_cron;
-  CompactionCheckerRange compaction_checker_range{-1, -1};
+  Cron compaction_checker_cron;
   int64_t force_compact_file_age;
   int force_compact_file_min_deleted_percentage;
   bool repl_namespace_enabled = false;
@@ -249,6 +241,7 @@ struct Config {
   std::string bgsave_cron_str_;
   std::string dbsize_scan_cron_str_;
   std::string compaction_checker_range_str_;
+  std::string compaction_checker_cron_str_;
   std::string profiling_sample_commands_str_;
   std::map<std::string, std::unique_ptr<ConfigField>> fields_;
   std::vector<std::string> rename_command_;

src/server/server.cc

@@ -204,16 +204,18 @@ Status Server::Start() {
       if (storage->IsClosing()) continue;
 
       if (!is_loading_ && ++counter % 600 == 0  // check every minute
-          && config_->compaction_checker_range.Enabled()) {
-        auto now_hours = util::GetTimeStamp<std::chrono::hours>();
-        if (now_hours >= config_->compaction_checker_range.start &&
-            now_hours <= config_->compaction_checker_range.stop) {
+          && config_->compaction_checker_cron.IsEnabled()) {
+        auto t_now = static_cast<time_t>(util::GetTimeStamp());
+        std::tm now{};
+        localtime_r(&t_now, &now);
+        if (config_->compaction_checker_cron.IsTimeMatch(&now)) {
           const auto &column_family_list = engine::ColumnFamilyConfigs::ListAllColumnFamilies();
           for (auto &column_family : column_family_list) {
             compaction_checker.PickCompactionFilesForCf(column_family);
           }
         }
         // compact once per day
+        auto now_hours = t_now / 3600;
         if (now_hours != 0 && last_compact_date != now_hours / 24) {
           last_compact_date = now_hours / 24;
           compaction_checker.CompactPropagateAndPubSubFiles();
@@ -756,7 +758,7 @@ void Server::cron() {
       std::tm now{};
       localtime_r(&t, &now);
       // disable compaction cron when the compaction checker was enabled
-      if (!config_->compaction_checker_range.Enabled() && config_->compact_cron.IsEnabled() &&
+      if (!config_->compaction_checker_cron.IsEnabled() && config_->compact_cron.IsEnabled() &&
           config_->compact_cron.IsTimeMatch(&now)) {
         Status s = AsyncCompactDB();
         LOG(INFO) << "[server] Schedule to compact the db, result: " << s.Msg();

tests/cppunit/cron_test.cc

@@ -372,3 +372,43 @@ TEST_F(CronTestWeekDayInterval, ToString) {
   std::string got = cron_->ToString();
   ASSERT_EQ("0 * * * */4", got);
 }
+
+class CronTestNumberAndRange : public testing::Test {
+ protected:
+  explicit CronTestNumberAndRange() {
+    cron_ = std::make_unique<Cron>();
+    std::vector<std::string> schedule{"*", "1,3,6-10,20", "*", "*", "*"};
+    auto s = cron_->SetScheduleTime(schedule);
+    EXPECT_TRUE(s.IsOK());
+  }
+  ~CronTestNumberAndRange() override = default;
+
+  std::unique_ptr<Cron> cron_;
+};
+
+TEST_F(CronTestNumberAndRange, IsTimeMatch) {
+  std::time_t t = std::time(nullptr);
+  std::tm *now = std::localtime(&t);
+  now->tm_hour = 1;
+  ASSERT_TRUE(cron_->IsTimeMatch(now));
+  now->tm_hour = 3;
+  ASSERT_TRUE(cron_->IsTimeMatch(now));
+  now->tm_hour = 6;
+  ASSERT_TRUE(cron_->IsTimeMatch(now));
+  now->tm_hour = 8;
+  ASSERT_TRUE(cron_->IsTimeMatch(now));
+  now->tm_hour = 10;
+  ASSERT_TRUE(cron_->IsTimeMatch(now));
+  now->tm_hour = 20;
+  ASSERT_TRUE(cron_->IsTimeMatch(now));
+  now->tm_hour = 0;
+  ASSERT_FALSE(cron_->IsTimeMatch(now));
+  now->tm_hour = 2;
+  ASSERT_FALSE(cron_->IsTimeMatch(now));
+  now->tm_hour = 5;
+  ASSERT_FALSE(cron_->IsTimeMatch(now));
+  now->tm_hour = 14;
+  ASSERT_FALSE(cron_->IsTimeMatch(now));
+  now->tm_hour = 22;
+  ASSERT_FALSE(cron_->IsTimeMatch(now));
+}