From e0dfaef51bdc166e338b82551d395199feea6d34 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Tue, 5 Sep 2017 09:14:39 +0100 Subject: [PATCH] Keep all dependencies (soft and regular) in a single list. We had separate lists for regular dependencies, dependents, and soft dependencies and dependents. Combining the two dependency types simplifies code in several places and allows for adding new dependency types much more easily. --- src/service.cc | 172 ++++++++++++++++++++++++------------------------- src/service.h | 37 +++++++---- 2 files changed, 107 insertions(+), 102 deletions(-) diff --git a/src/service.cc b/src/service.cc index 47081d7..c170dad 100644 --- a/src/service.cc +++ b/src/service.cc @@ -70,10 +70,12 @@ void service_record::stopped() noexcept force_stop = false; // If we are a soft dependency of another target, break the acquisition from that target now: - for (auto dependent : soft_dpts) { - if (dependent->holding_acq) { - dependent->holding_acq = false; - release(); + for (auto & dependent : dependents) { + if (dependent->dep_type == dependency_type::SOFT) { + if (dependent->holding_acq) { + dependent->holding_acq = false; + release(); + } } } @@ -82,7 +84,7 @@ void service_record::stopped() noexcept for (auto dependency : depends_on) { // we signal dependencies in case they are waiting for us to stop: - dependency->dependent_stopped(); + dependency.get_to()->dependent_stopped(); } service_state = service_state_t::STOPPED; @@ -436,15 +438,11 @@ void service_record::release() noexcept void service_record::release_dependencies() noexcept { - for (sr_iter i = depends_on.begin(); i != depends_on.end(); ++i) { - (*i)->release(); - } - - for (auto i = soft_deps.begin(); i != soft_deps.end(); ++i) { - service_record * to = i->get_to(); - if (i->holding_acq) { - to->release(); - i->holding_acq = false; + for (auto & dependency : depends_on) { + service_record * dep_to = dependency.get_to(); + if (dependency.holding_acq) { + dep_to->release(); + dependency.holding_acq = false; } } } @@ -488,16 +486,10 @@ void service_record::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) { - service_record * to = i->get_to(); - to->require(); - i->holding_acq = true; + for (auto & dep : depends_on) { + dep.get_to()->require(); + dep.holding_acq = true; } - prop_require = false; } @@ -570,40 +562,41 @@ bool service_record::start_check_dependencies(bool start_deps) noexcept { bool all_deps_started = true; - for (sr_iter i = depends_on.begin(); i != depends_on.end(); ++i) { - if ((*i)->service_state != service_state_t::STARTED) { - if (start_deps) { - all_deps_started = false; - (*i)->prop_start = true; - services->add_prop_queue(*i); - } - else { - return false; - } - } - } - - for (auto i = soft_deps.begin(); i != soft_deps.end(); ++i) { - service_record * to = i->get_to(); - if (start_deps) { - if (to->service_state != service_state_t::STARTED) { - to->prop_start = true; - services->add_prop_queue(to); - i->waiting_on = true; - all_deps_started = false; - } - else { - i->waiting_on = false; + for (auto dep : depends_on) { + if (dep.dep_type == dependency_type::REGULAR) { + if (dep.get_to()->service_state != service_state_t::STARTED) { + if (start_deps) { + all_deps_started = false; + dep.get_to()->prop_start = true; + services->add_prop_queue(dep.get_to()); + } + else { + return false; + } } } - else if (i->waiting_on) { - if (to->service_state != service_state_t::STARTING) { - // Service has either started or is no longer starting - i->waiting_on = false; + else if (dep.dep_type == dependency_type::SOFT) { + service_record * to = dep.get_to(); + if (start_deps) { + if (to->service_state != service_state_t::STARTED) { + to->prop_start = true; + services->add_prop_queue(to); + dep.waiting_on = true; + all_deps_started = false; + } + else { + dep.waiting_on = false; + } } - else { - // We are still waiting on this service - return false; + else if (dep.waiting_on) { + if (to->service_state != service_state_t::STARTING) { + // Service has either started or is no longer starting + dep.waiting_on = false; + } + else { + // We are still waiting on this service + return false; + } } } } @@ -826,11 +819,8 @@ void service_record::started() noexcept } // Notify any dependents whose desired state is STARTED: - for (auto i = dependents.begin(); i != dependents.end(); i++) { - (*i)->dependencyStarted(); - } - for (auto i = soft_dpts.begin(); i != soft_dpts.end(); i++) { - (*i)->get_from()->dependencyStarted(); + for (auto dept : dependents) { + dept->get_from()->dependencyStarted(); } } @@ -850,20 +840,22 @@ void service_record::failed_to_start(bool depfailed) noexcept notify_listeners(service_event::FAILEDSTART); // Cancel start of dependents: - for (sr_iter i = dependents.begin(); i != dependents.end(); i++) { - if ((*i)->service_state == service_state_t::STARTING) { - (*i)->prop_failure = true; - services->add_prop_queue(*i); - } - } - for (auto i = soft_dpts.begin(); i != soft_dpts.end(); i++) { - // We can send 'start', because this is only a soft dependency. - // Our startup failure means that they don't have to wait for us. - if ((*i)->waiting_on) { - (*i)->holding_acq = false; - (*i)->waiting_on = false; - (*i)->get_from()->dependencyStarted(); - release(); + for (auto & dept : dependents) { + if (dept->dep_type == dependency_type::REGULAR) { + if (dept->get_from()->service_state == service_state_t::STARTING) { + dept->get_from()->prop_failure = true; + services->add_prop_queue(dept->get_from()); + } + } + else if (dept->dep_type == dependency_type::SOFT) { + if (dept->waiting_on) { + dept->waiting_on = false; + dept->get_from()->dependencyStarted(); + } + if (dept->holding_acq) { + dept->holding_acq = false; + release(); + } } } } @@ -1184,8 +1176,8 @@ void service_record::do_stop() noexcept bool service_record::stop_check_dependents() noexcept { bool all_deps_stopped = true; - for (sr_iter i = dependents.begin(); i != dependents.end(); ++i) { - if (! (*i)->is_stopped()) { + for (auto dept : dependents) { + if (dept->dep_type == dependency_type::REGULAR && ! dept->get_from()->is_stopped()) { all_deps_stopped = false; break; } @@ -1197,22 +1189,24 @@ bool service_record::stop_check_dependents() noexcept bool service_record::stop_dependents() noexcept { bool all_deps_stopped = true; - for (sr_iter i = dependents.begin(); i != dependents.end(); ++i) { - if (! (*i)->is_stopped()) { - // Note we check *first* since if the dependent service is not stopped, - // 1. We will issue a stop to it shortly and - // 2. It will notify us when stopped, at which point the stop_check_dependents() - // check is run anyway. - all_deps_stopped = false; - } + for (auto dept : dependents) { + if (dept->dep_type == dependency_type::REGULAR) { + if (! dept->get_from()->is_stopped()) { + // Note we check *first* since if the dependent service is not stopped, + // 1. We will issue a stop to it shortly and + // 2. It will notify us when stopped, at which point the stop_check_dependents() + // check is run anyway. + all_deps_stopped = false; + } - if (force_stop) { - // If this service is to be forcefully stopped, dependents must also be. - (*i)->forced_stop(); - } + if (force_stop) { + // If this service is to be forcefully stopped, dependents must also be. + dept->get_from()->forced_stop(); + } - (*i)->prop_stop = true; - services->add_prop_queue(*i); + dept->get_from()->prop_stop = true; + services->add_prop_queue(dept->get_from()); + } } return all_deps_stopped; diff --git a/src/service.h b/src/service.h index 46b08ea..2040fe5 100644 --- a/src/service.h +++ b/src/service.h @@ -163,6 +163,14 @@ class service_record; class service_set; class base_process_service; +enum class dependency_type +{ + REGULAR, + SOFT, // dependency starts in parallel, failure/stop does not affect dependent + WAITS_FOR, // as for SOFT, but dependent waits until dependency starts/fails before starting + MILESTONE // dependency must start successfully, but once started the dependency becomes soft +}; + /* Service dependency record */ class service_dep { @@ -175,7 +183,10 @@ class service_dep /* Whether the 'from' service is holding an acquire on the 'to' service */ bool holding_acq; - service_dep(service_record * from, service_record * to) noexcept : from(from), to(to), waiting_on(false), holding_acq(false) + const dependency_type dep_type; + + service_dep(service_record * from, service_record * to, dependency_type dep_type_p) noexcept + : from(from), to(to), waiting_on(false), holding_acq(false), dep_type(dep_type_p) { } service_record * get_from() noexcept @@ -276,15 +287,15 @@ class service_record typedef sr_list::iterator sr_iter; // list of soft dependencies - typedef std::list softdep_list; + typedef std::list dep_list; // list of soft dependents - typedef std::list softdpt_list; + typedef std::list dpt_list; - sr_list depends_on; // services this one depends on - sr_list dependents; // services depending on this one - 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 + //sr_list depends_on; // services this one depends on + //sr_list dependents; // services depending on this one + dep_list depends_on; // services this one depends on via a soft dependency + dpt_list dependents; // 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 */ @@ -439,17 +450,17 @@ class service_record services = set; service_name = name; this->record_type = record_type_p; - this->depends_on = std::move(pdepends_on); - for (sr_iter i = depends_on.begin(); i != depends_on.end(); ++i) { - (*i)->dependents.push_back(this); + for (auto pdep : pdepends_on) { + auto b = depends_on.emplace(depends_on.end(), this, pdep, dependency_type::REGULAR); + pdep->dependents.push_back(&(*b)); } // Soft dependencies - auto b_iter = soft_deps.end(); + auto b_iter = depends_on.end(); for (auto 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 = depends_on.emplace(b_iter, this, *i, dependency_type::SOFT); + (*i)->dependents.push_back(&(*b_iter)); ++b_iter; } } -- 2.25.1