Merge pull request ceph#61623 from ronen-fr/wip-rf-m2ore-keys

ronen-fr · web-flow · commit 0b18c7ce5859 · 2025-02-06T09:44:10.000+02:00
common,osd: replace obsolete get_tracked_conf_keys()

Reviewed-by: Radoslaw Zarzynski &lt;rzarzyns@redhat.com&gt;
diff --git a/src/common/TracepointProvider.cc b/src/common/TracepointProvider.cc
@@ -6,7 +6,7 @@
 
 TracepointProvider::TracepointProvider(CephContext *cct, const char *library,
                                        const char *config_key)
-  : m_cct(cct), m_library(library), m_config_keys{config_key, NULL}
+  : m_cct(cct), m_library(library), m_config_key{config_key}
 {
   m_cct->_conf.add_observer(this);
   verify_config(m_cct->_conf);
@@ -21,7 +21,7 @@ TracepointProvider::~TracepointProvider() {
 
 void TracepointProvider::handle_conf_change(
     const ConfigProxy& conf, const std::set<std::string> &changed) {
-  if (changed.count(m_config_keys[0])) {
+  if (changed.count(m_config_key)) {
     verify_config(conf);
   }
 }
@@ -34,7 +34,7 @@ void TracepointProvider::verify_config(const ConfigProxy& conf) {
 
   char buf[10];
   char *pbuf = buf;
-  if (conf.get_val(m_config_keys[0], &pbuf, sizeof(buf)) != 0 ||
+  if (conf.get_val(m_config_key, &pbuf, sizeof(buf)) != 0 ||
       strncmp(buf, "true", 5) != 0) {
     return;
   }
diff --git a/src/common/TracepointProvider.h b/src/common/TracepointProvider.h
@@ -62,16 +62,16 @@ class TracepointProvider : public md_config_obs_t {
   }
 
 protected:
-  const char** get_tracked_conf_keys() const override {
-    return m_config_keys;
+  std::vector<std::string> get_tracked_keys() const noexcept override {
+    return std::vector<std::string>{m_config_key};
   }
   void handle_conf_change(const ConfigProxy& conf,
 			  const std::set <std::string> &changed) override;
 
 private:
   CephContext *m_cct;
   std::string m_library;
-  mutable const char* m_config_keys[2];
+  std::string m_config_key;
 
   ceph::mutex m_lock = ceph::make_mutex("TracepointProvider::m_lock");
   void* m_handle = nullptr;
diff --git a/src/common/WorkQueue.cc b/src/common/WorkQueue.cc
@@ -34,13 +34,6 @@ ThreadPool::ThreadPool(CephContext *cct_, std::string nm, std::string tn, int n,
 {
   if (option) {
     _thread_num_option = option;
-    // set up conf_keys
-    _conf_keys = new const char*[2];
-    _conf_keys[0] = _thread_num_option.c_str();
-    _conf_keys[1] = NULL;
-  } else {
-    _conf_keys = new const char*[1];
-    _conf_keys[0] = NULL;
   }
 }
 
@@ -58,7 +51,6 @@ void ThreadPool::TPHandle::reset_tp_timeout()
 ThreadPool::~ThreadPool()
 {
   ceph_assert(_threads.empty());
-  delete[] _conf_keys;
 }
 
 void ThreadPool::handle_conf_change(const ConfigProxy& conf,
diff --git a/src/common/WorkQueue.h b/src/common/WorkQueue.h
@@ -108,11 +108,15 @@ class ThreadPool : public md_config_obs_t {
   // track thread pool size changes
   unsigned _num_threads;
   std::string _thread_num_option;
-  const char **_conf_keys;
 
-  const char **get_tracked_conf_keys() const override {
-    return _conf_keys;
+  std::vector<std::string> get_tracked_keys() const noexcept override {
+    if (_thread_num_option.empty()) {
+      return {};
+    } else {
+      return {_thread_num_option};
+    }
   }
+
   void handle_conf_change(const ConfigProxy& conf,
 			  const std::set <std::string> &changed) override;
 
diff --git a/src/common/ceph_context.cc b/src/common/ceph_context.cc
@@ -387,14 +387,12 @@ class CephContextObs : public md_config_obs_t {
 public:
   explicit CephContextObs(CephContext *cct) : cct(cct) {}
 
-  const char** get_tracked_conf_keys() const override {
-    static const char *KEYS[] = {
-      "enable_experimental_unrecoverable_data_corrupting_features",
-      "crush_location",
-      "container_image",  // just so we don't hear complaints about it!
-      NULL
+  std::vector<std::string> get_tracked_keys() const noexcept override {
+    return {
+      "enable_experimental_unrecoverable_data_corrupting_features"s,
+      "crush_location"s,
+      "container_image"s  // just so we don't hear complaints about it!
     };
-    return KEYS;
   }
 
   void handle_conf_change(const ConfigProxy& conf,
diff --git a/src/common/pick_address.cc b/src/common/pick_address.cc
@@ -201,14 +201,11 @@ const struct sockaddr *find_ip_in_subnet_list(
 #ifndef WITH_SEASTAR
 // observe this change
 struct Observer : public md_config_obs_t {
-  const char *keys[2];
-  explicit Observer(const char *c) {
-    keys[0] = c;
-    keys[1] = NULL;
-  }
+  const std::string key;
+  explicit Observer(const char *c) : key(c) {}
 
-  const char** get_tracked_conf_keys() const override {
-    return (const char **)keys;
+  std::vector<std::string> get_tracked_keys() const noexcept override {
+    return std::vector<std::string>{key};
   }
   void handle_conf_change(const ConfigProxy& conf,
 			  const std::set <std::string> &changed) override {
diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
@@ -192,6 +192,7 @@ using ceph::make_mutex;
 using namespace ceph::osd::scheduler;
 using TOPNSPC::common::cmd_getval;
 using TOPNSPC::common::cmd_getval_or;
+using namespace std::literals;
 
 static ostream& _prefix(std::ostream* _dout, int whoami, epoch_t epoch) {
   return *_dout << "osd." << whoami << " " << epoch << " ";
@@ -9993,63 +9994,61 @@ void OSD::dequeue_delete(
 
 // --------------------------------
 
-const char** OSD::get_tracked_conf_keys() const
-{
-  static const char* KEYS[] = {
-    "osd_max_backfills",
-    "osd_min_recovery_priority",
-    "osd_max_trimming_pgs",
-    "osd_op_complaint_time",
-    "osd_op_log_threshold",
-    "osd_op_history_size",
-    "osd_op_history_duration",
-    "osd_op_history_slow_op_size",
-    "osd_op_history_slow_op_threshold",
-    "osd_enable_op_tracker",
-    "osd_map_cache_size",
-    "osd_pg_epoch_max_lag_factor",
-    "osd_pg_epoch_persisted_max_stale",
-    "osd_recovery_sleep",
-    "osd_recovery_sleep_hdd",
-    "osd_recovery_sleep_ssd",
-    "osd_recovery_sleep_hybrid",
-    "osd_delete_sleep",
-    "osd_delete_sleep_hdd",
-    "osd_delete_sleep_ssd",
-    "osd_delete_sleep_hybrid",
-    "osd_snap_trim_sleep",
-    "osd_snap_trim_sleep_hdd",
-    "osd_snap_trim_sleep_ssd",
-    "osd_snap_trim_sleep_hybrid",
-    "osd_scrub_sleep",
-    "osd_recovery_max_active",
-    "osd_recovery_max_active_hdd",
-    "osd_recovery_max_active_ssd",
+std::vector<std::string> OSD::get_tracked_keys() const noexcept
+{
+  return {
+    "osd_max_backfills"s,
+    "osd_min_recovery_priority"s,
+    "osd_max_trimming_pgs"s,
+    "osd_op_complaint_time"s,
+    "osd_op_log_threshold"s,
+    "osd_op_history_size"s,
+    "osd_op_history_duration"s,
+    "osd_op_history_slow_op_size"s,
+    "osd_op_history_slow_op_threshold"s,
+    "osd_enable_op_tracker"s,
+    "osd_map_cache_size"s,
+    "osd_pg_epoch_max_lag_factor"s,
+    "osd_pg_epoch_persisted_max_stale"s,
+    "osd_recovery_sleep"s,
+    "osd_recovery_sleep_hdd"s,
+    "osd_recovery_sleep_ssd"s,
+    "osd_recovery_sleep_hybrid"s,
+    "osd_delete_sleep"s,
+    "osd_delete_sleep_hdd"s,
+    "osd_delete_sleep_ssd"s,
+    "osd_delete_sleep_hybrid"s,
+    "osd_snap_trim_sleep"s,
+    "osd_snap_trim_sleep_hdd"s,
+    "osd_snap_trim_sleep_ssd"s,
+    "osd_snap_trim_sleep_hybrid"s,
+    "osd_scrub_sleep"s,
+    "osd_recovery_max_active"s,
+    "osd_recovery_max_active_hdd"s,
+    "osd_recovery_max_active_ssd"s,
     // clog & admin clog
-    "clog_to_monitors",
-    "clog_to_syslog",
-    "clog_to_syslog_facility",
-    "clog_to_syslog_level",
-    "osd_objectstore_fuse",
-    "clog_to_graylog",
-    "clog_to_graylog_host",
-    "clog_to_graylog_port",
-    "host",
-    "fsid",
-    "osd_recovery_delay_start",
-    "osd_client_message_size_cap",
-    "osd_client_message_cap",
-    "osd_heartbeat_min_size",
-    "osd_heartbeat_interval",
-    "osd_object_clean_region_max_num_intervals",
-    "osd_scrub_min_interval",
-    "osd_scrub_max_interval",
-    "osd_op_thread_timeout",
-    "osd_op_thread_suicide_timeout",
-    "osd_max_scrubs",
-    nullptr
+    "clog_to_monitors"s,
+    "clog_to_syslog"s,
+    "clog_to_syslog_facility"s,
+    "clog_to_syslog_level"s,
+    "osd_objectstore_fuse"s,
+    "clog_to_graylog"s,
+    "clog_to_graylog_host"s,
+    "clog_to_graylog_port"s,
+    "host"s,
+    "fsid"s,
+    "osd_recovery_delay_start"s,
+    "osd_client_message_size_cap"s,
+    "osd_client_message_cap"s,
+    "osd_heartbeat_min_size"s,
+    "osd_heartbeat_interval"s,
+    "osd_object_clean_region_max_num_intervals"s,
+    "osd_scrub_min_interval"s,
+    "osd_scrub_max_interval"s,
+    "osd_op_thread_timeout"s,
+    "osd_op_thread_suicide_timeout"s,
+    "osd_max_scrubs"s
   };
-  return KEYS;
 }
 
 void OSD::handle_conf_change(const ConfigProxy& conf,
diff --git a/src/osd/OSD.h b/src/osd/OSD.h
@@ -1077,7 +1077,7 @@ class OSD : public Dispatcher,
 
 public:
   // config observer bits
-  const char** get_tracked_conf_keys() const override;
+  std::vector<std::string> get_tracked_keys() const noexcept override;
   void handle_conf_change(const ConfigProxy& conf,
                           const std::set <std::string> &changed) override;
   void update_log_config();
diff --git a/src/osd/scheduler/mClockScheduler.cc b/src/osd/scheduler/mClockScheduler.cc
@@ -21,6 +21,7 @@
 
 namespace dmc = crimson::dmclock;
 using namespace std::placeholders;
+using namespace std::literals;
 
 #define dout_context cct
 #define dout_subsys ceph_subsys_mclock
@@ -590,26 +591,24 @@ std::string mClockScheduler::display_queues() const
   return out.str();
 }
 
-const char** mClockScheduler::get_tracked_conf_keys() const
+std::vector<std::string> mClockScheduler::get_tracked_keys() const noexcept
 {
-  static const char* KEYS[] = {
-    "osd_mclock_scheduler_client_res",
-    "osd_mclock_scheduler_client_wgt",
-    "osd_mclock_scheduler_client_lim",
-    "osd_mclock_scheduler_background_recovery_res",
-    "osd_mclock_scheduler_background_recovery_wgt",
-    "osd_mclock_scheduler_background_recovery_lim",
-    "osd_mclock_scheduler_background_best_effort_res",
-    "osd_mclock_scheduler_background_best_effort_wgt",
-    "osd_mclock_scheduler_background_best_effort_lim",
-    "osd_mclock_max_capacity_iops_hdd",
-    "osd_mclock_max_capacity_iops_ssd",
-    "osd_mclock_max_sequential_bandwidth_hdd",
-    "osd_mclock_max_sequential_bandwidth_ssd",
-    "osd_mclock_profile",
-    NULL
+  return {
+    "osd_mclock_scheduler_client_res"s,
+    "osd_mclock_scheduler_client_wgt"s,
+    "osd_mclock_scheduler_client_lim"s,
+    "osd_mclock_scheduler_background_recovery_res"s,
+    "osd_mclock_scheduler_background_recovery_wgt"s,
+    "osd_mclock_scheduler_background_recovery_lim"s,
+    "osd_mclock_scheduler_background_best_effort_res"s,
+    "osd_mclock_scheduler_background_best_effort_wgt"s,
+    "osd_mclock_scheduler_background_best_effort_lim"s,
+    "osd_mclock_max_capacity_iops_hdd"s,
+    "osd_mclock_max_capacity_iops_ssd"s,
+    "osd_mclock_max_sequential_bandwidth_hdd"s,
+    "osd_mclock_max_sequential_bandwidth_ssd"s,
+    "osd_mclock_profile"s
   };
-  return KEYS;
 }
 
 void mClockScheduler::handle_conf_change(
diff --git a/src/osd/scheduler/mClockScheduler.h b/src/osd/scheduler/mClockScheduler.h
@@ -269,7 +269,7 @@ class mClockScheduler : public OpScheduler, md_config_obs_t {
     return op_queue_type_t::mClockScheduler;
   }
 
-  const char** get_tracked_conf_keys() const final;
+  std::vector<std::string> get_tracked_keys() const noexcept final;
   void handle_conf_change(const ConfigProxy& conf,
 			  const std::set<std::string> &changed) final;
 
diff --git a/src/osdc/Objecter.cc b/src/osdc/Objecter.cc
@@ -90,6 +90,7 @@ using ceph::timespan;
 using ceph::shunique_lock;
 using ceph::acquire_shared;
 using ceph::acquire_unique;
+using namespace std::literals;
 
 namespace bc = boost::container;
 namespace bs = boost::system;
@@ -218,15 +219,13 @@ std::unique_lock<std::mutex> Objecter::OSDSession::get_lock(object_t& oid)
   return {completion_locks[h % num_locks], std::defer_lock};
 }
 
-const char** Objecter::get_tracked_conf_keys() const
+std::vector<std::string> Objecter::get_tracked_keys() const noexcept
 {
-  static const char *config_keys[] = {
-    "crush_location",
-    "rados_mon_op_timeout",
-    "rados_osd_op_timeout",
-    NULL
+  return {
+    "crush_location"s,
+    "rados_mon_op_timeout"s,
+    "rados_osd_op_timeout"s
   };
-  return config_keys;
 }
 
 
diff --git a/src/osdc/Objecter.h b/src/osdc/Objecter.h
@@ -1692,7 +1692,7 @@ class Objecter : public md_config_obs_t, public Dispatcher {
   using OpCompletion = boost::asio::any_completion_handler<OpSignature>;
 
   // config observer bits
-  const char** get_tracked_conf_keys() const override;
+  std::vector<std::string> get_tracked_keys() const noexcept override;
   void handle_conf_change(const ConfigProxy& conf,
                           const std::set <std::string> &changed) override;
 

Original file line number	Diff line number	Diff line change
`@@ -6,7 +6,7 @@`
`6`	`6`
`7`	`7`	`TracepointProvider::TracepointProvider(CephContext cct, const char library,`
`8`	`8`	`const char *config_key)`
`9`		`- : m_cct(cct), m_library(library), m_config_keys{config_key, NULL}`
	`9`	`+ : m_cct(cct), m_library(library), m_config_key{config_key}`
`10`	`10`	`{`
`11`	`11`	`m_cct->_conf.add_observer(this);`
`12`	`12`	`verify_config(m_cct->_conf);`
`@@ -21,7 +21,7 @@ TracepointProvider::~TracepointProvider() {`
`21`	`21`
`22`	`22`	`void TracepointProvider::handle_conf_change(`
`23`	`23`	`const ConfigProxy& conf, const std::set<std::string> &changed) {`
`24`		`- if (changed.count(m_config_keys[0])) {`
	`24`	`+ if (changed.count(m_config_key)) {`
`25`	`25`	`verify_config(conf);`
`26`	`26`	`}`
`27`	`27`	`}`
`@@ -34,7 +34,7 @@ void TracepointProvider::verify_config(const ConfigProxy& conf) {`
`34`	`34`
`35`	`35`	`char buf[10];`
`36`	`36`	`char *pbuf = buf;`
`37`		`- if (conf.get_val(m_config_keys[0], &pbuf, sizeof(buf)) != 0 \|\|`
	`37`	`+ if (conf.get_val(m_config_key, &pbuf, sizeof(buf)) != 0 \|\|`
`38`	`38`	`strncmp(buf, "true", 5) != 0) {`
`39`	`39`	`return;`
`40`	`40`	`}`
Original file line number	Diff line number	Diff line change
`@@ -34,13 +34,6 @@ ThreadPool::ThreadPool(CephContext *cct_, std::string nm, std::string tn, int n,`
`34`	`34`	`{`
`35`	`35`	`if (option) {`
`36`	`36`	`_thread_num_option = option;`
`37`		`- // set up conf_keys`
`38`		`- _conf_keys = new const char*[2];`
`39`		`- _conf_keys[0] = _thread_num_option.c_str();`
`40`		`- _conf_keys[1] = NULL;`
`41`		`- } else {`
`42`		`- _conf_keys = new const char*[1];`
`43`		`- _conf_keys[0] = NULL;`
`44`	`37`	`}`
`45`	`38`	`}`
`46`	`39`
`@@ -58,7 +51,6 @@ void ThreadPool::TPHandle::reset_tp_timeout()`
`58`	`51`	`ThreadPool::~ThreadPool()`
`59`	`52`	`{`
`60`	`53`	`ceph_assert(_threads.empty());`
`61`		`- delete[] _conf_keys;`
`62`	`54`	`}`
`63`	`55`
`64`	`56`	`void ThreadPool::handle_conf_change(const ConfigProxy& conf,`
Original file line number	Diff line number	Diff line change
`@@ -269,7 +269,7 @@ class mClockScheduler : public OpScheduler, md_config_obs_t {`
`269`	`269`	`return op_queue_type_t::mClockScheduler;`
`270`	`270`	`}`
`271`	`271`
`272`		`- const char** get_tracked_conf_keys() const final;`
	`272`	`+ std::vector<std::string> get_tracked_keys() const noexcept final;`
`273`	`273`	`void handle_conf_change(const ConfigProxy& conf,`
`274`	`274`	`const std::set<std::string> &changed) final;`
`275`	`275`