Improve sairedis notifications (#10)

* Notifications should use ntf consumer and producer * Use SelectableEvent to notify thread end * Remove signal handler * Implement service_method_table * Add getopt options
sonic-net · Apr 26, 2016 · 59f5f53 · 59f5f53
1 parent 2ddebf6
commit 59f5f53
Show file tree

Hide file tree

Showing 6 changed files with 224 additions and 132 deletions.
diff --git a/lib/inc/sai_redis.h b/lib/inc/sai_redis.h
@@ -16,6 +16,8 @@ extern "C" {
 #include "swss/dbconnector.h"
 #include "swss/producertable.h"
 #include "swss/consumertable.h"
+#include "swss/notificationconsumer.h"
+#include "swss/notificationproducer.h"
 #include "swss/table.h"
 #include "swss/select.h"
 #include "swss/scheme.h"
@@ -25,11 +27,11 @@ extern service_method_table_t           g_services;
 extern swss::DBConnector               *g_db;
 extern swss::ProducerTable             *g_asicState;
 
-extern swss::ProducerTable             *g_notifySyncdProducer;
+extern swss::NotificationProducer      *g_notifySyncdProducer;
 extern swss::ProducerTable             *g_redisGetProducer;
 extern swss::ConsumerTable             *g_redisGetConsumer;
-extern swss::ConsumerTable             *g_redisNotifications;
-extern swss::ConsumerTable             *g_notifySyncdConsumer;
+extern swss::NotificationConsumer      *g_redisNotifications;
+extern swss::NotificationConsumer      *g_notifySyncdConsumer;
 
 extern swss::Table *g_vidToRid;
 extern swss::Table *g_ridToVid;

diff --git a/lib/src/sai_redis_interfacequery.cpp b/lib/src/sai_redis_interfacequery.cpp
@@ -12,11 +12,11 @@ swss::DBConnector     *g_dbNtf = NULL;
 swss::ProducerTable   *g_asicState = NULL;
 
 // we probably don't need those to tables to access GET requests
-swss::ProducerTable   *g_notifySyncdProducer = NULL;
-swss::ProducerTable   *g_redisGetProducer = NULL;
-swss::ConsumerTable   *g_redisGetConsumer = NULL;
-swss::ConsumerTable   *g_redisNotifications = NULL;
-swss::ConsumerTable   *g_notifySyncdConsumer = NULL;
+swss::NotificationProducer   *g_notifySyncdProducer = NULL;
+swss::ProducerTable          *g_redisGetProducer = NULL;
+swss::ConsumerTable          *g_redisGetConsumer = NULL;
+swss::NotificationConsumer   *g_redisNotifications = NULL;
+swss::NotificationConsumer   *g_notifySyncdConsumer = NULL;
 
 swss::RedisClient     *g_redisClient = NULL;
 
@@ -60,7 +60,7 @@ sai_status_t sai_api_initialize(
     if (g_notifySyncdProducer != NULL)
         delete g_notifySyncdProducer;
 
-    g_notifySyncdProducer = new swss::ProducerTable(g_db, "NOTIFYSYNCDREQUERY");
+    g_notifySyncdProducer = new swss::NotificationProducer(g_db, "NOTIFYSYNCDREQUERY");
 
     if (g_redisGetProducer != NULL)
         delete g_redisGetProducer;
@@ -70,7 +70,7 @@ sai_status_t sai_api_initialize(
     if (g_notifySyncdConsumer != NULL)
         delete g_notifySyncdConsumer;
 
-    g_notifySyncdConsumer = new swss::ConsumerTable(g_db, "NOTIFYSYNCRESPONSE");
+    g_notifySyncdConsumer = new swss::NotificationConsumer(g_db, "NOTIFYSYNCRESPONSE");
 
     if (g_redisGetConsumer != NULL)
         delete g_redisGetConsumer;
@@ -80,7 +80,7 @@ sai_status_t sai_api_initialize(
     if (g_redisNotifications != NULL)
         delete g_redisNotifications;
 
-    g_redisNotifications = new swss::ConsumerTable(g_dbNtf, "NOTIFICATIONS");
+    g_redisNotifications = new swss::NotificationConsumer(g_dbNtf, "NOTIFICATIONS");
 
     if (g_redisClient != NULL)
         delete g_redisClient;

diff --git a/lib/src/sai_redis_switch.cpp b/lib/src/sai_redis_switch.cpp
@@ -1,13 +1,15 @@
 #include "sai_redis.h"
 #include <thread>
 
+#include "selectableevent.h"
+
 // if we will not get response in 60 seconds when
 // notify syncd to compile new state or to switch
 // to compiled state, then there is something wrong
 #define NOTIFY_SYNCD_TIMEOUT (60*1000)
 
-#define NOTIFY_COMPILE  "compile"
-#define NOTIFY_SWITCH   "switch"
+#define NOTIFY_SAI_COMPILE_VIEW  "sai_compile_view"
+#define NOTIFY_SAI_SWITCH_VIEW   "sai_switch_view"
 
 sai_switch_notification_t redis_switch_notifications;
 
@@ -16,41 +18,45 @@ volatile bool g_run = false;
 
 std::shared_ptr<std::thread> notification_thread;
 
+// this event is used to nice end notifications thread
+swss::SelectableEvent g_redisNotificationTrheadEvent;
+
 void ntf_thread()
 {
     SWSS_LOG_ENTER();
 
     swss::Select s;
 
     s.addSelectable(g_redisNotifications);
+    s.addSelectable(&g_redisNotificationTrheadEvent);
 
     while (g_run)
     {
         swss::Selectable *sel;
 
         int fd;
 
-        int result = s.select(&sel, &fd, 500);
+        int result = s.select(&sel, &fd);
+
+        if (sel == &g_redisNotificationTrheadEvent)
+        {
+            // user requested shutdown_switch
+            break;
+        }
 
         if (result == swss::Select::OBJECT)
         {
             swss::KeyOpFieldsValuesTuple kco;
 
-            g_redisNotifications->pop(kco);
+            std::string op;
+            std::string data;
+            std::vector<swss::FieldValueTuple> values;
 
-            const std::string &op = kfvOp(kco);
-            const std::string &key = kfvKey(kco);
-            const std::vector<swss::FieldValueTuple> &values = kfvFieldsValues(kco);
+            g_redisNotifications->pop(op, data, values);
 
-            SWSS_LOG_DEBUG("notification: op = %s, key = %s", op.c_str(), key.c_str());
+            SWSS_LOG_DEBUG("notification: op = %s, data = %s", op.c_str(), data.c_str());
 
-            if (op != "ntf")
-                continue;
-
-            const std::string &ntf = key.substr(0, key.find_first_of(":"));
-            const std::string &data = key.substr(key.find_last_of(":") + 1);
-
-            handle_notification(ntf, data, values);
+            handle_notification(op, data, values);
         }
     }
 }
@@ -61,7 +67,7 @@ sai_status_t notify_syncd(const std::string &op)
 
     std::vector<swss::FieldValueTuple> entry;
 
-    g_notifySyncdProducer->set("", entry, op);
+    g_notifySyncdProducer->send(op, "", entry);
 
     swss::Select s;
 
@@ -79,9 +85,13 @@ sai_status_t notify_syncd(const std::string &op)
     {
         swss::KeyOpFieldsValuesTuple kco;
 
-        g_notifySyncdConsumer->pop(kco);
+        std::string op;
+        std::string data;
+        std::vector<swss::FieldValueTuple> values;
 
-        const std::string &strStatus = kfvOp(kco);
+        g_notifySyncdConsumer->pop(op, data, values);
+
+        const std::string &strStatus = op;
 
         sai_status_t status;
 
@@ -134,7 +144,7 @@ sai_status_t redis_initialize_switch(
 
     SWSS_LOG_INFO("operation: '%s'", op.c_str());
 
-    if (op == NOTIFY_COMPILE || op == NOTIFY_SWITCH)
+    if (op == NOTIFY_SAI_COMPILE_VIEW || op == NOTIFY_SAI_SWITCH_VIEW)
     {
         sai_status_t status = notify_syncd(op);
 
@@ -219,6 +229,9 @@ void  redis_shutdown_switch(
 
     g_run = false;
 
+    // notify thread that it should end
+    g_redisNotificationTrheadEvent.notify();
+
     notification_thread->join();
 
     g_switchInitialized = false;