From 2371e197374e37a7fe91add5bf4ef96e026b6996 Mon Sep 17 00:00:00 2001 From: Kamoltat Date: Thu, 13 Jul 2023 18:23:13 +0000 Subject: [PATCH] pybind/mgr/pg_autoscaler: noautoscale flag retains individual pool configs Problem: The pg_autoscaler `noautoscale flag` doesn't retain individual pool states of `autoscale mode`. For example turn the flag `ON` and then `OFF` again all the pools will have `autoscale mode on` which is inconvenience for the user because sometimes the user just want to temporary disable the autoscaler on all pools and will enable it back after a period of time while retaining individual pool states of `autoscale mode` Solution: We store noautoscale flag in the OSDMAP such that it is persistent. We then get rid of noautoscale MODULE OPTION in the pg_autoscaler module since we do not need it anymore. Everytime we set, unset or get the flag we rely on looking up the OSDMAP, we did this because we want to avoid inconsistancy between the `noautoscale flag`. This is because `noautoscale flag` can easily be set by doing `ceph osd set noautoscale`. Fixes: https://tracker.ceph.com/issues/61922 Signed-off-by: Kamoltat (cherry picked from commit 0bfdf6362a6b9705212705ee96fa3d6933e51437) Conflicts: src/pybind/mgr/pg_autoscaler/module.py - trivial fix --- src/include/rados.h | 1 + src/mon/MonCommands.h | 4 +- src/mon/OSDMonitor.cc | 4 ++ src/osd/OSDMap.cc | 2 + src/pybind/mgr/pg_autoscaler/module.py | 67 ++++++++++++-------------- 5 files changed, 40 insertions(+), 38 deletions(-) diff --git a/src/include/rados.h b/src/include/rados.h index ae4ab59de8f5b..ac12bd8d76df0 100644 --- a/src/include/rados.h +++ b/src/include/rados.h @@ -172,6 +172,7 @@ extern const char *ceph_osd_state_name(int s); #define CEPH_OSDMAP_PURGED_SNAPDIRS (1<<20) /* osds have converted snapsets */ #define CEPH_OSDMAP_NOSNAPTRIM (1<<21) /* disable snap trimming */ #define CEPH_OSDMAP_PGLOG_HARDLIMIT (1<<22) /* put a hard limit on pg log length */ +#define CEPH_OSDMAP_NOAUTOSCALE (1<<23) /* block pg autoscale */ /* these are hidden in 'ceph status' view */ #define CEPH_OSDMAP_SEMIHIDDEN_FLAGS (CEPH_OSDMAP_REQUIRE_JEWEL| \ diff --git a/src/mon/MonCommands.h b/src/mon/MonCommands.h index 4894deec981d9..8fa39ed5df58f 100644 --- a/src/mon/MonCommands.h +++ b/src/mon/MonCommands.h @@ -842,13 +842,13 @@ COMMAND("osd erasure-code-profile ls", COMMAND("osd set " "name=key,type=CephChoices,strings=full|pause|noup|nodown|" "noout|noin|nobackfill|norebalance|norecover|noscrub|nodeep-scrub|" - "notieragent|nosnaptrim|pglog_hardlimit " + "notieragent|nosnaptrim|pglog_hardlimit|noautoscale " "name=yes_i_really_mean_it,type=CephBool,req=false", "set ", "osd", "rw") COMMAND("osd unset " "name=key,type=CephChoices,strings=full|pause|noup|nodown|"\ "noout|noin|nobackfill|norebalance|norecover|noscrub|nodeep-scrub|" - "notieragent|nosnaptrim", + "notieragent|nosnaptrim|noautoscale", "unset ", "osd", "rw") COMMAND("osd require-osd-release "\ "name=release,type=CephChoices,strings=octopus|pacific|quincy " diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc index 5da4874c8526e..297d4b6c43501 100644 --- a/src/mon/OSDMonitor.cc +++ b/src/mon/OSDMonitor.cc @@ -11581,6 +11581,8 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op, err = -EPERM; goto reply; } + } else if (key == "noautoscale") { + return prepare_set_flag(op, CEPH_OSDMAP_NOAUTOSCALE); } else { ss << "unrecognized flag '" << key << "'"; err = -EINVAL; @@ -11613,6 +11615,8 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op, return prepare_unset_flag(op, CEPH_OSDMAP_NOTIERAGENT); else if (key == "nosnaptrim") return prepare_unset_flag(op, CEPH_OSDMAP_NOSNAPTRIM); + else if (key == "noautoscale") + return prepare_unset_flag(op, CEPH_OSDMAP_NOAUTOSCALE); else { ss << "unrecognized flag '" << key << "'"; err = -EINVAL; diff --git a/src/osd/OSDMap.cc b/src/osd/OSDMap.cc index 01608d38e1799..78316779b524f 100644 --- a/src/osd/OSDMap.cc +++ b/src/osd/OSDMap.cc @@ -3935,6 +3935,8 @@ string OSDMap::get_flag_string(unsigned f) s += ",purged_snapdirs"; if (f & CEPH_OSDMAP_PGLOG_HARDLIMIT) s += ",pglog_hardlimit"; + if (f & CEPH_OSDMAP_NOAUTOSCALE) + s += ",noautoscale"; if (s.length()) s.erase(0, 1); return s; diff --git a/src/pybind/mgr/pg_autoscaler/module.py b/src/pybind/mgr/pg_autoscaler/module.py index ae7ebf09441f8..52701776a3b9a 100644 --- a/src/pybind/mgr/pg_autoscaler/module.py +++ b/src/pybind/mgr/pg_autoscaler/module.py @@ -135,12 +135,6 @@ class PgAutoscaler(MgrModule): '`PG_NUM` before being accepted. Cannot be less than 1.0'), default=3.0, min=1.0), - Option( - name='noautoscale', - type='bool', - desc='global autoscale flag', - long_desc=('Option to turn on/off the autoscaler for all pools'), - default=False), ] def __init__(self, *args: Any, **kwargs: Any) -> None: @@ -155,7 +149,6 @@ class PgAutoscaler(MgrModule): self.sleep_interval = 60 self.mon_target_pg_per_osd = 0 self.threshold = 3.0 - self.noautoscale = False def config_notify(self) -> None: for opt in self.NATIVE_OPTIONS: @@ -238,7 +231,7 @@ class PgAutoscaler(MgrModule): p['pg_num_target'], # p['pg_num_ideal'], final, - p['pg_autoscale_mode'], + 'off' if self.has_noautoscale_flag() else p['pg_autoscale_mode'], str(p['bulk']) ]) return 0, table.get_string(), '' @@ -260,16 +253,13 @@ class PgAutoscaler(MgrModule): self.remote('progress', 'complete', ev.ev_id) del self._event[pool_id] - def set_autoscale_mode_all_pools(self, status: str) -> None: - osdmap = self.get_osdmap() - pools = osdmap.get_pools_by_name() - for pool_name, _ in pools.items(): - self.mon_command({ - 'prefix': 'osd pool set', - 'pool': pool_name, - 'var': 'pg_autoscale_mode', - 'val': status - }) + def has_noautoscale_flag(self) -> bool: + flags = self.get_osdmap().dump().get('flags', '') + if 'noautoscale' in flags: + return True + else: + return False + @CLIWriteCommand("osd pool get noautoscale") def get_noautoscale(self) -> Tuple[int, str, str]: """ @@ -277,10 +267,7 @@ class PgAutoscaler(MgrModule): are setting the autoscaler on or off as well as newly created pools in the future. """ - - if self.noautoscale == None: - raise TypeError("noautoscale cannot be None") - elif self.noautoscale: + if self.has_noautoscale_flag(): return 0, "", "noautoscale is on" else: return 0, "", "noautoscale is off" @@ -289,21 +276,23 @@ class PgAutoscaler(MgrModule): def unset_noautoscale(self) -> Tuple[int, str, str]: """ Unset the noautoscale flag so all pools will - have autoscale enabled (including newly created - pools in the future). + go back to its previous mode. Newly created + pools in the future will autoscaler on by default. """ - if not self.noautoscale: + if not self.has_noautoscale_flag(): return 0, "", "noautoscale is already unset!" else: - self.set_module_option("noautoscale", False) self.mon_command({ 'prefix': 'config set', 'who': 'global', 'name': 'osd_pool_default_pg_autoscale_mode', 'value': 'on' }) - self.set_autoscale_mode_all_pools("on") - return 0, "", "noautoscale is unset, all pools now have autoscale on" + self.mon_command({ + 'prefix': 'osd unset', + 'key': 'noautoscale' + }) + return 0, "", "noautoscale is unset, all pools now back to its previous mode" @CLIWriteCommand("osd pool set noautoscale") def set_noautoscale(self) -> Tuple[int, str, str]: @@ -313,25 +302,28 @@ class PgAutoscaler(MgrModule): and complete all on-going progress events regarding PG-autoscaling. """ - if self.noautoscale: + if self.has_noautoscale_flag(): return 0, "", "noautoscale is already set!" else: - self.set_module_option("noautoscale", True) self.mon_command({ 'prefix': 'config set', 'who': 'global', 'name': 'osd_pool_default_pg_autoscale_mode', 'value': 'off' }) - self.set_autoscale_mode_all_pools("off") + self.mon_command({ + 'prefix': 'osd set', + 'key': 'noautoscale' + }) self.complete_all_progress_events() return 0, "", "noautoscale is set, all pools now have autoscale off" def serve(self) -> None: self.config_notify() while not self._shutdown.is_set(): - self._maybe_adjust() - self._update_progress_events() + if not self.has_noautoscale_flag(): + self._maybe_adjust() + self._update_progress_events() self._shutdown.wait(timeout=self.sleep_interval) def shutdown(self) -> None: @@ -670,7 +662,9 @@ class PgAutoscaler(MgrModule): return (ret, root_map) def _update_progress_events(self) -> None: - if self.noautoscale: + # Update progress events if necessary + if self.has_noautoscale_flag(): + self.log.debug("noautoscale_flag is set.") return osdmap = self.get_osdmap() pools = osdmap.get_pools() @@ -685,9 +679,10 @@ class PgAutoscaler(MgrModule): ev.update(self, (ev.pg_num - pool_data['pg_num']) / (ev.pg_num - ev.pg_num_target)) def _maybe_adjust(self) -> None: - if self.noautoscale: - return self.log.info('_maybe_adjust') + if self.has_noautoscale_flag(): + self.log.debug("noautoscale_flag is set.") + return osdmap = self.get_osdmap() if osdmap.get_require_osd_release() < 'nautilus': return -- 2.39.5