From 999f386e42baffbf7bd4e33fd9d02c88ad72e391 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Wed, 9 Sep 2015 20:52:23 +0100 Subject: [PATCH] Finish soft dependency support. If a soft dependency fails to start, it no longer cancels its dependent's start. --- service.cc | 78 +++++++++++++++++++++++++++++++++++++----------------- service.h | 54 ++++++++++++++++++++++++++++++++++--- 2 files changed, 104 insertions(+), 28 deletions(-) diff --git a/service.cc b/service.cc index a040deb..27a3a1e 100644 --- a/service.cc +++ b/service.cc @@ -132,20 +132,20 @@ void ServiceRecord::start() // TODO any listeners waiting for stop should be notified of // its cancellation } - - desired_state = SVC_STARTED; - if (service_state != SVC_STOPPED) { - // Either we need do nothing (service is already started/starting) - // or the service is currently being stopped and we must wait for - // that to complete. + auto old_desired_state = desired_state; + desired_state = SVC_STARTED; + + if (service_state == SVC_STARTED || service_state == SVC_STARTING) { + // We couldn't be started or starting unless all dependencies have + // already started: so there's nothing left to do. return; } - // Service state is SVC_STOPPED. Start the service. - - // First, start dependencies bool all_deps_started = true; + + // Ask dependencies to start, mark them as being waited on. + for (sr_iter i = depends_on.begin(); i != depends_on.end(); ++i) { // Note, we cannot treat a dependency as started if its force_stop // flag is set. @@ -155,6 +155,35 @@ void ServiceRecord::start() } } + if (old_desired_state != SVC_STARTED) { + // This is a fresh start, so we mark all soft dependencies as 'waiting on' and ask them + // to start: + for (auto i = soft_deps.begin(); i != soft_deps.end(); ++i) { + if (i->getTo()->service_state != SVC_STARTED) { + all_deps_started = false; + i->getTo()->start(); + i->waiting_on = true; + } + } + } + else { + // This is (or at least may be) a notification that a dependency is ready; let's + // just check them: + for (auto i = soft_deps.begin(); i != soft_deps.end(); ++i) { + ServiceRecord * to = i->getTo(); + if (i->waiting_on) { + if ((to->desired_state != SVC_STARTED && to->service_state != SVC_STARTING) || to->service_state == SVC_STARTED) { + // Service has either started or is no longer starting + i->waiting_on = false; + } + else { + all_deps_started = false; + } + } + } + } + + if (! all_deps_started) { // The dependencies will notify this service once they've started. return; @@ -189,14 +218,14 @@ void ServiceRecord::started() if (desired_state == SVC_STARTED) { // Start any dependents whose desired state is SVC_STARTED: - for (sr_iter i = dependents.begin(); i != dependents.end(); i++) { + for (auto i = dependents.begin(); i != dependents.end(); i++) { if ((*i)->desired_state == SVC_STARTED) { (*i)->start(); } } - for (sr_iter i = soft_dependents.begin(); i != soft_dependents.end(); i++) { - if ((*i)->desired_state == SVC_STARTED) { - (*i)->start(); + for (auto i = soft_dpts.begin(); i != soft_dpts.end(); i++) { + if ((*i)->getFrom()->desired_state == SVC_STARTED) { + (*i)->getFrom()->start(); } } } @@ -217,14 +246,11 @@ void ServiceRecord::failed_to_start() (*i)->failed_dependency(); } } - // What about soft dependents? - // TODO we should probably send them "start" rather than "failed_dependency", - // or add a parameter to failed_dependency which says whether the dependency - // was soft and change start handling as appropriate. - // For now just fail the startup: - for (sr_iter i = soft_dependents.begin(); i != soft_dependents.end(); i++) { - if ((*i)->desired_state == SVC_STARTED) { - (*i)->failed_dependency(); + for (auto i = soft_dpts.begin(); i != soft_dpts.end(); i++) { + if ((*i)->getFrom()->desired_state == SVC_STARTED) { + // We can send 'start', because this is only a soft dependency. + // Our startup failure means that they don't have to wait for us. + (*i)->getFrom()->start(); } } } @@ -352,14 +378,16 @@ void ServiceRecord::failed_dependency() service_state = SVC_STOPPED; // Notify dependents of this service also - for (sr_iter i = dependents.begin(); i != dependents.end(); i++) { + for (auto i = dependents.begin(); i != dependents.end(); i++) { if ((*i)->desired_state == SVC_STARTED) { (*i)->failed_dependency(); } } - for (sr_iter i = soft_dependents.begin(); i != soft_dependents.end(); i++) { - if ((*i)->desired_state == SVC_STARTED) { - (*i)->failed_dependency(); + for (auto i = soft_dpts.begin(); i != soft_dpts.end(); i++) { + if ((*i)->getFrom()->desired_state == SVC_STARTED) { + // It's a soft dependency, so send them 'started' rather than + // 'failed dep'. + (*i)->getFrom()->started(); } } } diff --git a/service.h b/service.h index 5dea8a8..c7ba8de 100644 --- a/service.h +++ b/service.h @@ -81,9 +81,35 @@ class ServiceDescriptionExc : public ServiceLoadExc } }; - +class ServiceRecord; // forward declaration class ServiceSet; // forward declaration +/* Service dependency record */ +class ServiceDep +{ + ServiceRecord * from; + ServiceRecord * to; + + public: + /* Whether the 'from' service is waiting for the 'to' service to start */ + bool waiting_on; + + ServiceDep(ServiceRecord * from, ServiceRecord * to) : from(from), to(to), waiting_on(false) + { } + + ServiceRecord * getFrom() + { + return from; + } + + ServiceRecord * getTo() + { + return to; + } +}; + + + class ServiceRecord { typedef std::string string; @@ -102,9 +128,17 @@ class ServiceRecord typedef std::list sr_list; typedef sr_list::iterator sr_iter; + // list of soft dependencies + typedef std::list softdep_list; + + // list of soft dependents + typedef std::list softdpt_list; + sr_list depends_on; // services this one depends on sr_list dependents; // services depending on this one - sr_list soft_dependents; // services depending on this one via a "soft" dependency + softdep_list soft_deps; // services this one depends on via a soft dependency + softdpt_list soft_dpts; // services depending on this one via a soft dependency + // unsigned wait_count; /* if we are waiting for dependents/dependencies to // start/stop, this is how many we're waiting for */ @@ -176,8 +210,21 @@ class ServiceRecord // TODO splice the contents from the depends_on parameter // rather than duplicating the list. this->depends_on = *pdepends_on; - this->depends_on.insert(this->depends_on.end(), pdepends_soft->begin(), pdepends_soft->end()); + //this->depends_on.insert(this->depends_on.end(), pdepends_soft->begin(), pdepends_soft->end()); + + for (sr_iter i = pdepends_on->begin(); i != pdepends_on->end(); ++i) { + (*i)->dependents.push_back(this); + } + // Soft dependencies + auto b_iter = soft_deps.end(); + for (sr_iter i = pdepends_soft->begin(); i != pdepends_soft->end(); ++i) { + b_iter = soft_deps.emplace(b_iter, this, *i); + (*i)->soft_dpts.push_back(&(*b_iter)); + ++b_iter; + } + + /* // For each dependency, add us as a dependent. for (sr_iter i = pdepends_on->begin(); i != pdepends_on->end(); ++i) { (*i)->dependents.push_back(this); @@ -185,6 +232,7 @@ class ServiceRecord for (sr_iter i = pdepends_soft->begin(); i != pdepends_soft->end(); ++i) { (*i)->soft_dependents.push_back(this); } + */ } // Set logfile, should be done before service is started -- 2.25.1