Skip to content

Commit

Permalink
mgr: only queue notify events that modules ask for
Browse files Browse the repository at this point in the history
Signed-off-by: Sage Weil <sage@newdream.net>
(cherry picked from commit ee4e3ec)
  • Loading branch information
liewegas authored and neha-ojha committed Feb 2, 2022
1 parent e054c3a commit 18a6e44
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 2 deletions.
8 changes: 7 additions & 1 deletion src/mgr/ActivePyModules.cc
Expand Up @@ -557,9 +557,12 @@ void ActivePyModules::notify_all(const std::string &notify_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){
Expand All @@ -574,6 +577,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.
//
Expand Down
35 changes: 35 additions & 0 deletions src/mgr/PyModule.cc
Expand Up @@ -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...
Expand Down Expand Up @@ -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,
Expand Down
7 changes: 7 additions & 0 deletions src/mgr/PyModule.h
Expand Up @@ -85,6 +85,9 @@ class PyModule
int load_options();
std::map<std::string, MgrMap::ModuleOption> options;

int load_notify_types();
std::set<std::string> notify_types;

public:
static std::string mgr_store_prefix;

Expand Down Expand Up @@ -152,6 +155,10 @@ class PyModule
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;
}
Expand Down
5 changes: 5 additions & 0 deletions src/mgr/PyModuleRegistry.h
Expand Up @@ -191,6 +191,11 @@ class PyModuleRegistry
}
}

bool should_notify(const std::string& name,
const std::string& notify_type) {
return modules.at(name)->should_notify(notify_type);
}

std::map<std::string, std::string> get_services() const
{
ceph_assert(active_modules);
Expand Down
4 changes: 3 additions & 1 deletion src/pybind/mgr/mgr_module.py
Expand Up @@ -982,7 +982,9 @@ def get_context(self) -> object:
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,
Expand Down

0 comments on commit 18a6e44

Please sign in to comment.