From dfa4e453b71a5f0c20ae5735aba5980d43c18bf9 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Wed, 9 Nov 2016 14:28:48 +0000 Subject: [PATCH] Combine start/stop queues, and add propagation queue. The propagation queue is used to propagate changes other than immediate start/stop requests. It might be possible to combine the two queues at a later stage. This commit eliminates all recursion from service.cc. --- src/service.cc | 92 +++++++++++++++++++++++++++++++++++++++----------- src/service.h | 84 +++++++++++++++++++++++++-------------------- 2 files changed, 119 insertions(+), 57 deletions(-) diff --git a/src/service.cc b/src/service.cc index d140fc4..4df414d 100644 --- a/src/service.cc +++ b/src/service.cc @@ -296,14 +296,11 @@ Rearm ServiceIoWatcher::fdEvent(EventLoop_t &loop, int fd, int flags) noexcept void ServiceRecord::require() noexcept { if (required_by++ == 0) { - // Need to require all our dependencies - for (sr_iter i = depends_on.begin(); i != depends_on.end(); ++i) { - (*i)->require(); - } - - for (auto i = soft_deps.begin(); i != soft_deps.end(); ++i) { - ServiceRecord * to = i->getTo(); - to->require(); + + if (! prop_require) { + prop_require = true; + prop_release = false; + service_set->addToPropQueue(this); } if (service_state == ServiceState::STOPPED) { @@ -319,7 +316,9 @@ void ServiceRecord::release() noexcept desired_state = ServiceState::STOPPED; // Can stop, and release dependencies once we're stopped. if (service_state == ServiceState::STOPPED) { - release_dependencies(); + prop_release = true; + prop_require = false; + service_set->addToPropQueue(this); } else { service_set->addToStopQueue(this); @@ -359,6 +358,65 @@ void ServiceRecord::start(bool activate) noexcept service_set->addToStartQueue(this); } +void ServiceRecord::do_propagation() noexcept +{ + if (prop_require) { + // Need to require all our dependencies + for (sr_iter i = depends_on.begin(); i != depends_on.end(); ++i) { + (*i)->require(); + } + + for (auto i = soft_deps.begin(); i != soft_deps.end(); ++i) { + ServiceRecord * to = i->getTo(); + to->require(); + } + + prop_require = false; + } + + if (prop_release) { + release_dependencies(); + prop_release = false; + } + + if (prop_failure) { + prop_failure = false; + failed_to_start(true); + } + + if (waiting_for_deps) { + if (service_state == ServiceState::STARTING) { + if (startCheckDependencies(false)) { + allDepsStarted(); + } + } + else if (service_state == ServiceState::STOPPING) { + if (stopCheckDependents()) { + allDepsStopped(); + } + } + } +} + +void ServiceRecord::execute_transition() noexcept +{ + bool is_started = (service_state == ServiceState::STARTED) + || (service_state == ServiceState::STARTING && can_interrupt_start()); + bool is_stopped = (service_state == ServiceState::STOPPED) + || (service_state == ServiceState::STOPPING && can_interrupt_stop()); + + if (is_started && (desired_state == ServiceState::STOPPED || force_stop)) { + if (! pinned_started) { + do_stop(); + } + } + else if (is_stopped && desired_state == ServiceState::STARTED) { + if (! pinned_stopped) { + do_start(); + } + } +} + void ServiceRecord::do_start() noexcept { if (pinned_stopped) return; @@ -390,12 +448,8 @@ void ServiceRecord::do_start() noexcept void ServiceRecord::dependencyStarted() noexcept { - if (service_state != ServiceState::STARTING || ! waiting_for_deps) { - return; - } - - if (startCheckDependencies(false)) { - allDepsStarted(); + if (service_state == ServiceState::STARTING && waiting_for_deps) { + service_set->addToPropQueue(this); } } @@ -633,7 +687,8 @@ void ServiceRecord::failed_to_start(bool depfailed) noexcept // Cancel start of dependents: for (sr_iter i = dependents.begin(); i != dependents.end(); i++) { if ((*i)->service_state == ServiceState::STARTING) { - (*i)->failed_to_start(true); + (*i)->prop_failure = true; + service_set->addToPropQueue(*i); } } for (auto i = soft_dpts.begin(); i != soft_dpts.end(); i++) { @@ -856,10 +911,7 @@ void ServiceRecord::forceStop() noexcept void ServiceRecord::dependentStopped() noexcept { if (service_state == ServiceState::STOPPING && waiting_for_deps) { - // Check the other dependents before we stop. - if (stopCheckDependents()) { - allDepsStopped(); - } + service_set->addToPropQueue(this); } } diff --git a/src/service.h b/src/service.h index 6ff70bb..179799f 100644 --- a/src/service.h +++ b/src/service.h @@ -234,6 +234,11 @@ class ServiceRecord bool doing_recovery : 1; // if we are currently recovering a BGPROCESS (restarting process, while // holding STARTED service state) bool start_explicit : 1; // whether we are are explictly required to be started + + bool prop_require : 1; // require must be propagated + bool prop_release : 1; // release must be propagated + bool prop_failure : 1; // failure to start must be propagated + int required_by = 0; // number of dependents wanting this service to be started typedef std::list sr_list; @@ -287,8 +292,8 @@ class ServiceRecord // Next service (after this one) in the queue for the console. Intended to only be used by ServiceSet class. ServiceRecord *next_for_console; - // Start/stop queues - ServiceRecord *next_in_start_queue = nullptr; + // Propagation and start/stop queues + ServiceRecord *next_in_prop_queue = nullptr; ServiceRecord *next_in_stop_queue = nullptr; @@ -393,7 +398,8 @@ class ServiceRecord : service_state(ServiceState::STOPPED), desired_state(ServiceState::STOPPED), auto_restart(false), pinned_stopped(false), pinned_started(false), waiting_for_deps(false), waiting_for_execstat(false), doing_recovery(false), - start_explicit(false), force_stop(false), child_listener(this), child_status_listener(this) + start_explicit(false), prop_require(false), prop_release(false), prop_failure(false), + force_stop(false), child_listener(this), child_status_listener(this) { service_set = set; service_name = name; @@ -426,7 +432,12 @@ class ServiceRecord } // TODO write a destructor - + + // begin transition from stopped to started state or vice versa depending on current and desired state + void execute_transition() noexcept; + + void do_propagation() noexcept; + // Called on transition of desired state from stopped to started (or unpinned stop) void do_start() noexcept; @@ -549,15 +560,14 @@ class ServiceRecord * two "queues" (not really queues since their order is not important) are used to prevent too * much recursion and to prevent service states from "bouncing" too rapidly. * - * A service that wishes to stop puts itself on the stop queue; a service that wishes to start - * puts itself on the start queue. Any operation that potentially manipulates the queues must - * be folloed by a "process queues" order (processQueues method, which can be instructed to - * process either the start queue or the stop queue first). + * A service that wishes to start or stop puts itself on the start/stop queue; a service that + * needs to propagate changes to dependent services or dependencies puts itself on the + * propagation queue. Any operation that potentially manipulates the queues must be followed + * by a "process queues" order (processQueues() method). * - * Note that which queue it does process first, processQueues always repeatedly processes both - * queues until they are empty. The process is finite because starting a service can never - * cause services to be added to the stop queue, unless they fail to start, which should cause - * them to stop semi-permanently. + * Note that processQueues always repeatedly processes both queues until they are empty. The + * process is finite because starting a service can never cause services to stop, unless they + * fail to start, which should cause them to stop semi-permanently. */ class ServiceSet { @@ -571,8 +581,8 @@ class ServiceSet ServiceRecord * console_queue_head = nullptr; // first record in console queue ServiceRecord * console_queue_tail = nullptr; // last record in console queue - // start/stop "queue" - list of services waiting to stop/start - ServiceRecord * first_start_queue = nullptr; + // Propagation and start/stop "queues" - list of services waiting for processing + ServiceRecord * first_prop_queue = nullptr; ServiceRecord * first_stop_queue = nullptr; // Private methods @@ -628,16 +638,23 @@ class ServiceSet // transition to the 'stopped' state. void stopService(const std::string &name) noexcept; - // Add a service record to the start queue - void addToStartQueue(ServiceRecord *service) noexcept + // Add a service record to the state propogation queue + void addToPropQueue(ServiceRecord *service) noexcept { - if (service->next_in_start_queue == nullptr && first_start_queue != service) { - service->next_in_start_queue = first_start_queue; - first_start_queue = service; + if (service->next_in_prop_queue == nullptr && first_prop_queue != service) { + service->next_in_prop_queue = first_prop_queue; + first_prop_queue = service; } } - // Add a service to the stop queue + // Add a service record to the start queue; called by service record + void addToStartQueue(ServiceRecord *service) noexcept + { + // The start/stop queue is actually one queue: + addToStopQueue(service); + } + + // Add a service to the stop queue; called by service record void addToStopQueue(ServiceRecord *service) noexcept { if (service->next_in_stop_queue == nullptr && first_stop_queue != service) { @@ -646,29 +663,22 @@ class ServiceSet } } - void processQueues(bool do_start_first) noexcept + // Process state propagation and start/stop queues, until they are empty. + // TODO remove the pointless parameter + void processQueues(bool ignoredparam = false) noexcept { - if (! do_start_first) { - while (first_stop_queue != nullptr) { - auto next = first_stop_queue; - first_stop_queue = next->next_in_stop_queue; - next->next_in_stop_queue = nullptr; - next->do_stop(); - } - } - - while (first_stop_queue != nullptr || first_start_queue != nullptr) { - while (first_start_queue != nullptr) { - auto next = first_start_queue; - first_start_queue = next->next_in_start_queue; - next->next_in_start_queue = nullptr; - next->do_start(); + while (first_stop_queue != nullptr || first_prop_queue != nullptr) { + while (first_prop_queue != nullptr) { + auto next = first_prop_queue; + first_prop_queue = next->next_in_prop_queue; + next->next_in_prop_queue = nullptr; + next->do_propagation(); } while (first_stop_queue != nullptr) { auto next = first_stop_queue; first_stop_queue = next->next_in_stop_queue; next->next_in_stop_queue = nullptr; - next->do_stop(); + next->execute_transition(); } } } -- 2.25.1