feat: introduce config set command (#1574)

feat: introduce "config set" command

Solves #1551. In detail, we introduce ConfigRegistry that holds
a curated set of flags that could be changed at run-time.

Signed-off-by: Roman Gershman <roman@dragonflydb.io>

src/facade/error.h

@@ -11,6 +11,7 @@
 namespace facade {
 
 std::string WrongNumArgsError(std::string_view cmd);
+std::string ConfigSetFailed(std::string_view config_name);
 std::string InvalidExpireTime(std::string_view cmd);
 std::string UnknownSubCmd(std::string_view subcmd, std::string_view cmd);
 

src/facade/facade.cc

@@ -63,6 +63,10 @@ string UnknownSubCmd(string_view subcmd, string_view cmd) {
                       cmd, " HELP.");
 }
 
+string ConfigSetFailed(string_view config_name) {
+  return absl::StrCat("CONFIG SET failed (possibly related to argument '", config_name, "').");
+}
+
 const char kSyntaxErr[] = "syntax error";
 const char kWrongTypeErr[] = "-WRONGTYPE Operation against a key holding the wrong kind of value";
 const char kKeyNotFoundErr[] = "no such key";

src/server/CMakeLists.txt

@@ -15,7 +15,7 @@ add_library(dfly_transaction db_slice.cc malloc_stats.cc engine_shard_set.cc blo
 cxx_link(dfly_transaction dfly_core strings_lib)
 
 add_library(dragonfly_lib channel_store.cc command_registry.cc
-            config_flags.cc conn_context.cc debugcmd.cc dflycmd.cc
+            config_registry.cc conn_context.cc debugcmd.cc dflycmd.cc
             generic_family.cc hset_family.cc json_family.cc
             search/search_family.cc search/doc_index.cc search/doc_accessors.cc
             list_family.cc main_service.cc memory_cmd.cc rdb_load.cc rdb_save.cc replica.cc

src/server/common.cc

@@ -102,6 +102,9 @@ bool ParseHumanReadableBytes(std::string_view str, int64_t* num_bytes) {
   char* end;
   double d = strtod(cstr, &end);
 
+  if (end == cstr)  // did not succeed to advance
+    return false;
+
   int64 scale = 1;
   switch (*end) {
     // Considers just the first character after the number

src/server/config_flags.cc

@@ -1,19 +0,0 @@
-// Copyright 2022, DragonflyDB authors.  All rights reserved.
-// See LICENSE for licensing terms.
-//
-#include "server/config_flags.h"
-
-namespace dfly {
-
-bool ValidateConfigEnum(const char* nm, const std::string& val, const ConfigEnum* ptr, unsigned len,
-                     int* dest) {
-  for (unsigned i = 0; i < len; ++i) {
-    if (val == ptr[i].first) {
-      *dest = ptr[i].second;
-      return true;
-    }
-  }
-  return false;
-}
-
-}  // namespace dfly

src/server/config_flags.h

@@ -1,91 +0,0 @@
-// Copyright 2022, DragonflyDB authors.  All rights reserved.
-// See LICENSE for licensing terms.
-//
-
-#pragma once
-
-#include <string.h>
-#include <absl/base/macros.h>
-
-#include "base/flags.h"
-
-namespace dfly {
-
-// DashStr - replaces all underscores to dash characters and keeps the rest as is.
-template <unsigned N> class DashStr {
- public:
-  DashStr(const char* s) {
-    memcpy(str_, s, N);
-    for (unsigned i = 0; i < N; ++i) {
-      if (str_[i] == '_')
-        str_[i] = '-';
-    }
-  }
-
-  const char* str() const {
-    return str_;
-  }
-
- private:
-  char str_[N];
-};
-
-using ConfigEnum = std::pair<const char*, int>;
-
-bool ValidateConfigEnum(const char* nm, const std::string& val, const ConfigEnum* ptr, unsigned len,
-                        int* dest);
-
-}  // namespace dfly
-
-inline bool TrueValidator(const char* nm, const std::string& val) {
-  return true;
-}
-
-#define DEFINE_CONFIG_VAR(type, shorttype, name, value, help, validator)                          \
-  namespace fL##shorttype {                                                                       \
-    type FLAGS_##name = value;                                                                    \
-    static type FLAGS_no##name = value;                                                           \
-    static ::dfly::DashStr<sizeof(#name)> _dash_##name(#name);                                   \
-    static GFLAGS_NAMESPACE::FlagRegisterer o_##name(                                             \
-        _dash_##name.str(), MAYBE_STRIPPED_HELP(help), __FILE__, &FLAGS_##name, &FLAGS_no##name); \
-    static const bool name##_val_reg =                                                            \
-                         GFLAGS_NAMESPACE::RegisterFlagValidator(&FLAGS_##name, validator);       \
-  }                                                                                               \
-  using fL##shorttype::FLAGS_##name
-
-#define BIND_CONFIG(var) [](const char* nm, auto val) { \
-          var = val;                                    \
-          return true;}
-
-
-#define BIND_ENUM_CONFIG(enum_arr, dest_var) [](const char* nm, const std::string& val) { \
-         return ::dfly::ValidateConfigEnum(nm, val, enum_arr, ABSL_ARRAYSIZE(enum_arr),  \
-                         &(dest_var));}
-
-#define CONFIG_uint64(name,val, txt, validator) \
-   DEFINE_CONFIG_VAR(GFLAGS_NAMESPACE::uint64, U64, name, val, txt, validator)
-
-
-#define CONFIG_string(name, val, txt, validator)                                       \
-  namespace fLS {                                                           \
-    using ::fLS::clstring;                                                  \
-    using ::fLS::StringFlagDestructor;                                      \
-    static union { void* align; char s[sizeof(clstring)]; } s_##name[2];    \
-    clstring* const FLAGS_no##name = ::fLS::                                \
-                                   dont_pass0toDEFINE_string(s_##name[0].s, \
-                                                             val);          \
-    static ::dfly::DashStr<sizeof(#name)> _dash_##name(#name);             \
-    static GFLAGS_NAMESPACE::FlagRegisterer o_##name(                       \
-        _dash_##name.str(), MAYBE_STRIPPED_HELP(txt), __FILE__,             \
-        FLAGS_no##name, new (s_##name[1].s) clstring(*FLAGS_no##name));     \
-    static StringFlagDestructor d_##name(s_##name[0].s, s_##name[1].s);     \
-    extern GFLAGS_DLL_DEFINE_FLAG clstring& FLAGS_##name;                   \
-    using fLS::FLAGS_##name;                                                \
-    clstring& FLAGS_##name = *FLAGS_no##name;                               \
-    static const bool name##_val_reg =                                      \
-      GFLAGS_NAMESPACE::RegisterFlagValidator(&FLAGS_##name, validator);    \
-  }                                                                         \
-  using fLS::FLAGS_##name
-
-#define CONFIG_enum(name, val, txt, enum_arr, dest_var)    \
-    CONFIG_string(name, val, txt, BIND_ENUM_CONFIG(enum_arr, dest_var))

src/server/config_registry.cc

@@ -0,0 +1,49 @@
+// Copyright 2023, Roman Gershman.  All rights reserved.
+// See LICENSE for licensing terms.
+//
+#include "src/server/config_registry.h"
+
+#include <absl/flags/reflection.h>
+
+#include "base/logging.h"
+
+namespace dfly {
+
+using namespace std;
+
+ConfigRegistry& ConfigRegistry::Register(std::string_view name, WriteCb cb) {
+  absl::CommandLineFlag* flag = absl::FindCommandLineFlag(name);
+  CHECK(flag) << "Unknown config name: " << name;
+
+  unique_lock lk(mu_);
+  auto [it, inserted] = registry_.emplace(name, std::move(cb));
+  CHECK(inserted) << "Duplicate config name: " << name;
+  return *this;
+}
+
+// Returns true if the value was updated.
+bool ConfigRegistry::Set(std::string_view config_name, std::string_view value) {
+  unique_lock lk(mu_);
+  auto it = registry_.find(config_name);
+  if (it == registry_.end())
+    return false;
+  auto cb = it->second;
+  lk.unlock();
+
+  absl::CommandLineFlag* flag = absl::FindCommandLineFlag(config_name);
+  CHECK(flag);
+  string error;
+  if (!flag->ParseFrom(value, &error))
+    return false;
+
+  return cb(*flag);
+}
+
+void ConfigRegistry::Reset() {
+  unique_lock lk(mu_);
+  registry_.clear();
+}
+
+ConfigRegistry config_registry;
+
+}  // namespace dfly

src/server/config_registry.h

@@ -0,0 +1,31 @@
+// Copyright 2023, Roman Gershman.  All rights reserved.
+// See LICENSE for licensing terms.
+//
+
+#include <absl/container/flat_hash_map.h>
+#include <absl/flags/reflection.h>
+
+#include "util/fibers/synchronization.h"
+
+namespace dfly {
+
+class ConfigRegistry {
+ public:
+  // Accepts the new value as argument. Return true if config was successfully updated.
+  using WriteCb = std::function<bool(const absl::CommandLineFlag&)>;
+
+  ConfigRegistry& Register(std::string_view name, WriteCb cb);
+
+  // Returns true if the value was updated.
+  bool Set(std::string_view config_name, std::string_view value);
+
+  void Reset();
+
+ private:
+  util::fb2::Mutex mu_;
+  absl::flat_hash_map<std::string, WriteCb> registry_ ABSL_GUARDED_BY(mu_);
+};
+
+extern ConfigRegistry config_registry;
+
+}  // namespace dfly

src/server/dfly_main.cc

@@ -46,31 +46,6 @@
 
 using namespace std;
 
-struct MaxMemoryFlag {
-  MaxMemoryFlag() = default;
-  MaxMemoryFlag(const MaxMemoryFlag&) = default;
-  MaxMemoryFlag& operator=(const MaxMemoryFlag&) = default;
-  MaxMemoryFlag(uint64_t v) : value(v) {
-  }  // NOLINT
-
-  uint64_t value;
-};
-
-bool AbslParseFlag(absl::string_view in, MaxMemoryFlag* flag, std::string* err) {
-  int64_t val;
-  if (dfly::ParseHumanReadableBytes(in, &val) && val >= 0) {
-    flag->value = val;
-    return true;
-  }
-
-  *err = "Use human-readable format, eg.: 1G, 1GB, 10GB";
-  return false;
-}
-
-std::string AbslUnparseFlag(const MaxMemoryFlag& flag) {
-  return strings::HumanReadableNumBytes(flag.value);
-}
-
 ABSL_DECLARE_FLAG(uint32_t, port);
 ABSL_DECLARE_FLAG(uint32_t, memcached_port);
 ABSL_DECLARE_FLAG(uint16_t, admin_port);
@@ -87,10 +62,6 @@ ABSL_FLAG(string, unixsocketperm, "", "Set permissions for unixsocket, in octal
 ABSL_FLAG(bool, force_epoll, false,
           "If true - uses linux epoll engine underneath."
           "Can fit for kernels older than 5.10.");
-ABSL_FLAG(MaxMemoryFlag, maxmemory, MaxMemoryFlag(0),
-          "Limit on maximum-memory that is used by the database. "
-          "0 - means the program will automatically determine its maximum memory usage. "
-          "default: 0");
 
 ABSL_FLAG(bool, version_check, true,
           "If true, Will monitor for new releases on Dragonfly servers once a day.");
@@ -302,7 +273,7 @@ string NormalizePaths(std::string_view path) {
 }
 
 bool RunEngine(ProactorPool* pool, AcceptServer* acceptor) {
-  auto maxmemory = GetFlag(FLAGS_maxmemory).value;
+  uint64_t maxmemory = GetMaxMemoryFlag();
   if (maxmemory > 0 && maxmemory < pool->size() * 256_MB) {
     LOG(ERROR) << "There are " << pool->size() << " threads, so "
                << HumanReadableNumBytes(pool->size() * 256_MB) << " are required. Exiting...";
@@ -691,25 +662,26 @@ Usage: dragonfly [FLAGS]
   if (memory.swap_total != 0)
     LOG(WARNING) << "SWAP is enabled. Consider disabling it when running Dragonfly.";
 
-  if (GetFlag(FLAGS_maxmemory).value == 0) {
+  dfly::max_memory_limit = dfly::GetMaxMemoryFlag();
+
+  if (dfly::max_memory_limit == 0) {
     LOG(INFO) << "maxmemory has not been specified. Deciding myself....";
 
     size_t available = memory.mem_avail;
     size_t maxmemory = size_t(0.8 * available);
     LOG(INFO) << "Found " << HumanReadableNumBytes(available)
               << " available memory. Setting maxmemory to " << HumanReadableNumBytes(maxmemory);
-    absl::SetFlag(&FLAGS_maxmemory, MaxMemoryFlag(maxmemory));
+
+    SetMaxMemoryFlag(maxmemory);
+    dfly::max_memory_limit = maxmemory;
   } else {
-    size_t limit = GetFlag(FLAGS_maxmemory).value;
-    string hr_limit = HumanReadableNumBytes(limit);
-    if (limit > memory.mem_avail)
+    string hr_limit = HumanReadableNumBytes(dfly::max_memory_limit);
+    if (dfly::max_memory_limit > memory.mem_avail)
       LOG(WARNING) << "Got memory limit " << hr_limit << ", however only "
                    << HumanReadableNumBytes(memory.mem_avail) << " was found.";
     LOG(INFO) << "Max memory limit is: " << hr_limit;
   }
 
-  dfly::max_memory_limit = GetFlag(FLAGS_maxmemory).value;
-
   mi_option_enable(mi_option_show_errors);
   mi_option_set(mi_option_max_warnings, 0);
   mi_option_set(mi_option_decommit_delay, 0);

src/server/main_service.cc

@@ -41,12 +41,20 @@ extern "C" {
 #include "server/transaction.h"
 #include "server/version.h"
 #include "server/zset_family.h"
+#include "strings/human_readable.h"
 #include "util/html/sorted_table.h"
 #include "util/varz.h"
 
 using namespace std;
 using dfly::operator""_KB;
 
+struct MaxMemoryFlag {
+  uint64_t value = 0;
+};
+
+static bool AbslParseFlag(std::string_view in, MaxMemoryFlag* flag, std::string* err);
+static std::string AbslUnparseFlag(const MaxMemoryFlag& flag);
+
 ABSL_FLAG(uint32_t, port, 6379, "Redis port");
 ABSL_FLAG(uint32_t, memcached_port, 0, "Memcached port");
 
@@ -64,6 +72,26 @@ ABSL_FLAG(bool, admin_nopass, false,
           "If set, would enable open admin access to console on the assigned port, without auth "
           "token needed.");
 
+ABSL_FLAG(MaxMemoryFlag, maxmemory, MaxMemoryFlag{},
+          "Limit on maximum-memory that is used by the database. "
+          "0 - means the program will automatically determine its maximum memory usage. "
+          "default: 0");
+
+bool AbslParseFlag(std::string_view in, MaxMemoryFlag* flag, std::string* err) {
+  int64_t val;
+  if (dfly::ParseHumanReadableBytes(in, &val) && val >= 0) {
+    flag->value = val;
+    return true;
+  }
+
+  *err = "Use human-readable format, eg.: 500MB, 1G, 1TB";
+  return false;
+}
+
+std::string AbslUnparseFlag(const MaxMemoryFlag& flag) {
+  return strings::HumanReadableNumBytes(flag.value);
+}
+
 namespace dfly {
 
 #if __GLIBC__ == 2 && __GLIBC_MINOR__ < 30
@@ -537,6 +565,15 @@ void Service::Init(util::AcceptServer* acceptor, std::vector<facade::Listener*>
                    const InitOpts& opts) {
   InitRedisTables();
 
+  config_registry.Register("maxmemory", [](const absl::CommandLineFlag& flag) {
+    auto res = flag.TryGet<MaxMemoryFlag>();
+    if (!res)
+      return false;
+
+    max_memory_limit = res->value;
+    return true;
+  });
+
   pp_.Await([](uint32_t index, ProactorBase* pb) { ServerState::Init(index); });
 
   uint32_t shard_num = GetFlag(FLAGS_num_shards);
@@ -568,6 +605,8 @@ void Service::Shutdown() {
     facade::Connection::ShutdownThreadLocal();
   });
 
+  config_registry.Reset();
+
   // to shutdown all the runtime components that depend on EngineShard.
   server_family_.Shutdown();
   StringFamily::Shutdown();
@@ -1955,4 +1994,12 @@ void Service::RegisterCommands() {
   }
 }
 
+void SetMaxMemoryFlag(uint64_t value) {
+  absl::SetFlag(&FLAGS_maxmemory, {value});
+}
+
+uint64_t GetMaxMemoryFlag() {
+  return absl::GetFlag(FLAGS_maxmemory).value;
+}
+
 }  // namespace dfly

src/server/main_service.h

@@ -9,6 +9,7 @@
 #include "facade/service_interface.h"
 #include "server/cluster/cluster_family.h"
 #include "server/command_registry.h"
+#include "server/config_registry.h"
 #include "server/engine_shard_set.h"
 #include "server/server_family.h"
 
@@ -19,8 +20,8 @@ class AcceptServer;
 namespace dfly {
 
 class Interpreter;
-class ObjectExplorer;  // for Interpreter
 using facade::MemcacheParser;
+
 class Service : public facade::ServiceInterface {
  public:
   using error_code = std::error_code;
@@ -151,4 +152,7 @@ class Service : public facade::ServiceInterface {
   GlobalState global_state_ = GlobalState::ACTIVE;  // protected by mu_;
 };
 
+uint64_t GetMaxMemoryFlag();
+void SetMaxMemoryFlag(uint64_t value);
+
 }  // namespace dfly

src/server/server_family.cc

@@ -1393,7 +1393,18 @@ void ServerFamily::Config(CmdArgList args, ConnectionContext* cntx) {
   string_view sub_cmd = ArgS(args, 0);
 
   if (sub_cmd == "SET") {
-    return (*cntx)->SendOk();
+    if (args.size() < 3) {
+      return (*cntx)->SendError(WrongNumArgsError("config|set"));
+    }
+
+    ToLower(&args[1]);
+    string_view config_name = ArgS(args, 1);
+    bool success = config_registry.Set(config_name, ArgS(args, 2));
+    if (success) {
+      return (*cntx)->SendOk();
+    } else {
+      return (*cntx)->SendError(ConfigSetFailed(config_name), kSyntaxErrType);
+    }
   } else if (sub_cmd == "GET" && args.size() == 2) {
     // Send empty response, like Redis does, unless the param is supported
     std::vector<std::string> res;