diff --git a/common/consumertablebase.cpp b/common/consumertablebase.cpp index 530f4578f..c506ccfd6 100644 --- a/common/consumertablebase.cpp +++ b/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) { diff --git a/common/dbconnector.cpp b/common/dbconnector.cpp index b4216f0ec..3879ba014 100644 --- a/common/dbconnector.cpp +++ b/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_dbId(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_dbId(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_dbId; } 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); } diff --git a/common/dbconnector.h b/common/dbconnector.h index 99a62aeb9..d14f83d67 100644 --- a/common/dbconnector.h +++ b/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_dbId; }; } diff --git a/common/notificationconsumer.cpp b/common/notificationconsumer.cpp index 3bd8694ef..25f49a14f 100644 --- a/common/notificationconsumer.cpp +++ b/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); diff --git a/common/producerstatetable.cpp b/common/producerstatetable.cpp index 9eee190d9..fbd70d167 100644 --- a/common/producerstatetable.cpp +++ b/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) diff --git a/common/producertable.cpp b/common/producertable.cpp index b06402d65..14f0c51bf 100644 --- a/common/producertable.cpp +++ b/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) diff --git a/common/redispipeline.h b/common/redispipeline.h index 88cfd23f0..0180a8c15 100644 --- a/common/redispipeline.h +++ b/common/redispipeline.h @@ -92,6 +92,11 @@ class RedisPipeline { return m_remaining; } + int getDbId() + { + return m_db->getDbId(); + } + private: DBConnector *m_db; std::queue m_expectedTypes; diff --git a/common/subscriberstatetable.cpp b/common/subscriberstatetable.cpp index cdaaa08d5..7d07ce22d 100644 --- a/common/subscriberstatetable.cpp +++ b/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 &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()); diff --git a/common/table.cpp b/common/table.cpp index bdc06e52d..927ec3d8a 100644 --- a/common/table.cpp +++ b/common/table.cpp @@ -12,14 +12,31 @@ 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) +// NOTE: Vertical bar ('|') is the new standard for table name separator +// moving forward. We plan to eventually deprecate the colon separator +// and transition all databases to use the vertical bar. +const std::string TableBase::TABLE_NAME_SEPARATOR_COLON = ":"; +const std::string TableBase::TABLE_NAME_SEPARATOR_VBAR = "|"; + +const TableNameSeparatorMap TableBase::tableNameSeparatorMap = { + { APPL_DB, TABLE_NAME_SEPARATOR_COLON }, + { ASIC_DB, TABLE_NAME_SEPARATOR_COLON }, + { COUNTERS_DB, TABLE_NAME_SEPARATOR_COLON }, + { LOGLEVEL_DB, TABLE_NAME_SEPARATOR_COLON }, + { CONFIG_DB, TABLE_NAME_SEPARATOR_VBAR }, + { PFC_WD_DB, TABLE_NAME_SEPARATOR_COLON }, + { FLEX_COUNTER_DB, TABLE_NAME_SEPARATOR_COLON }, + { STATE_DB, TABLE_NAME_SEPARATOR_VBAR } +}; + +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) diff --git a/common/table.h b/common/table.h index 413406e93..957e37c2a 100644 --- a/common/table.h +++ b/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 TableNameSeparatorMap; typedef std::pair FieldValueTuple; #define fvField std::get<0> @@ -34,17 +34,26 @@ typedef std::map 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')", TABLE_NAME_SEPARATOR_VBAR.c_str()); + m_tableSeparator = TABLE_NAME_SEPARATOR_VBAR; + } } 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 tableNamekey */ 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 TABLE_NAME_SEPARATOR_COLON; + static const std::string TABLE_NAME_SEPARATOR_VBAR; + 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) */ diff --git a/tests/redis_piped_ut.cpp b/tests/redis_piped_ut.cpp index 353a6de9a..1da77d572 100644 --- a/tests/redis_piped_ut.cpp +++ b/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; diff --git a/tests/redis_subscriber_state_ut.cpp b/tests/redis_subscriber_state_ut.cpp index 25d1398b8..61fdb1f86 100644 --- a/tests/redis_subscriber_state_ut.cpp +++ b/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++) { diff --git a/tests/redis_ut.cpp b/tests/redis_ut.cpp index 2ba577952..675b8b369 100644 --- a/tests/redis_ut.cpp +++ b/tests/redis_ut.cpp @@ -16,7 +16,7 @@ using namespace std; using namespace swss; -#define TEST_VIEW (7) +#define TEST_DB (15) // Default Redis config supports 16 databases, max DB ID is 15 #define NUMBER_OF_THREADS (64) // Spawning more than 256 threads causes libc++ to except #define NUMBER_OF_OPS (1000) #define MAX_FIELDS_DIV (30) // Testing up to 30 fields objects @@ -74,7 +74,7 @@ void validateFields(const string& key, const vector& f) void producerWorker(int index) { string tableName = "UT_REDIS_THREAD_" + to_string(index); - DBConnector db(TEST_VIEW, "localhost", 6379, 0); + DBConnector db(TEST_DB, "localhost", 6379, 0); ProducerTable p(&db, tableName); for (int i = 0; i < NUMBER_OF_OPS; i++) @@ -102,7 +102,7 @@ void producerWorker(int index) void consumerWorker(int index) { string tableName = "UT_REDIS_THREAD_" + to_string(index); - DBConnector db(TEST_VIEW, "localhost", 6379, 0); + DBConnector db(TEST_DB, "localhost", 6379, 0); ConsumerTable c(&db, tableName); Select cs; Selectable *selectcs; @@ -138,21 +138,19 @@ void consumerWorker(int index) void clearDB() { - DBConnector db(TEST_VIEW, "localhost", 6379, 0); + DBConnector db(TEST_DB, "localhost", 6379, 0); RedisReply r(&db, "FLUSHALL", REDIS_REPLY_STATUS); r.checkStatusOK(); } -void TableBasicTest(string tableName, string separator) +void TableBasicTest(string tableName) { - DBConnector db(TEST_VIEW, "localhost", 6379, 0); + DBConnector db(TEST_DB, "localhost", 6379, 0); - Table t(&db, tableName, separator); - string tableNameSeparator = t.getTableNameSeparator(); - ASSERT_STREQ(tableNameSeparator.c_str(), separator.c_str()); + Table t(&db, tableName); clearDB(); - cout << "Starting table manipulations, table name separator is " << separator << endl; + cout << "Starting table manipulations" << endl; string key_1 = "a"; string key_2 = "b"; @@ -263,7 +261,7 @@ TEST(DBConnector, test) TEST(DBConnector, multitable) { - DBConnector db(TEST_VIEW, "localhost", 6379, 0); + DBConnector db(TEST_DB, "localhost", 6379, 0); ConsumerTable *consumers[NUMBER_OF_THREADS]; thread *producerThreads[NUMBER_OF_THREADS]; KeyOpFieldsValuesTuple kco; @@ -328,7 +326,7 @@ void notificationProducer() { sleep(1); - DBConnector db(TEST_VIEW, "localhost", 6379, 0); + DBConnector db(TEST_DB, "localhost", 6379, 0); NotificationProducer np(&db, "UT_REDIS_CHANNEL"); vector values; @@ -341,7 +339,7 @@ void notificationProducer() TEST(DBConnector, notifications) { - DBConnector db(TEST_VIEW, "localhost", 6379, 0); + DBConnector db(TEST_DB, "localhost", 6379, 0); NotificationConsumer nc(&db, "UT_REDIS_CHANNEL"); Select s; s.addSelectable(&nc); @@ -451,24 +449,26 @@ TEST(DBConnector, selectabletimer) TEST(Table, basic) { - TableBasicTest("TABLE_UT_TEST", ":"); + TableBasicTest("TABLE_UT_TEST"); } TEST(Table, separator_in_table_name) { - TableBasicTest("TABLE_UT:TEST", ":"); + std::string tableName = "TABLE_UT|TEST"; + + TableBasicTest(tableName); } TEST(Table, table_separator_test) { - TableBasicTest("TABLE_UT_TEST", CONFIGDB_TABLE_NAME_SEPARATOR); + TableBasicTest("TABLE_UT_TEST"); } TEST(ProducerConsumer, Prefix) { std::string tableName = "tableName"; - DBConnector db(TEST_VIEW, "localhost", 6379, 0); + DBConnector db(TEST_DB, "localhost", 6379, 0); ProducerTable p(&db, tableName); std::vector values; @@ -497,7 +497,7 @@ TEST(ProducerConsumer, Pop) { std::string tableName = "tableName"; - DBConnector db(TEST_VIEW, "localhost", 6379, 0); + DBConnector db(TEST_DB, "localhost", 6379, 0); ProducerTable p(&db, tableName); std::vector values; @@ -525,7 +525,7 @@ TEST(ProducerConsumer, Pop2) { std::string tableName = "tableName"; - DBConnector db(TEST_VIEW, "localhost", 6379, 0); + DBConnector db(TEST_DB, "localhost", 6379, 0); ProducerTable p(&db, tableName); std::vector values; @@ -564,7 +564,7 @@ TEST(ProducerConsumer, PopEmpty) { std::string tableName = "tableName"; - DBConnector db(TEST_VIEW, "localhost", 6379, 0); + DBConnector db(TEST_DB, "localhost", 6379, 0); ConsumerTable c(&db, tableName); diff --git a/tests/test_redis_ut.py b/tests/test_redis_ut.py index 3cc36b130..18e122d1a 100644 --- a/tests/test_redis_ut.py +++ b/tests/test_redis_ut.py @@ -28,7 +28,7 @@ def test_Table(): def test_SubscriberStateTable(): db = swsscommon.DBConnector(0, "localhost", 6379, 0) - t = swsscommon.Table(db, "testsst", '|') + t = swsscommon.Table(db, "testsst") sel = swsscommon.Select() cst = swsscommon.SubscriberStateTable(db, "testsst") sel.addSelectable(cst)