From 7a8d07117e146d5123fb47ebb20a23c4a34bae89 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Thu, 2 Dec 2021 10:23:45 -0500 Subject: [PATCH] mgr: only queue notify events that modules ask for Signed-off-by: Sage Weil (cherry picked from commit ee4e3ecd6e0754854c00f3dfa0eaaa17bbf3603d) --- src/mgr/ActivePyModules.cc | 8 +++++++- src/mgr/PyModule.cc | 35 +++++++++++++++++++++++++++++++++++ src/mgr/PyModule.h | 7 +++++++ src/mgr/PyModuleRegistry.h | 5 +++++ src/pybind/mgr/mgr_module.py | 4 +++- 5 files changed, 57 insertions(+), 2 deletions(-) diff --git a/src/mgr/ActivePyModules.cc b/src/mgr/ActivePyModules.cc index 5e545d02d811a..c6a08964c1c40 100644 --- a/src/mgr/ActivePyModules.cc +++ b/src/mgr/ActivePyModules.cc @@ -580,9 +580,12 @@ void ActivePyModules::notify_all(const std::string ¬ify_type, dout(10) << __func__ << ": notify_all " << notify_type << dendl; for (auto& [name, module] : modules) { + if (!py_module_registry.should_notify(name, notify_type)) { + continue; + } // Send all python calls down a Finisher to avoid blocking // C++ code, and avoid any potential lock cycles. - dout(15) << "queuing notify to " << name << dendl; + dout(15) << "queuing notify (" << notify_type << ") to " << name << dendl; // workaround for https://bugs.llvm.org/show_bug.cgi?id=35984 finisher.queue(new LambdaContext([module=module, notify_type, notify_id] (int r){ @@ -597,6 +600,9 @@ void ActivePyModules::notify_all(const LogEntry &log_entry) dout(10) << __func__ << ": notify_all (clog)" << dendl; for (auto& [name, module] : modules) { + if (!py_module_registry.should_notify(name, "clog")) { + continue; + } // Send all python calls down a Finisher to avoid blocking // C++ code, and avoid any potential lock cycles. // diff --git a/src/mgr/PyModule.cc b/src/mgr/PyModule.cc index afbff01ae6ebc..19d02332d2d51 100644 --- a/src/mgr/PyModule.cc +++ b/src/mgr/PyModule.cc @@ -356,6 +356,8 @@ int PyModule::load(PyThreadState *pMainThreadState) return r; } + load_notify_types(); + // We've imported the module and found a MgrModule subclass, at this // point the module is considered loaded. It might still not be // runnable though, can_run populated later... @@ -467,6 +469,39 @@ int PyModule::register_options(PyObject *cls) return 0; } +int PyModule::load_notify_types() +{ + PyObject *ls = PyObject_GetAttrString(pClass, "NOTIFY_TYPES"); + if (ls == nullptr) { + derr << "Module " << get_name() << " has missing NOTIFY_TYPES member" << dendl; + return -EINVAL; + } + if (!PyObject_TypeCheck(ls, &PyList_Type)) { + // Relatively easy mistake for human to make, e.g. defining COMMANDS + // as a {} instead of a [] + derr << "Module " << get_name() << " has NOTIFY_TYPES that is not a list" << dendl; + return -EINVAL; + } + + const size_t list_size = PyList_Size(ls); + for (size_t i = 0; i < list_size; ++i) { + PyObject *notify_type = PyList_GetItem(ls, i); + ceph_assert(notify_type != nullptr); + + if (!PyObject_TypeCheck(notify_type, &PyUnicode_Type)) { + derr << "Module " << get_name() << " has non-string entry in NOTIFY_TYPES list" + << dendl; + return -EINVAL; + } + + notify_types.insert(PyUnicode_AsUTF8(notify_type)); + } + Py_DECREF(ls); + dout(10) << "Module " << get_name() << " notify_types " << notify_types << dendl; + + return 0; +} + int PyModule::load_commands() { PyObject *pRegCmd = PyObject_CallMethod(pClass, diff --git a/src/mgr/PyModule.h b/src/mgr/PyModule.h index 6df3ee5860f4c..fe2e16238ef1c 100644 --- a/src/mgr/PyModule.h +++ b/src/mgr/PyModule.h @@ -85,6 +85,9 @@ private: int load_options(); std::map options; + int load_notify_types(); + std::set notify_types; + public: static std::string mgr_store_prefix; @@ -152,6 +155,10 @@ public: bool is_loaded() const { std::lock_guard l(lock) ; return loaded; } bool is_always_on() const { std::lock_guard l(lock) ; return always_on; } + bool should_notify(const std::string& notify_type) const { + return notify_types.count(notify_type); + } + const std::string &get_name() const { std::lock_guard l(lock) ; return module_name; } diff --git a/src/mgr/PyModuleRegistry.h b/src/mgr/PyModuleRegistry.h index ad48af9dafea6..6c72af8932a12 100644 --- a/src/mgr/PyModuleRegistry.h +++ b/src/mgr/PyModuleRegistry.h @@ -191,6 +191,11 @@ public: } } + bool should_notify(const std::string& name, + const std::string& notify_type) { + return modules.at(name)->should_notify(notify_type); + } + std::map get_services() const { ceph_assert(active_modules); diff --git a/src/pybind/mgr/mgr_module.py b/src/pybind/mgr/mgr_module.py index 8b12cd29791ee..4d9797d38ccf0 100644 --- a/src/pybind/mgr/mgr_module.py +++ b/src/pybind/mgr/mgr_module.py @@ -1005,7 +1005,9 @@ class MgrModule(ceph_module.BaseMgrModule, MgrModuleLoggingMixin): def notify(self, notify_type: NotifyType, notify_id: str) -> None: """ Called by the ceph-mgr service to notify the Python plugin - that new state is available. + that new state is available. This method is *only* called for + notify_types that are listed in the NOTIFY_TYPES string list + member of the module class. :param notify_type: string indicating what kind of notification, such as osd_map, mon_map, fs_map, mon_status, -- 2.39.5