Merge pull request ceph#61394 from ronen-fr/wip-rf-cacher-v2

common: modify md_config_obs_impl API

Reviewed-by: Radoslaw Zarzynski <[email protected]>
Reviewed-by: Patrick Donnelly <[email protected]>

Author: ronen-fr

src/common/ceph_context.cc

@@ -120,9 +120,8 @@ class LockdepObs : public md_config_obs_t {
     }
   }
 
-  const char** get_tracked_conf_keys() const override {
-    static const char *KEYS[] = {"lockdep", NULL};
-    return KEYS;
+  std::vector<std::string> get_tracked_keys() const noexcept override {
+    return {"lockdep"s};
   }
 
   void handle_conf_change(const ConfigProxy& conf,
@@ -164,12 +163,8 @@ class MempoolObs : public md_config_obs_t,
   }
 
   // md_config_obs_t
-  const char** get_tracked_conf_keys() const override {
-    static const char *KEYS[] = {
-      "mempool_debug",
-      NULL
-    };
-    return KEYS;
+  std::vector<std::string> get_tracked_keys() const noexcept override {
+    return {"mempool_debug"s};
   }
 
   void handle_conf_change(const ConfigProxy& conf,
@@ -278,29 +273,27 @@ class LogObs : public md_config_obs_t {
     : log(l), lock(ceph::make_mutex("log_obs")) {
   }
 
-  const char** get_tracked_conf_keys() const override {
-    static const char *KEYS[] = {
-      "log_file",
-      "log_max_new",
-      "log_max_recent",
-      "log_to_file",
-      "log_to_syslog",
-      "err_to_syslog",
-      "log_stderr_prefix",
-      "log_to_stderr",
-      "err_to_stderr",
-      "log_to_graylog",
-      "err_to_graylog",
-      "log_graylog_host",
-      "log_graylog_port",
-      "log_to_journald",
-      "err_to_journald",
-      "log_coarse_timestamps",
-      "fsid",
-      "host",
-      NULL
+  std::vector<std::string> get_tracked_keys() const noexcept override {
+    return std::vector<std::string>{
+      "log_file"s,
+      "log_max_new"s,
+      "log_max_recent"s,
+      "log_to_file"s,
+      "log_to_syslog"s,
+      "err_to_syslog"s,
+      "log_stderr_prefix"s,
+      "log_to_stderr"s,
+      "err_to_stderr"s,
+      "log_to_graylog"s,
+      "err_to_graylog"s,
+      "log_graylog_host"s,
+      "log_graylog_port"s,
+      "log_to_journald"s,
+      "err_to_journald"s,
+      "log_coarse_timestamps"s,
+      "fsid"s,
+      "host"s
     };
-    return KEYS;
   }
 
   void handle_conf_change(const ConfigProxy& conf,

src/common/config_cacher.h

@@ -31,28 +31,28 @@
 template <typename ValueT>
 class md_config_cacher_t : public md_config_obs_t {
   ConfigProxy& conf;
-  const char* keys[2];
+  const std::string option_name;
   std::atomic<ValueT> value_cache;
 
-  const char** get_tracked_conf_keys() const override {
-    return const_cast<const char**>(keys);
+  std::vector<std::string> get_tracked_keys() const noexcept override {
+    return std::vector<std::string>{option_name};
   }
 
   void handle_conf_change(const ConfigProxy& conf,
                           const std::set<std::string>& changed) override {
-    if (changed.contains(keys[0])) {
-      value_cache.store(conf.get_val<ValueT>(keys[0]));
+    if (changed.contains(option_name)) {
+      value_cache.store(conf.get_val<ValueT>(option_name));
     }
   }
 
 public:
   md_config_cacher_t(ConfigProxy& conf,
                      const char* const option_name)
-    : conf(conf),
-      keys{option_name, nullptr} {
+    : conf(conf)
+    , option_name{option_name} {
     conf.add_observer(this);
     std::atomic_init(&value_cache,
-                     conf.get_val<ValueT>(keys[0]));
+                     conf.get_val<ValueT>(option_name));
   }
 
   ~md_config_cacher_t() {

src/common/config_obs.h

@@ -16,6 +16,7 @@
 #define CEPH_CONFIG_OBS_H
 
 #include <set>
+#include <vector>
 #include <string>
 
 #include "common/config_fwd.h"
@@ -31,17 +32,32 @@ template<class ConfigProxy>
 class md_config_obs_impl {
 public:
   virtual ~md_config_obs_impl() {}
+
   /** @brief Get a table of strings specifying the configuration keys in which the object is interested.
    * This is called when the object is subscribed to configuration changes with add_observer().
    * The returned table should not be freed until the observer is removed with remove_observer().
-   * Note that it is not possible to change the set of tracked keys without re-subscribing. */
-  virtual const char** get_tracked_conf_keys() const = 0;
+   * Note that it is not possible to change the set of tracked keys without re-subscribing.
+   *
+   * DEPRECATED! Use get_tracked_keys() instead.
+   */
+  virtual const char** get_tracked_conf_keys() const {
+    return nullptr;
+  }
+
+  /**
+   * Returns a vector of strings specifying the configuration keys in which
+   * the object is interested.
+   * Note - the strings in the returned vector are 'moveable'. The caller
+   * (ostensibly the observer manager) is expected to move them into its
+   * map.
+   */
+  virtual std::vector<std::string> get_tracked_keys() const noexcept {
+    return {};
+  }
+
   /// React to a configuration change.
   virtual void handle_conf_change(const ConfigProxy& conf,
 				  const std::set <std::string> &changed) = 0;
-  /// Unused for now
-  virtual void handle_subsys_change(const ConfigProxy& conf,
-				    const std::set<int>& changed) { }
 };
 }
 

src/common/config_obs_mgr.h

@@ -60,10 +60,17 @@ class ObserverMgr : public ConfigTracker {
 template<class ConfigObs>
 void ObserverMgr<ConfigObs>::add_observer(ConfigObs* observer)
 {
-  const char **keys = observer->get_tracked_conf_keys();
   auto ptr = std::make_shared<ConfigObs*>(observer);
-  for (const char ** k = keys; *k; ++k) {
-    observers.emplace(*k, ptr);
+
+  for (auto&& k : observer->get_tracked_keys()) {
+    observers.emplace(std::move(k), ptr);
+  }
+
+  // legacy observer interface:
+  if (const char** keys = observer->get_tracked_conf_keys(); keys) {
+    for (const char** k = keys; *k; ++k) {
+      observers.emplace(*k, ptr);
+    }
   }
 }