osd: add hdd and ssd variants for osd_recovery_max_active

Semi-arbitrarily set the SSD max to 10 (instead of 3).  This should be
tuned based on some real data.

Signed-off-by: Sage Weil <sage@redhat.com>

diff --git a/PendingReleaseNotes b/PendingReleaseNotes
index d433e2f4ae38369c49df8fd57b21a9b441268f40..abecf9ace7e5e697f644b0acec93719ea5abcad3 100644 (file)
--- a/PendingReleaseNotes
+++ b/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.
\ No newline at end of file

diff --git a/doc/rados/configuration/osd-config-ref.rst b/doc/rados/configuration/osd-config-ref.rst
index edcd91e5607de3aeaaf1095cbd9ede63cd1b4e1b..6afc1cc4f1d44d9489a0c94c7fc2337d99206143 100644 (file)
--- a/doc/rados/configuration/osd-config-ref.rst
+++ b/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``
 

diff --git a/src/common/legacy_config_opts.h b/src/common/legacy_config_opts.h
index d2fa55af353d0394de64b8f1424178d79c0bbe6e..f6d957041863becc81cdb7d7136b8ef539910147 100644 (file)
--- a/src/common/legacy_config_opts.h
+++ b/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

diff --git a/src/common/options.cc b/src/common/options.cc
index 2b02dbfa907d8be299f602d247368787d655a54b..8f9c899e6ee0e44e4c9cc6612af2673b7467658d 100644 (file)
--- a/src/common/options.cc
+++ b/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)

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index 155ddd3b827ed600e108486f264d5dc97af3773b..67c0834d58758705156eaf591a8e0901b6ebd65c 100644 (file)
--- a/src/osd/OSD.cc
+++ b/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

diff --git a/src/osd/OSD.h b/src/osd/OSD.h
index b2a9df58e8e4653e98d7f0b87356eec74a9786cb..40b6a07eacd8846a369f5500e915020cb27b782f 100644 (file)
--- a/src/osd/OSD.h
+++ b/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: