WarmRestart class changes

The current WarmStart class assumes that restarting applications will invoke checkWarmStart() method during initialization. Currently, this method serves two main purposes: 1) a static-constructor and 2) checks if WarmRestart feature has been enabled for a given application. In this PR i'm proposing to split these different logics in two separated methods. The main goal here is to allow applications such as fpmSyncd to call checkWarmStart whenever it please (i.e. fpmSyncd restart or zebra<->fpmSyncd tcp re-establishment). There could be other applications in the future interested in this differentiated approch. Other than that i'm making small adjustments in the code to enhance readibility. Signed-off-by: Rodny Molina <[email protected]>
sonic-net · Sep 5, 2018 · 1f7a0cc · 1f7a0cc
1 parent e2e35e3
commit 1f7a0cc
Show file tree

Hide file tree

Showing 6 changed files with 103 additions and 52 deletions.
diff --git a/cfgmgr/vlanmgrd.cpp b/cfgmgr/vlanmgrd.cpp
@@ -57,6 +57,7 @@ int main(int argc, char **argv)
         DBConnector appDb(APPL_DB, DBConnector::DEFAULT_UNIXSOCKET, 0);
         DBConnector stateDb(STATE_DB, DBConnector::DEFAULT_UNIXSOCKET, 0);
 
+        WarmStart::initialize("vlanmgrd");
         WarmStart::checkWarmStart("vlanmgrd");
 
         /*

diff --git a/orchagent/main.cpp b/orchagent/main.cpp
@@ -101,6 +101,8 @@ int main(int argc, char **argv)
     swss::Logger::linkToDbNative("orchagent");
 
     SWSS_LOG_ENTER();
+
+    WarmStart::initialize("orchagent");
     WarmStart::checkWarmStart("orchagent");
 
     if (signal(SIGHUP, sighup_handler) == SIG_ERR)

diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp
@@ -350,7 +350,7 @@ void OrchDaemon::start()
  */
 bool OrchDaemon::warmRestoreAndSyncUp()
 {
-    WarmStart::setWarmStartState("orchagent", WarmStart::INIT);
+    WarmStart::setWarmStartState("orchagent", WarmStart::INITIALIZED);
 
     for (Orch *o : m_orchList)
     {

diff --git a/portsyncd/portsyncd.cpp b/portsyncd/portsyncd.cpp
@@ -75,6 +75,7 @@ int main(int argc, char **argv)
     ProducerStateTable p(&appl_db, APP_PORT_TABLE_NAME);
     SubscriberStateTable portCfg(&cfgDb, CFG_PORT_TABLE_NAME);
 
+    WarmStart::initialize("portsyncd");
     WarmStart::checkWarmStart("portsyncd");
     if (WarmStart::isWarmStart())
     {

diff --git a/warmrestart/warm_restart.cpp b/warmrestart/warm_restart.cpp
@@ -1,24 +1,54 @@
 #include <string>
+#include <climits>
 #include "logger.h"
 #include "schema.h"
-#include <climits>
 #include "warm_restart.h"
 
 namespace swss {
 
 const WarmStart::WarmStartStateNameMap WarmStart::warmStartStateNameMap =
 {
-    {INIT,          "init"},
+    {INITIALIZED,   "initialized"},
     {RESTORED,      "restored"},
     {RECONCILED,    "reconciled"}
 };
 
-WarmStart &WarmStart::getInstance()
+WarmStart &WarmStart::getInstance(void)
 {
     static WarmStart m_warmStart;
     return m_warmStart;
 }
 
+/*
+ * WarmStart's initialization method -- to be invoked once per application.
+ */
+void WarmStart::initialize(const std::string &app_name,
+                           const std::string &docker_name)
+{
+    auto& warmStart = getInstance();
+
+    if (warmStart.m_initialized)
+    {
+        return;
+    }
+
+    warmStart.m_stateDb =
+        std::make_shared<swss::DBConnector>(STATE_DB, swss::DBConnector::DEFAULT_UNIXSOCKET, 0);
+
+    warmStart.m_stateWarmRestartTable =
+        std::unique_ptr<Table>(new Table(warmStart.m_stateDb.get(), STATE_WARM_RESTART_TABLE_NAME));
+
+    warmStart.m_cfgDb =
+        std::make_shared<swss::DBConnector>(CONFIG_DB, swss::DBConnector::DEFAULT_UNIXSOCKET, 0);
+
+    warmStart.m_cfgWarmRestartTable =
+        std::unique_ptr<Table>(new Table(warmStart.m_cfgDb.get(), CFG_WARM_RESTART_TABLE_NAME));
+
+    setWarmStartState(app_name, WarmStart::INITIALIZED);
+
+    warmStart.m_initialized = true;
+}
+
 /*
  * <1> Upon system reboot, the system enable knob will be checked.
  * If enabled, database data will be preserved, if not, database will be flushed.
@@ -34,45 +64,39 @@ WarmStart &WarmStart::getInstance()
  * the database has been flushed, do cold start. Otherwise warm start.
  */
 
-// Check warm start flag at the very begining of application, do it once for each process.
-bool WarmStart::checkWarmStart(const std::string &app_name, const std::string &docker_name)
+/*
+ * Method to verify/obtain the state of Warm-Restart feature for any warm-reboot
+ * capable component. Typically, this function will be called during initialization of
+ * SONiC modules; however, method could be invoked at any given point to verify the
+ * latest state of Warm-Restart functionality.
+ */
+bool WarmStart::checkWarmStart(const std::string &app_name,
+                               const std::string &docker_name)
 {
-    auto& warmStart = getInstance();
-
-    if (warmStart.m_stateDb)
-    {
-        return true;
-    }
-
-    warmStart.m_stateDb     = std::make_shared<swss::DBConnector>(STATE_DB, swss::DBConnector::DEFAULT_UNIXSOCKET, 0);
-    warmStart.m_stateWarmRestartTable = std::unique_ptr<Table>(new Table(warmStart.m_stateDb.get(), STATE_WARM_RESTART_TABLE_NAME));
-
-    warmStart.m_cfgDb       = std::make_shared<swss::DBConnector>(CONFIG_DB, swss::DBConnector::DEFAULT_UNIXSOCKET, 0);
-    warmStart.m_cfgWarmRestartTable   = std::unique_ptr<Table>(new Table(warmStart.m_cfgDb.get(), CFG_WARM_RESTART_TABLE_NAME));
+    std::string value;
 
-    warmStart.enabled = false;
+    auto& warmStart = getInstance();
 
-    std::string value;
-    // Check system level warm restart config first
+    // Check system level warm-restart config first
     warmStart.m_cfgWarmRestartTable->hget("system", "enable", value);
     if (value == "true")
     {
-        warmStart.enabled = true;
+        warmStart.m_enabled = true;
     }
 
-    // docker level warm restart configuration
+    // Check docker level warm-restart configuration
     warmStart.m_cfgWarmRestartTable->hget(docker_name, "enable", value);
     if (value == "true")
     {
-        warmStart.enabled = true;
+        warmStart.m_enabled = true;
     }
 
     // For cold start, the whole state db will be flushed including warm start table.
     // Create the entry for this app here.
-    if (!warmStart.enabled)
+    if (!warmStart.m_enabled)
     {
         warmStart.m_stateWarmRestartTable->hset(app_name, "restart_count", "0");
-        return true;
+        return false;
     }
 
     uint32_t restart_count = 0;
@@ -81,28 +105,34 @@ bool WarmStart::checkWarmStart(const std::string &app_name, const std::string &d
     {
         SWSS_LOG_WARN("%s doing warm start, but restart_count not found in stateDB %s table, fall back to cold start",
                 app_name.c_str(), STATE_WARM_RESTART_TABLE_NAME);
-        warmStart.enabled = false;
+        warmStart.m_enabled = false;
         warmStart.m_stateWarmRestartTable->hset(app_name, "restart_count", "0");
-        return true;
+        return false;
     }
     else
     {
         restart_count = (uint32_t)stoul(value);
     }
 
     restart_count++;
-    warmStart.m_stateWarmRestartTable->hset(app_name, "restart_count", std::to_string(restart_count));
-    SWSS_LOG_NOTICE("%s doing warm start, restart count %d", app_name.c_str(), restart_count);
+    warmStart.m_stateWarmRestartTable->hset(app_name, "restart_count",
+                                            std::to_string(restart_count));
+
+    SWSS_LOG_NOTICE("%s doing warm start, restart count %d", app_name.c_str(),
+                    restart_count);
 
     return true;
 }
 
 /*
- * Get warmStartTimer for an application in a docker (default to be swss)
- * return timer value. Value 0 means invalid.
+ * Obtain the time-interval defined by a warm-restart-capable application
+ * corresponding to the amount of time required to complete a full-restart cycle.
+ * This time-duration (warmStartTimer) will be taken into account by the
+ * warm-restart logic to kick-off the reconciliation process of this application.
+ * A returned value of '0' implies that no valid interval was found.
  */
 uint32_t WarmStart::getWarmStartTimer(const std::string &app_name,
-    const std::string &docker_name)
+                                      const std::string &docker_name)
 {
     auto& warmStart = getInstance();
     std::string timer_name = app_name + "_timer";
@@ -111,34 +141,41 @@ uint32_t WarmStart::getWarmStartTimer(const std::string &app_name,
     warmStart.m_cfgWarmRestartTable->hget(docker_name, timer_name, timer_value_str);
 
     unsigned long int temp_value = strtoul(timer_value_str.c_str(), NULL, 0);
-    if (temp_value != 0 && temp_value != ULONG_MAX && temp_value <= MAXIMUN_WARMRESTART_TIMER_VALUE)
+
+    if (temp_value != 0 && temp_value != ULONG_MAX &&
+        temp_value <= MAXIMUN_WARMRESTART_TIMER_VALUE)
     {
         SWSS_LOG_NOTICE("Getting warmStartTimer for docker: %s, app: %s, value: %lu",
-                docker_name.c_str(), app_name.c_str(), temp_value);
+                        docker_name.c_str(), app_name.c_str(), temp_value);
         return (uint32_t)temp_value;
     }
     else
     {
         SWSS_LOG_NOTICE("warmStartTimer is not configured or invalid for docker: %s, app: %s",
-                docker_name.c_str(), app_name.c_str());
+                        docker_name.c_str(), app_name.c_str());
         return 0;
     }
 }
 
-bool WarmStart::isWarmStart()
+bool WarmStart::isWarmStart(void)
 {
     auto& warmStart = getInstance();
 
-    return warmStart.enabled;
+    return warmStart.m_enabled;
 }
 
-// Set the state restored flag
+// Set the WarmStart FSM state for a particular application.
 void WarmStart::setWarmStartState(const std::string &app_name, WarmStartState state)
 {
     auto& warmStart = getInstance();
 
-    warmStart.m_stateWarmRestartTable->hset(app_name, "state", warmStartStateNameMap.at(state).c_str());
-    SWSS_LOG_NOTICE("%s warm start state changed to %s", app_name.c_str(), warmStartStateNameMap.at(state).c_str());
+    warmStart.m_stateWarmRestartTable->hset(app_name,
+                                            "state",
+                                            warmStartStateNameMap.at(state).c_str());
+
+    SWSS_LOG_NOTICE("%s warm start state changed to %s",
+                    app_name.c_str(),
+                    warmStartStateNameMap.at(state).c_str());
 }
 
 }
diff --git a/warmrestart/warm_restart.h b/warmrestart/warm_restart.h
@@ -14,27 +14,37 @@ class WarmStart
 public:
 	enum WarmStartState
 	{
-	    INIT,
+	    INITIALIZED,
 	    RESTORED,
 	    RECONCILED,
 	};
 
     typedef std::map<WarmStartState, std::string>  WarmStartStateNameMap;
     static const WarmStartStateNameMap warmStartStateNameMap;
 
-    static WarmStart &getInstance();
+    static WarmStart &getInstance(void);
+
+    static void initialize(const std::string &app_name,
+                           const std::string &docker_name = "swss");
+
+    static bool checkWarmStart(const std::string &app_name,
+                               const std::string &docker_name = "swss");
+
+    static bool isWarmStart(void);
+
+    static void setWarmStartState(const std::string &app_name,
+                                  WarmStartState     state);
 
-    static bool checkWarmStart(const std::string &app_name, const std::string &docker_name = "swss");
     static uint32_t getWarmStartTimer(const std::string &app_name,
-        const std::string &docker_name ="swss");
-    static bool isWarmStart();
-    static void setWarmStartState(const std::string &app_name, WarmStartState state);
+                                      const std::string &docker_name ="swss");
+
 private:
-	std::shared_ptr<swss::DBConnector>          m_stateDb;
-	std::shared_ptr<swss::DBConnector>          m_cfgDb;
-	std::unique_ptr<Table>         m_stateWarmRestartTable;
-	std::unique_ptr<Table>         m_cfgWarmRestartTable;
-	bool enabled;
+    std::shared_ptr<swss::DBConnector>   m_stateDb;
+    std::shared_ptr<swss::DBConnector>   m_cfgDb;
+    std::unique_ptr<Table>               m_stateWarmRestartTable;
+    std::unique_ptr<Table>               m_cfgWarmRestartTable;
+    bool                                 m_initialized;
+    bool                                 m_enabled;
 };
 
 }