Add map of database ID -> table name separator; No longer pass table name separator when constructing Table objects

common/consumertablebase.cpp

@@ -3,7 +3,7 @@
 namespace swss {
 
 ConsumerTableBase::ConsumerTableBase(DBConnector *db, std::string tableName, int popBatchSize):
-        TableConsumable(tableName),
+        TableConsumable(db->getDbId(), tableName),
         RedisTransactioner(db),
         POP_BATCH_SIZE(popBatchSize)
 {

common/dbconnector.cpp

@@ -17,7 +17,7 @@ constexpr const char *DBConnector::DEFAULT_UNIXSOCKET;
 void DBConnector::select(DBConnector *db)
 {
     string select("SELECT ");
-    select += to_string(db->getDB());
+    select += to_string(db->getDbId());
 
     RedisReply r(db, select, REDIS_REPLY_STATUS);
     r.checkStatusOK();
@@ -28,9 +28,9 @@ DBConnector::~DBConnector()
     redisFree(m_conn);
 }
 
-DBConnector::DBConnector(int db, string hostname, int port,
+DBConnector::DBConnector(int dbId, string hostname, int port,
                          unsigned int timeout) :
-    m_db(db)
+    m_id(dbId)
 {
     struct timeval tv = {0, (suseconds_t)timeout * 1000};
 
@@ -46,8 +46,8 @@ DBConnector::DBConnector(int db, string hostname, int port,
     select(this);
 }
 
-DBConnector::DBConnector(int db, string unixPath, unsigned int timeout) :
-    m_db(db)
+DBConnector::DBConnector(int dbId, string unixPath, unsigned int timeout) :
+    m_id(dbId)
 {
     struct timeval tv = {0, (suseconds_t)timeout * 1000};
 
@@ -68,20 +68,20 @@ redisContext *DBConnector::getContext()
     return m_conn;
 }
 
-int DBConnector::getDB()
+int DBConnector::getDbId()
 {
-    return m_db;
+    return m_id;
 }
 
 DBConnector *DBConnector::newConnector(unsigned int timeout)
 {
     if (getContext()->connection_type == REDIS_CONN_TCP)
-        return new DBConnector(getDB(),
+        return new DBConnector(getDbId(),
                                getContext()->tcp.host,
                                getContext()->tcp.port,
                                timeout);
     else
-        return new DBConnector(getDB(),
+        return new DBConnector(getDbId(),
                                getContext()->unix_sock.path,
                                timeout);
 }

common/dbconnector.h

@@ -20,13 +20,13 @@ class DBConnector
      * Timeout - The time in milisecond until exception is been thrown. For
      *           infinite wait, set this value to 0
      */
-    DBConnector(int db, std::string hostname, int port, unsigned int timeout);
-    DBConnector(int db, std::string unixPath, unsigned int timeout);
+    DBConnector(int dbId, std::string hostname, int port, unsigned int timeout);
+    DBConnector(int dbId, std::string unixPath, unsigned int timeout);
 
     ~DBConnector();
 
     redisContext *getContext();
-    int getDB();
+    int getDbId();
 
     static void select(DBConnector *db);
 
@@ -35,7 +35,7 @@ class DBConnector
 
 private:
     redisContext *m_conn;
-    int m_db;
+    int m_id;
 };
 
 }

common/notificationconsumer.cpp

@@ -40,12 +40,12 @@ void swss::NotificationConsumer::subscribe()
 
     /* Create new new context to DB */
     if (m_db->getContext()->connection_type == REDIS_CONN_TCP)
-        m_subscribe = new DBConnector(m_db->getDB(),
+        m_subscribe = new DBConnector(m_db->getDbId(),
                                       m_db->getContext()->tcp.host,
                                       m_db->getContext()->tcp.port,
                                       NOTIFICATION_SUBSCRIBE_TIMEOUT);
     else
-        m_subscribe = new DBConnector(m_db->getDB(),
+        m_subscribe = new DBConnector(m_db->getDbId(),
                                       m_db->getContext()->unix_sock.path,
                                       NOTIFICATION_SUBSCRIBE_TIMEOUT);
 

common/producerstatetable.cpp

@@ -20,7 +20,7 @@ ProducerStateTable::ProducerStateTable(DBConnector *db, string tableName)
 }
 
 ProducerStateTable::ProducerStateTable(RedisPipeline *pipeline, string tableName, bool buffered)
-    : TableBase(tableName)
+    : TableBase(pipeline->getDbId(), tableName)
     , TableName_KeySet(tableName)
     , m_buffered(buffered)
     , m_pipeowned(false)

common/producertable.cpp

@@ -19,7 +19,7 @@ ProducerTable::ProducerTable(DBConnector *db, string tableName)
 }
 
 ProducerTable::ProducerTable(RedisPipeline *pipeline, string tableName, bool buffered)
-    : TableBase(tableName)
+    : TableBase(pipeline->getDbId(), tableName)
     , TableName_KeyValueOpQueues(tableName)
     , m_buffered(buffered)
     , m_pipeowned(false)

common/redispipeline.h

@@ -92,6 +92,11 @@ class RedisPipeline {
         return m_remaining;
     }
 
+    int getDbId()
+    {
+        return m_db->getDbId();
+    }
+
 private:
     DBConnector *m_db;
     std::queue<int> m_expectedTypes;

common/subscriberstatetable.cpp

@@ -15,11 +15,11 @@ using namespace std;
 namespace swss {
 
 SubscriberStateTable::SubscriberStateTable(DBConnector *db, string tableName)
-    : ConsumerTableBase(db, tableName), m_table(db, tableName, CONFIGDB_TABLE_NAME_SEPARATOR)
+    : ConsumerTableBase(db, tableName), m_table(db, tableName)
 {
     m_keyspace = "__keyspace@";
 
-    m_keyspace += to_string(db->getDB()) + "__:" + tableName + CONFIGDB_TABLE_NAME_SEPARATOR + "*";
+    m_keyspace += to_string(db->getDbId()) + "__:" + tableName + m_table.getTableNameSeparator() + "*";
 
     psubscribe(m_db, m_keyspace);
 
@@ -134,7 +134,7 @@ void SubscriberStateTable::pops(deque<KeyOpFieldsValuesTuple> &vkco, string /*pr
         }
 
         string table_entry = msg.substr(pos + 1);
-        pos = table_entry.find(CONFIGDB_TABLE_NAME_SEPARATOR);
+        pos = table_entry.find(m_table.getTableNameSeparator());
         if (pos == table_entry.npos)
         {
             SWSS_LOG_ERROR("invalid key %s returned for pmessage of %s", ctx->str, m_keyspace.c_str());

common/table.cpp

@@ -12,14 +12,28 @@ using namespace std;
 using namespace swss;
 using json = nlohmann::json;
 
-Table::Table(DBConnector *db, string tableName, string tableSeparator)
-    : Table(new RedisPipeline(db, 1), tableName, tableSeparator, false)
+const std::string TableBase::LEGACY_TABLE_NAME_SEPARATOR = ":";
+const std::string TableBase::NEW_TABLE_NAME_SEPARATOR = "|";
+
+const TableNameSeparatorMap TableBase::tableNameSeparatorMap = {
+   { APPL_DB,         LEGACY_TABLE_NAME_SEPARATOR },
+   { ASIC_DB,         LEGACY_TABLE_NAME_SEPARATOR },
+   { COUNTERS_DB,     LEGACY_TABLE_NAME_SEPARATOR },
+   { LOGLEVEL_DB,     LEGACY_TABLE_NAME_SEPARATOR },
+   { CONFIG_DB,       NEW_TABLE_NAME_SEPARATOR },
+   { PFC_WD_DB,       LEGACY_TABLE_NAME_SEPARATOR },
+   { FLEX_COUNTER_DB, LEGACY_TABLE_NAME_SEPARATOR },
+   { STATE_DB,        NEW_TABLE_NAME_SEPARATOR }
+};
+
+Table::Table(DBConnector *db, string tableName)
+    : Table(new RedisPipeline(db, 1), tableName, false)
 {
     m_pipeowned = true;
 }
 
-Table::Table(RedisPipeline *pipeline, string tableName, string tableSeparator, bool buffered)
-    : TableBase(tableName, tableSeparator)
+Table::Table(RedisPipeline *pipeline, string tableName, bool buffered)
+    : TableBase(pipeline->getDbId(), tableName)
     , m_buffered(buffered)
     , m_pipeowned(false)
     , m_pipe(pipeline)

common/table.h

@@ -18,8 +18,8 @@
 
 namespace swss {
 
-#define DEFAULT_TABLE_NAME_SEPARATOR    ":"
-#define CONFIGDB_TABLE_NAME_SEPARATOR   "|"
+// Mapping of DB ID to table name separator string
+typedef std::map<int, std::string> TableNameSeparatorMap;
 
 typedef std::pair<std::string, std::string> FieldValueTuple;
 #define fvField std::get<0>
@@ -34,17 +34,26 @@ typedef std::map<std::string,TableMap> TableDump;
 
 class TableBase {
 public:
-    TableBase(std::string tableName, std::string tableSeparator = DEFAULT_TABLE_NAME_SEPARATOR)
-        : m_tableName(tableName), m_tableSeparator(tableSeparator)
+    TableBase(int dbId, std::string tableName)
+        : m_tableName(tableName)
     {
-        const std::string legalSeparators = ":|";
-        if (legalSeparators.find(tableSeparator) == std::string::npos)
-            throw std::invalid_argument("Invalid table name separator");
+        /* Look up table separator for the provided DB */
+        auto it = tableNameSeparatorMap.find(dbId);
+
+        if (it != tableNameSeparatorMap.end())
+        {
+            m_tableSeparator = it->second;
+        }
+        else
+        {
+            SWSS_LOG_NOTICE("Unrecognized database ID. Using default table name separator ('%s')", NEW_TABLE_NAME_SEPARATOR.c_str());
+            m_tableSeparator = NEW_TABLE_NAME_SEPARATOR;
+        }
     }
 
     std::string getTableName() const { return m_tableName; }
 
-    /* Return the actual key name as a comibation of tableName:key */
+    /* Return the actual key name as a combination of tableName<table_separator>key */
     std::string getKeyName(std::string key)
     {
         if (key == "") return m_tableName;
@@ -59,6 +68,10 @@ class TableBase {
 
     std::string getChannelName() { return m_tableName + "_CHANNEL"; }
 private:
+    static const std::string LEGACY_TABLE_NAME_SEPARATOR;
+    static const std::string NEW_TABLE_NAME_SEPARATOR;
+    static const TableNameSeparatorMap tableNameSeparatorMap;
+
     std::string m_tableName;
     std::string m_tableSeparator;
 };
@@ -95,7 +108,7 @@ class TableConsumable : public TableBase, public TableEntryPoppable, public Redi
     /* The default value of pop batch size is 128 */
     static constexpr int DEFAULT_POP_BATCH_SIZE = 128;
 
-    TableConsumable(std::string tableName) : TableBase(tableName) { }
+    TableConsumable(int dbId, std::string tableName) : TableBase(dbId, tableName) { }
 };
 
 class TableEntryEnumerable {
@@ -115,8 +128,8 @@ class TableEntryEnumerable {
 
 class Table : public TableBase, public TableEntryEnumerable {
 public:
-    Table(DBConnector *db, std::string tableName, std::string tableSeparator = DEFAULT_TABLE_NAME_SEPARATOR);
-    Table(RedisPipeline *pipeline, std::string tableName, std::string tableSeparator, bool buffered);
+    Table(DBConnector *db, std::string tableName);
+    Table(RedisPipeline *pipeline, std::string tableName, bool buffered);
     virtual ~Table();
 
     /* Set an entry in the DB directly (op not in use) */

tests/redis_piped_ut.cpp

@@ -327,7 +327,7 @@ TEST(Table, piped_test)
     string tableName = "TABLE_UT_TEST";
     DBConnector db(TEST_VIEW, "localhost", 6379, 0);
     RedisPipeline pipeline(&db);
-    Table t(&pipeline, tableName, DEFAULT_TABLE_NAME_SEPARATOR, true);
+    Table t(&pipeline, tableName, true);
 
     clearDB();
     cout << "Starting table manipulations" << endl;

tests/redis_subscriber_state_ut.cpp

@@ -95,7 +95,7 @@ static inline void clearDB()
 static void producerWorker(int index)
 {
     DBConnector db(TEST_VIEW, dbhost, dbport, 0);
-    Table p(&db, testTableName, CONFIGDB_TABLE_NAME_SEPARATOR);
+    Table p(&db, testTableName);
 
     for (int i = 0; i < NUMBER_OF_OPS; i++)
     {
@@ -179,7 +179,7 @@ TEST(SubscriberStateTable, set)
     /* Prepare producer */
     int index = 0;
     DBConnector db(TEST_VIEW, dbhost, dbport, 0);
-    Table p(&db, testTableName, CONFIGDB_TABLE_NAME_SEPARATOR);
+    Table p(&db, testTableName);
     string key = "TheKey";
     int maxNumOfFields = 2;
 
@@ -234,7 +234,7 @@ TEST(SubscriberStateTable, del)
     /* Prepare producer */
     int index = 0;
     DBConnector db(TEST_VIEW, dbhost, dbport, 0);
-    Table p(&db, testTableName, CONFIGDB_TABLE_NAME_SEPARATOR);
+    Table p(&db, testTableName);
     string key = "TheKey";
     int maxNumOfFields = 2;
 
@@ -287,7 +287,7 @@ TEST(SubscriberStateTable, table_state)
     /* Prepare producer */
     int index = 0;
     DBConnector db(TEST_VIEW, dbhost, dbport, 0);
-    Table p(&db, testTableName, CONFIGDB_TABLE_NAME_SEPARATOR);
+    Table p(&db, testTableName);
 
     for (int i = 0; i < NUMBER_OF_OPS; i++)
    {