Keep all dependencies (soft and regular) in a single list.
authorDavin McCall <davmac@davmac.org>
Tue, 5 Sep 2017 08:14:39 +0000 (09:14 +0100)
committerDavin McCall <davmac@davmac.org>
Tue, 5 Sep 2017 20:21:13 +0000 (21:21 +0100)
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
src/service.h

index 47081d7656fb5c8d417b792e340fb0e11825bb2a..c170dad12758f100a1c228d466bccf4a5bb0b963 100644 (file)
@@ -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;
index 46b08eac0e409f65685fd71aa23ed7e50b9a2eac..2040fe5c456d328d80721516a1b5258e6c471bc5 100644 (file)
@@ -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<service_dep> softdep_list;
+    typedef std::list<service_dep> dep_list;
     
     // list of soft dependents
-    typedef std::list<service_dep *> softdpt_list;
+    typedef std::list<service_dep *> 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;
         }
     }