From dd9f9e268b5562bae8972bce03ec0970f353ae14 Mon Sep 17 00:00:00 2001 From: Ramana Raja Date: Thu, 2 Nov 2023 14:38:02 -0400 Subject: [PATCH] mgr/Mgr: remove shutdown() and handle_signal() Mgr::shutdown() and Mgr::handle_signal() are never called since 3363a10. The mgr just exits on SIGINT or SIGTERM. Also found and removed other unreachable code associated with shutdown in mgr codebase. Fixes: https://tracker.ceph.com/issues/63410 Signed-off-by: Ramana Raja --- src/mgr/ActivePyModules.cc | 36 ---------------------------- src/mgr/ActivePyModules.h | 1 - src/mgr/ClusterState.cc | 8 ------- src/mgr/ClusterState.h | 1 - src/mgr/DaemonServer.cc | 19 --------------- src/mgr/DaemonServer.h | 2 -- src/mgr/Mgr.cc | 27 --------------------- src/mgr/Mgr.h | 3 --- src/mgr/MgrStandby.cc | 35 --------------------------- src/mgr/MgrStandby.h | 1 - src/mgr/PyModuleRegistry.cc | 47 ------------------------------------- src/mgr/PyModuleRegistry.h | 3 --- 12 files changed, 183 deletions(-) diff --git a/src/mgr/ActivePyModules.cc b/src/mgr/ActivePyModules.cc index 45038e734af..2f4f6bd1e80 100644 --- a/src/mgr/ActivePyModules.cc +++ b/src/mgr/ActivePyModules.cc @@ -555,42 +555,6 @@ void ActivePyModules::start_one(PyModuleRef py_module) })); } -void ActivePyModules::shutdown() -{ - std::lock_guard locker(lock); - - // Stop per active module finisher thread - for (auto& [name, module] : modules) { - dout(4) << "Stopping active module " << name << " finisher thread" << dendl; - module->finisher.wait_for_empty(); - module->finisher.stop(); - } - - // Signal modules to drop out of serve() and/or tear down resources - for (auto& [name, module] : modules) { - lock.unlock(); - dout(10) << "calling module " << name << " shutdown()" << dendl; - module->shutdown(); - dout(10) << "module " << name << " shutdown() returned" << dendl; - lock.lock(); - } - - // For modules implementing serve(), finish the threads where we - // were running that. - for (auto& [name, module] : modules) { - lock.unlock(); - dout(10) << "joining module " << name << dendl; - module->thread.join(); - dout(10) << "joined module " << name << dendl; - lock.lock(); - } - - cmd_finisher.wait_for_empty(); - cmd_finisher.stop(); - - modules.clear(); -} - void ActivePyModules::notify_all(const std::string ¬ify_type, const std::string ¬ify_id) { diff --git a/src/mgr/ActivePyModules.h b/src/mgr/ActivePyModules.h index 283f96a6ed9..d6ade4849f7 100644 --- a/src/mgr/ActivePyModules.h +++ b/src/mgr/ActivePyModules.h @@ -216,7 +216,6 @@ public: std::string *err); int init(); - void shutdown(); void start_one(PyModuleRef py_module); diff --git a/src/mgr/ClusterState.cc b/src/mgr/ClusterState.cc index 7f811a5e415..6b106268efc 100644 --- a/src/mgr/ClusterState.cc +++ b/src/mgr/ClusterState.cc @@ -225,14 +225,6 @@ void ClusterState::final_init() ceph_assert(r == 0); } -void ClusterState::shutdown() -{ - // unregister commands - g_ceph_context->get_admin_socket()->unregister_commands(asok_hook); - delete asok_hook; - asok_hook = NULL; -} - bool ClusterState::asok_command( std::string_view admin_command, const cmdmap_t& cmdmap, diff --git a/src/mgr/ClusterState.h b/src/mgr/ClusterState.h index 7939cd8eb8f..2beac362b47 100644 --- a/src/mgr/ClusterState.h +++ b/src/mgr/ClusterState.h @@ -152,7 +152,6 @@ public: } void final_init(); - void shutdown(); bool asok_command(std::string_view admin_command, const cmdmap_t& cmdmap, Formatter *f, diff --git a/src/mgr/DaemonServer.cc b/src/mgr/DaemonServer.cc index a4e85e2fc42..b1781316f82 100644 --- a/src/mgr/DaemonServer.cc +++ b/src/mgr/DaemonServer.cc @@ -98,7 +98,6 @@ DaemonServer::DaemonServer(MonClient *monc_, audit_clog(audit_clog_), pgmap_ready(false), timer(g_ceph_context, lock), - shutting_down(false), tick_event(nullptr), osd_perf_metric_collector_listener(this), osd_perf_metric_collector(osd_perf_metric_collector_listener), @@ -358,11 +357,6 @@ void DaemonServer::schedule_tick_locked(double delay_sec) tick_event = nullptr; } - // on shutdown start rejecting explicit requests to send reports that may - // originate from python land which may still be running. - if (shutting_down) - return; - tick_event = timer.add_event_after(delay_sec, new LambdaContext([this](int r) { tick(); @@ -407,19 +401,6 @@ void DaemonServer::handle_mds_perf_metric_query_updated() })); } -void DaemonServer::shutdown() -{ - dout(10) << "begin" << dendl; - msgr->shutdown(); - msgr->wait(); - cluster_state.shutdown(); - dout(10) << "done" << dendl; - - std::lock_guard l(lock); - shutting_down = true; - timer.shutdown(); -} - static DaemonKey key_from_service( const std::string& service_name, int peer_type, diff --git a/src/mgr/DaemonServer.h b/src/mgr/DaemonServer.h index a7b64561004..43125533e74 100644 --- a/src/mgr/DaemonServer.h +++ b/src/mgr/DaemonServer.h @@ -190,7 +190,6 @@ private: void maybe_ready(int32_t osd_id); SafeTimer timer; - bool shutting_down; Context *tick_event; void tick(); void schedule_tick_locked(double delay_sec); @@ -255,7 +254,6 @@ private: public: int init(uint64_t gid, entity_addrvec_t client_addrs); - void shutdown(); entity_addrvec_t get_myaddrs() const; diff --git a/src/mgr/Mgr.cc b/src/mgr/Mgr.cc index 63ad530fae2..5bd2ffb246c 100644 --- a/src/mgr/Mgr.cc +++ b/src/mgr/Mgr.cc @@ -214,12 +214,6 @@ std::map Mgr::load_store() return loaded; } -void Mgr::handle_signal(int signum) -{ - ceph_assert(signum == SIGINT || signum == SIGTERM); - shutdown(); -} - static void handle_mgr_signal(int signum) { derr << " *** Got signal " << sig_str(signum) << " ***" << dendl; @@ -490,27 +484,6 @@ void Mgr::load_all_metadata() } } - -void Mgr::shutdown() -{ - dout(10) << "mgr shutdown init" << dendl; - finisher.queue(new LambdaContext([&](int) { - { - std::lock_guard l(lock); - // First stop the server so that we're not taking any more incoming - // requests - server.shutdown(); - } - // after the messenger is stopped, signal modules to shutdown via finisher - py_module_registry->active_shutdown(); - })); - - // Then stop the finisher to ensure its enqueued contexts aren't going - // to touch references to the things we're about to tear down - finisher.wait_for_empty(); - finisher.stop(); -} - void Mgr::handle_osd_map() { ceph_assert(ceph_mutex_is_locked_by_me(lock)); diff --git a/src/mgr/Mgr.h b/src/mgr/Mgr.h index 22ebdb68041..65931c331f3 100644 --- a/src/mgr/Mgr.h +++ b/src/mgr/Mgr.h @@ -94,9 +94,6 @@ public: bool ms_dispatch2(const ceph::ref_t& m); void background_init(Context *completion); - void shutdown(); - - void handle_signal(int signum); std::map get_services() const; diff --git a/src/mgr/MgrStandby.cc b/src/mgr/MgrStandby.cc index 545624eb79b..052e6868177 100644 --- a/src/mgr/MgrStandby.cc +++ b/src/mgr/MgrStandby.cc @@ -295,41 +295,6 @@ void MgrStandby::tick() )); } -void MgrStandby::shutdown() -{ - finisher.queue(new LambdaContext([&](int) { - std::lock_guard l(lock); - - dout(4) << "Shutting down" << dendl; - - py_module_registry.shutdown(); - // stop sending beacon first, I use monc to talk with monitors - timer.shutdown(); - // client uses monc and objecter - client.shutdown(); - mgrc.shutdown(); - // Stop asio threads, so leftover events won't call into shut down - // monclient/objecter. - poolctx.finish(); - // stop monc, so mon won't be able to instruct me to shutdown/activate after - // the active_mgr is stopped - monc.shutdown(); - if (active_mgr) { - active_mgr->shutdown(); - } - // objecter is used by monc and active_mgr - objecter.shutdown(); - // client_messenger is used by all of them, so stop it in the end - client_messenger->shutdown(); - })); - - // Then stop the finisher to ensure its enqueued contexts aren't going - // to touch references to the things we're about to tear down - finisher.wait_for_empty(); - finisher.stop(); - mgr_perf_stop(g_ceph_context); -} - void MgrStandby::respawn() { // --- WARNING TO FUTURE COPY/PASTERS --- diff --git a/src/mgr/MgrStandby.h b/src/mgr/MgrStandby.h index 0f06e3074a0..5d238c85577 100644 --- a/src/mgr/MgrStandby.h +++ b/src/mgr/MgrStandby.h @@ -79,7 +79,6 @@ public: bool ms_handle_refused(Connection *con) override; int init(); - void shutdown(); void respawn(); int main(std::vector args); void tick(); diff --git a/src/mgr/PyModuleRegistry.cc b/src/mgr/PyModuleRegistry.cc index f5f5008023f..eb2d2babe75 100644 --- a/src/mgr/PyModuleRegistry.cc +++ b/src/mgr/PyModuleRegistry.cc @@ -217,53 +217,6 @@ void PyModuleRegistry::active_start( } } -void PyModuleRegistry::active_shutdown() -{ - std::lock_guard locker(lock); - - if (active_modules != nullptr) { - active_modules->shutdown(); - active_modules.reset(); - } -} - -void PyModuleRegistry::shutdown() -{ - std::lock_guard locker(lock); - - if (standby_modules != nullptr) { - standby_modules->shutdown(); - standby_modules.reset(); - } - - // Ideally, now, we'd be able to do this for all modules: - // - // Py_EndInterpreter(pMyThreadState); - // PyThreadState_Swap(pMainThreadState); - // - // Unfortunately, if the module has any other *python* threads active - // at this point, Py_EndInterpreter() will abort with: - // - // Fatal Python error: Py_EndInterpreter: not the last thread - // - // This can happen when using CherryPy in a module, becuase CherryPy - // runs an extra thread as a timeout monitor, which spends most of its - // life inside a time.sleep(60). Unless you are very, very lucky with - // the timing calling this destructor, that thread will still be stuck - // in a sleep, and Py_EndInterpreter() will abort. - // - // This could of course also happen with a poorly written module which - // made no attempt to clean up any additional threads it created. - // - // The safest thing to do is just not call Py_EndInterpreter(), and - // let Py_Finalize() kill everything after all modules are shut down. - - modules.clear(); - - PyEval_RestoreThread(pMainThreadState); - Py_Finalize(); -} - std::vector PyModuleRegistry::probe_modules(const std::string &path) const { const auto opt = g_conf().get_val("mgr_disabled_modules"); diff --git a/src/mgr/PyModuleRegistry.h b/src/mgr/PyModuleRegistry.h index 9af9abb5762..9d6d9c2cdd0 100644 --- a/src/mgr/PyModuleRegistry.h +++ b/src/mgr/PyModuleRegistry.h @@ -122,9 +122,6 @@ public: return standby_modules != nullptr; } - void active_shutdown(); - void shutdown(); - std::vector get_commands() const; std::vector get_py_commands() const; -- 2.39.5