Merge pull request #28677 from liewegas/wip-recovery-nvme

osd: add hdd and ssd variants for osd_recovery_max_active

Reviewed-by: Neha Ojha <nojha@redhat.com>
Reviewed-by: Josh Durgin <jdurgin@redhat.com>
Reviewed-by: Mark Nelson <mnelson@redhat.com>

PendingReleaseNotes

@@ -86,3 +86,13 @@
   parsing date or time values from the unstructured human-readable
   output should be modified to parse the structured output instead, as
   the human-readable output may change without notice.
+
+* The ``osd_recovery_max_active`` option now has
+  ``osd_recovery_max_active_hdd`` and ``osd_recovery_max_active_ssd``
+  variants, each with different default values for HDD and SSD-backed
+  OSDs, respectively.  By default ``osd_recovery_max_active`` now
+  defaults to zero, which means that the OSD will conditionally use
+  the HDD or SSD option values.  Administrators who have customized
+  this value may want to consider whether they have set this to a
+  value similar to the new defaults (3 for HDDs and 10 for SSDs) and,
+  if so, remove the option from their configuration entirely.

doc/rados/configuration/osd-config-ref.rst

@@ -915,9 +915,30 @@ perform well in a degraded state.
               requests will accelerate recovery, but the requests places an
               increased load on the cluster.
 
+	      This value is only used if it is non-zero. Normally it
+	      is ``0``, which means that the ``hdd`` or ``ssd`` values
+	      (below) are used, depending on the type of the primary
+	      device backing the OSD.
+
+:Type: 32-bit Integer
+:Default: ``0``
+
+``osd recovery max active hdd``
+
+:Description: The number of active recovery requests per OSD at one time, if the
+	      primary device is rotational.
+
 :Type: 32-bit Integer
 :Default: ``3``
 
+``osd recovery max active ssd``
+
+:Description: The number of active recovery requests per OSD at one time, if the
+	      priary device is non-rotational (i.e., an SSD).
+
+:Type: 32-bit Integer
+:Default: ``10``
+
 
 ``osd recovery max chunk``
 

src/common/legacy_config_opts.h

@@ -689,6 +689,8 @@ OPTION(osd_default_data_pool_replay_window, OPT_INT)
 OPTION(osd_auto_mark_unfound_lost, OPT_BOOL)
 OPTION(osd_recovery_delay_start, OPT_FLOAT)
 OPTION(osd_recovery_max_active, OPT_U64)
+OPTION(osd_recovery_max_active_hdd, OPT_U64)
+OPTION(osd_recovery_max_active_ssd, OPT_U64)
 OPTION(osd_recovery_max_single_start, OPT_U64)
 OPTION(osd_recovery_max_chunk, OPT_U64)  // max size of push chunk
 OPTION(osd_recovery_max_omap_entries_per_chunk, OPT_U64) // max number of omap entries per chunk; 0 to disable limit

src/common/options.cc

@@ -3352,8 +3352,22 @@ std::vector<Option> get_global_options() {
     .set_description(""),
 
     Option("osd_recovery_max_active", Option::TYPE_UINT, Option::LEVEL_ADVANCED)
+    .set_default(0)
+    .set_description("Number of simultaneous active recovery operations per OSD (overrides _ssd and _hdd if non-zero)")
+    .add_see_also("osd_recovery_max_active_hdd")
+    .add_see_also("osd_recovery_max_active_ssd"),
+
+    Option("osd_recovery_max_active_hdd", Option::TYPE_UINT, Option::LEVEL_ADVANCED)
     .set_default(3)
-    .set_description(""),
+    .set_description("Number of simultaneous active recovery oeprations per OSD (for rotational devices)")
+    .add_see_also("osd_recovery_max_active")
+    .add_see_also("osd_recovery_max_active_ssd"),
+
+    Option("osd_recovery_max_active_ssd", Option::TYPE_UINT, Option::LEVEL_ADVANCED)
+    .set_default(10)
+    .set_description("Number of simultaneous active recovery oeprations per OSD (for non-rotational solid state devices)")
+    .add_see_also("osd_recovery_max_active")
+    .add_see_also("osd_recovery_max_active_hdd"),
 
     Option("osd_recovery_max_single_start", Option::TYPE_UINT, Option::LEVEL_ADVANCED)
     .set_default(1)

src/osd/OSD.cc

@@ -2689,6 +2689,16 @@ float OSD::get_osd_delete_sleep()
   return cct->_conf.get_val<double>("osd_delete_sleep_hdd");
 }
 
+int OSD::get_recovery_max_active()
+{
+  if (cct->_conf->osd_recovery_max_active)
+    return cct->_conf->osd_recovery_max_active;
+  if (store_is_rotational)
+    return cct->_conf->osd_recovery_max_active_hdd;
+  else
+    return cct->_conf->osd_recovery_max_active_ssd;
+}
+
 int OSD::init()
 {
   CompatSet initial, diff;
@@ -9055,7 +9065,7 @@ bool OSDService::_recover_now(uint64_t *available_pushes)
     return false;
   }
 
-  uint64_t max = cct->_conf->osd_recovery_max_active;
+  uint64_t max = osd->get_recovery_max_active();
   if (max <= recovery_ops_active + recovery_ops_reserved) {
     dout(15) << __func__ << " active " << recovery_ops_active
 	     << " + reserved " << recovery_ops_reserved
@@ -9148,7 +9158,7 @@ void OSDService::start_recovery_op(PG *pg, const hobject_t& soid)
   std::lock_guard l(recovery_lock);
   dout(10) << "start_recovery_op " << *pg << " " << soid
 	   << " (" << recovery_ops_active << "/"
-	   << cct->_conf->osd_recovery_max_active << " rops)"
+	   << osd->get_recovery_max_active() << " rops)"
 	   << dendl;
   recovery_ops_active++;
 
@@ -9164,7 +9174,8 @@ void OSDService::finish_recovery_op(PG *pg, const hobject_t& soid, bool dequeue)
   std::lock_guard l(recovery_lock);
   dout(10) << "finish_recovery_op " << *pg << " " << soid
 	   << " dequeue=" << dequeue
-	   << " (" << recovery_ops_active << "/" << cct->_conf->osd_recovery_max_active << " rops)"
+	   << " (" << recovery_ops_active << "/"
+	   << osd->get_recovery_max_active() << " rops)"
 	   << dendl;
 
   // adjust count

src/osd/OSD.h

@@ -2119,6 +2119,8 @@ private:
   float get_osd_recovery_sleep();
   float get_osd_delete_sleep();
 
+  int get_recovery_max_active();
+
   void probe_smart(const string& devid, ostream& ss);
 
 public: