1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
|
From 40d06ce623f19172721a2f1d5b4f4f8b642a3077 Mon Sep 17 00:00:00 2001
From: Kefu Chai <kchai@redhat.com>
Date: Thu, 30 Apr 2020 10:43:01 +0800
Subject: [PATCH] mgr/PyModuleRegistry: probe modules using std::filesystem
for better readability
Signed-off-by: Kefu Chai <kchai@redhat.com>
---
src/mgr/PyModuleRegistry.cc | 43 +++++++++++++++++--------------------
1 file changed, 20 insertions(+), 23 deletions(-)
diff --git a/src/mgr/PyModuleRegistry.cc b/src/mgr/PyModuleRegistry.cc
index 440f7c8bafc1e..466da5d404964 100644
--- a/src/mgr/PyModuleRegistry.cc
+++ b/src/mgr/PyModuleRegistry.cc
@@ -11,6 +11,17 @@
* Foundation. See file COPYING.
*/
+#include "PyModuleRegistry.h"
+
+#if __has_include(<filesystem>)
+#include <filesystem>
+namespace fs = std::filesystem;
+#elif __has_include(<experimental/filesystem>)
+#include <experimental/filesystem>
+namespace fs = std::experimental::filesystem;
+#else
+#error std::filesystem not available!
+#endif
#include "include/stringify.h"
#include "common/errno.h"
@@ -24,8 +35,6 @@
#include "ActivePyModules.h"
-#include "PyModuleRegistry.h"
-
#define dout_context g_ceph_context
#define dout_subsys ceph_subsys_mgr
@@ -258,29 +267,17 @@ void PyModuleRegistry::shutdown()
std::set<std::string> PyModuleRegistry::probe_modules(const std::string &path) const
{
- DIR *dir = opendir(path.c_str());
- if (!dir) {
- return {};
- }
-
- std::set<std::string> modules_out;
- struct dirent *entry = NULL;
- while ((entry = readdir(dir)) != NULL) {
- string n(entry->d_name);
- string fn = path + "/" + n;
- struct stat st;
- int r = ::stat(fn.c_str(), &st);
- if (r == 0 && S_ISDIR(st.st_mode)) {
- string initfn = fn + "/module.py";
- r = ::stat(initfn.c_str(), &st);
- if (r == 0) {
- modules_out.insert(n);
- }
+ std::set<std::string> modules;
+ for (const auto& entry: fs::directory_iterator(path)) {
+ if (!fs::is_directory(entry)) {
+ continue;
+ }
+ auto module_path = entry.path() / "module.py";
+ if (fs::exists(module_path)) {
+ modules.emplace(entry.path().filename());
}
}
- closedir(dir);
-
- return modules_out;
+ return modules;
}
int PyModuleRegistry::handle_command(
From 067adbf9a032b5de793fd0b41b071f24f075270a Mon Sep 17 00:00:00 2001
From: Kefu Chai <kchai@redhat.com>
Date: Thu, 30 Apr 2020 11:34:07 +0800
Subject: [PATCH] mgr: do not load disabled modules
an option named "mgr_disabled_modules" is added in this change to
prevent mgr from loading modules listed in this option. because mgr
loads *all* modules found in the configured path, and per
https://tracker.ceph.com/issues/45147, python subinterpreter could hang
when loading numpy, so this behavior practically creates a deadlock
in mgr.
this issue is found when mgr uses python3.8 runtime. in development
environment, it'd be inconvenient to disable the offending mgr module
without changing the source code, even if we can choose to not install
them, for instance, the enduser can workaround this issue by
uninstalling `ceph-mgr-diskprediction-local`.
an option would be useful in this case, so we can add the module to the
list before mgr tries to load it.
as this issue is found with python3.8 + diskprediction_local (numpy), so
this mgr module is disabled by default if mgr is compiled with python3.8
runtime.
Fixes: https://tracker.ceph.com/issues/45147
Signed-off-by: Kefu Chai <kchai@redhat.com>
---
CMakeLists.txt | 5 +++++
src/common/options.cc | 14 ++++++++++++++
src/include/config-h.in.cmake | 3 +++
src/mgr/PyModuleRegistry.cc | 11 ++++++++++-
4 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 0f7e86414c2d2..fa00d1316bcc0 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -442,6 +442,11 @@ if(WITH_MGR)
set(MGR_PYTHON_LIBRARIES ${Python3_LIBRARIES})
set(MGR_PYTHON_VERSION_MAJOR ${Python3_VERSION_MAJOR})
set(MGR_PYTHON_VERSION_MINOR ${Python3_VERSION_MINOR})
+ # https://tracker.ceph.com/issues/45147
+ if(Python3_VERSION VERSION_GREATER_EQUAL 3.8)
+ set(MGR_DISABLED_MODULES "diskprediction_local")
+ message(STATUS "mgr module disabled for ${Python3_VERSION}: ${MGR_DISABLED_MODULES}")
+ endif()
# Boost dependency check deferred to Boost section
endif(WITH_MGR)
diff --git a/src/common/options.cc b/src/common/options.cc
index be1e955ab51ea..c78d9b69d7591 100644
--- a/src/common/options.cc
+++ b/src/common/options.cc
@@ -5169,6 +5169,20 @@ std::vector<Option> get_global_options() {
.add_service("mgr")
.set_description("Filesystem path to manager modules."),
+ Option("mgr_disabled_modules", Option::TYPE_STR, Option::LEVEL_ADVANCED)
+#ifdef MGR_DISABLED_MODULES
+ .set_default(MGR_DISABLED_MODULES)
+#endif
+ .set_flag(Option::FLAG_STARTUP)
+ .add_service("mgr")
+ .set_description("List of manager modules never get loaded")
+ .set_long_description("A comma delimited list of module names. This list "
+ "is read by manager when it starts. By default, manager loads all "
+ "modules found in specified 'mgr_module_path', and it starts the "
+ "enabled ones as instructed. The modules in this list will not be "
+ "loaded at all.")
+ .add_see_also("mgr_module_path"),
+
Option("mgr_initial_modules", Option::TYPE_STR, Option::LEVEL_BASIC)
.set_default("restful iostat")
.set_flag(Option::FLAG_NO_MON_UPDATE)
diff --git a/src/include/config-h.in.cmake b/src/include/config-h.in.cmake
index dc213938f5c9c..ea550c81e3a0e 100644
--- a/src/include/config-h.in.cmake
+++ b/src/include/config-h.in.cmake
@@ -309,6 +309,9 @@
#cmakedefine MGR_PYTHON_EXECUTABLE "@MGR_PYTHON_EXECUTABLE@"
+/* the default value of "mgr_disabled_module" option */
+#cmakedefine MGR_DISABLED_MODULES "@MGR_DISABLED_MODULES@"
+
/* Define to 1 if you have the `getprogname' function. */
#cmakedefine HAVE_GETPROGNAME 1
diff --git a/src/mgr/PyModuleRegistry.cc b/src/mgr/PyModuleRegistry.cc
index 466da5d404964..2e2e080aa76c0 100644
--- a/src/mgr/PyModuleRegistry.cc
+++ b/src/mgr/PyModuleRegistry.cc
@@ -25,6 +25,7 @@ namespace fs = std::experimental::filesystem;
#include "include/stringify.h"
#include "common/errno.h"
+#include "common/split.h"
#include "BaseMgrModule.h"
#include "PyOSDMap.h"
@@ -267,14 +268,22 @@ void PyModuleRegistry::shutdown()
std::set<std::string> PyModuleRegistry::probe_modules(const std::string &path) const
{
+ const auto opt = g_conf().get_val<std::string>("mgr_disabled_modules");
+ const auto disabled_modules = ceph::split(opt);
+
std::set<std::string> modules;
for (const auto& entry: fs::directory_iterator(path)) {
if (!fs::is_directory(entry)) {
continue;
}
+ const std::string name = entry.path().filename();
+ if (std::count(disabled_modules.begin(), disabled_modules.end(), name)) {
+ dout(10) << "ignoring disabled module " << name << dendl;
+ continue;
+ }
auto module_path = entry.path() / "module.py";
if (fs::exists(module_path)) {
- modules.emplace(entry.path().filename());
+ modules.emplace(name);
}
}
return modules;
|