Skip to content

Commit 8213777

Browse files
jlevequejleveque
authored
Add map of database ID -> table name separator; No longer pass table name separator when constructing Table objects (#190)
* Also move TEST_DB to index 15, the largest index supported by default Redis config
1 parent e10a745 commit 8213777

14 files changed

Lines changed: 97 additions & 62 deletions

‎common/consumertablebase.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
namespace swss {
44

55
ConsumerTableBase::ConsumerTableBase(DBConnector *db, std::string tableName, int popBatchSize):
6-
TableConsumable(tableName),
6+
TableConsumable(db->getDbId(), tableName),
77
RedisTransactioner(db),
88
POP_BATCH_SIZE(popBatchSize)
99
{

‎common/dbconnector.cpp‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ constexpr const char *DBConnector::DEFAULT_UNIXSOCKET;
1717
void DBConnector::select(DBConnector *db)
1818
{
1919
string select("SELECT ");
20-
select += to_string(db->getDB());
20+
select += to_string(db->getDbId());
2121

2222
RedisReply r(db, select, REDIS_REPLY_STATUS);
2323
r.checkStatusOK();
@@ -28,9 +28,9 @@ DBConnector::~DBConnector()
2828
redisFree(m_conn);
2929
}
3030

31-
DBConnector::DBConnector(int db, string hostname, int port,
31+
DBConnector::DBConnector(int dbId, string hostname, int port,
3232
unsigned int timeout) :
33-
m_db(db)
33+
m_dbId(dbId)
3434
{
3535
struct timeval tv = {0, (suseconds_t)timeout * 1000};
3636

@@ -46,8 +46,8 @@ DBConnector::DBConnector(int db, string hostname, int port,
4646
select(this);
4747
}
4848

49-
DBConnector::DBConnector(int db, string unixPath, unsigned int timeout) :
50-
m_db(db)
49+
DBConnector::DBConnector(int dbId, string unixPath, unsigned int timeout) :
50+
m_dbId(dbId)
5151
{
5252
struct timeval tv = {0, (suseconds_t)timeout * 1000};
5353

@@ -68,20 +68,20 @@ redisContext *DBConnector::getContext()
6868
return m_conn;
6969
}
7070

71-
int DBConnector::getDB()
71+
int DBConnector::getDbId()
7272
{
73-
return m_db;
73+
return m_dbId;
7474
}
7575

7676
DBConnector *DBConnector::newConnector(unsigned int timeout)
7777
{
7878
if (getContext()->connection_type == REDIS_CONN_TCP)
79-
return new DBConnector(getDB(),
79+
return new DBConnector(getDbId(),
8080
getContext()->tcp.host,
8181
getContext()->tcp.port,
8282
timeout);
8383
else
84-
return new DBConnector(getDB(),
84+
return new DBConnector(getDbId(),
8585
getContext()->unix_sock.path,
8686
timeout);
8787
}

‎common/dbconnector.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,13 @@ class DBConnector
2020
* Timeout - The time in milisecond until exception is been thrown. For
2121
* infinite wait, set this value to 0
2222
*/
23-
DBConnector(int db, std::string hostname, int port, unsigned int timeout);
24-
DBConnector(int db, std::string unixPath, unsigned int timeout);
23+
DBConnector(int dbId, std::string hostname, int port, unsigned int timeout);
24+
DBConnector(int dbId, std::string unixPath, unsigned int timeout);
2525

2626
~DBConnector();
2727

2828
redisContext *getContext();
29-
int getDB();
29+
int getDbId();
3030

3131
static void select(DBConnector *db);
3232

@@ -35,7 +35,7 @@ class DBConnector
3535

3636
private:
3737
redisContext *m_conn;
38-
int m_db;
38+
int m_dbId;
3939
};
4040

4141
}

‎common/notificationconsumer.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,12 @@ void swss::NotificationConsumer::subscribe()
4040

4141
/* Create new new context to DB */
4242
if (m_db->getContext()->connection_type == REDIS_CONN_TCP)
43-
m_subscribe = new DBConnector(m_db->getDB(),
43+
m_subscribe = new DBConnector(m_db->getDbId(),
4444
m_db->getContext()->tcp.host,
4545
m_db->getContext()->tcp.port,
4646
NOTIFICATION_SUBSCRIBE_TIMEOUT);
4747
else
48-
m_subscribe = new DBConnector(m_db->getDB(),
48+
m_subscribe = new DBConnector(m_db->getDbId(),
4949
m_db->getContext()->unix_sock.path,
5050
NOTIFICATION_SUBSCRIBE_TIMEOUT);
5151

‎common/producerstatetable.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ ProducerStateTable::ProducerStateTable(DBConnector *db, string tableName)
2020
}
2121

2222
ProducerStateTable::ProducerStateTable(RedisPipeline *pipeline, string tableName, bool buffered)
23-
: TableBase(tableName)
23+
: TableBase(pipeline->getDbId(), tableName)
2424
, TableName_KeySet(tableName)
2525
, m_buffered(buffered)
2626
, m_pipeowned(false)

‎common/producertable.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ ProducerTable::ProducerTable(DBConnector *db, string tableName)
1919
}
2020

2121
ProducerTable::ProducerTable(RedisPipeline *pipeline, string tableName, bool buffered)
22-
: TableBase(tableName)
22+
: TableBase(pipeline->getDbId(), tableName)
2323
, TableName_KeyValueOpQueues(tableName)
2424
, m_buffered(buffered)
2525
, m_pipeowned(false)

‎common/redispipeline.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,11 @@ class RedisPipeline {
9292
return m_remaining;
9393
}
9494

95+
int getDbId()
96+
{
97+
return m_db->getDbId();
98+
}
99+
95100
private:
96101
DBConnector *m_db;
97102
std::queue<int> m_expectedTypes;

‎common/subscriberstatetable.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@ using namespace std;
1515
namespace swss {
1616

1717
SubscriberStateTable::SubscriberStateTable(DBConnector *db, string tableName)
18-
: ConsumerTableBase(db, tableName), m_table(db, tableName, CONFIGDB_TABLE_NAME_SEPARATOR)
18+
: ConsumerTableBase(db, tableName), m_table(db, tableName)
1919
{
2020
m_keyspace = "__keyspace@";
2121

22-
m_keyspace += to_string(db->getDB()) + "__:" + tableName + CONFIGDB_TABLE_NAME_SEPARATOR + "*";
22+
m_keyspace += to_string(db->getDbId()) + "__:" + tableName + m_table.getTableNameSeparator() + "*";
2323

2424
psubscribe(m_db, m_keyspace);
2525

@@ -134,7 +134,7 @@ void SubscriberStateTable::pops(deque<KeyOpFieldsValuesTuple> &vkco, string /*pr
134134
}
135135

136136
string table_entry = msg.substr(pos + 1);
137-
pos = table_entry.find(CONFIGDB_TABLE_NAME_SEPARATOR);
137+
pos = table_entry.find(m_table.getTableNameSeparator());
138138
if (pos == table_entry.npos)
139139
{
140140
SWSS_LOG_ERROR("invalid key %s returned for pmessage of %s", ctx->str, m_keyspace.c_str());

‎common/table.cpp‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,31 @@ using namespace std;
1212
using namespace swss;
1313
using json = nlohmann::json;
1414

15-
Table::Table(DBConnector *db, string tableName, string tableSeparator)
16-
: Table(new RedisPipeline(db, 1), tableName, tableSeparator, false)
15+
// NOTE: Vertical bar ('|') is the new standard for table name separator
16+
// moving forward. We plan to eventually deprecate the colon separator
17+
// and transition all databases to use the vertical bar.
18+
const std::string TableBase::TABLE_NAME_SEPARATOR_COLON = ":";
19+
const std::string TableBase::TABLE_NAME_SEPARATOR_VBAR = "|";
20+
21+
const TableNameSeparatorMap TableBase::tableNameSeparatorMap = {
22+
{ APPL_DB, TABLE_NAME_SEPARATOR_COLON },
23+
{ ASIC_DB, TABLE_NAME_SEPARATOR_COLON },
24+
{ COUNTERS_DB, TABLE_NAME_SEPARATOR_COLON },
25+
{ LOGLEVEL_DB, TABLE_NAME_SEPARATOR_COLON },
26+
{ CONFIG_DB, TABLE_NAME_SEPARATOR_VBAR },
27+
{ PFC_WD_DB, TABLE_NAME_SEPARATOR_COLON },
28+
{ FLEX_COUNTER_DB, TABLE_NAME_SEPARATOR_COLON },
29+
{ STATE_DB, TABLE_NAME_SEPARATOR_VBAR }
30+
};
31+
32+
Table::Table(DBConnector *db, string tableName)
33+
: Table(new RedisPipeline(db, 1), tableName, false)
1734
{
1835
m_pipeowned = true;
1936
}
2037

21-
Table::Table(RedisPipeline *pipeline, string tableName, string tableSeparator, bool buffered)
22-
: TableBase(tableName, tableSeparator)
38+
Table::Table(RedisPipeline *pipeline, string tableName, bool buffered)
39+
: TableBase(pipeline->getDbId(), tableName)
2340
, m_buffered(buffered)
2441
, m_pipeowned(false)
2542
, m_pipe(pipeline)

‎common/table.h‎

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@
1818

1919
namespace swss {
2020

21-
#define DEFAULT_TABLE_NAME_SEPARATOR ":"
22-
#define CONFIGDB_TABLE_NAME_SEPARATOR "|"
21+
// Mapping of DB ID to table name separator string
22+
typedef std::map<int, std::string> TableNameSeparatorMap;
2323

2424
typedef std::pair<std::string, std::string> FieldValueTuple;
2525
#define fvField std::get<0>
@@ -34,17 +34,26 @@ typedef std::map<std::string,TableMap> TableDump;
3434

3535
class TableBase {
3636
public:
37-
TableBase(std::string tableName, std::string tableSeparator = DEFAULT_TABLE_NAME_SEPARATOR)
38-
: m_tableName(tableName), m_tableSeparator(tableSeparator)
37+
TableBase(int dbId, std::string tableName)
38+
: m_tableName(tableName)
3939
{
40-
const std::string legalSeparators = ":|";
41-
if (legalSeparators.find(tableSeparator) == std::string::npos)
42-
throw std::invalid_argument("Invalid table name separator");
40+
/* Look up table separator for the provided DB */
41+
auto it = tableNameSeparatorMap.find(dbId);
42+
43+
if (it != tableNameSeparatorMap.end())
44+
{
45+
m_tableSeparator = it->second;
46+
}
47+
else
48+
{
49+
SWSS_LOG_NOTICE("Unrecognized database ID. Using default table name separator ('%s')", TABLE_NAME_SEPARATOR_VBAR.c_str());
50+
m_tableSeparator = TABLE_NAME_SEPARATOR_VBAR;
51+
}
4352
}
4453

4554
std::string getTableName() const { return m_tableName; }
4655

47-
/* Return the actual key name as a comibation of tableName:key */
56+
/* Return the actual key name as a combination of tableName<table_separator>key */
4857
std::string getKeyName(std::string key)
4958
{
5059
if (key == "") return m_tableName;
@@ -59,6 +68,10 @@ class TableBase {
5968

6069
std::string getChannelName() { return m_tableName + "_CHANNEL"; }
6170
private:
71+
static const std::string TABLE_NAME_SEPARATOR_COLON;
72+
static const std::string TABLE_NAME_SEPARATOR_VBAR;
73+
static const TableNameSeparatorMap tableNameSeparatorMap;
74+
6275
std::string m_tableName;
6376
std::string m_tableSeparator;
6477
};
@@ -95,7 +108,7 @@ class TableConsumable : public TableBase, public TableEntryPoppable, public Redi
95108
/* The default value of pop batch size is 128 */
96109
static constexpr int DEFAULT_POP_BATCH_SIZE = 128;
97110

98-
TableConsumable(std::string tableName) : TableBase(tableName) { }
111+
TableConsumable(int dbId, std::string tableName) : TableBase(dbId, tableName) { }
99112
};
100113

101114
class TableEntryEnumerable {
@@ -115,8 +128,8 @@ class TableEntryEnumerable {
115128

116129
class Table : public TableBase, public TableEntryEnumerable {
117130
public:
118-
Table(DBConnector *db, std::string tableName, std::string tableSeparator = DEFAULT_TABLE_NAME_SEPARATOR);
119-
Table(RedisPipeline *pipeline, std::string tableName, std::string tableSeparator, bool buffered);
131+
Table(DBConnector *db, std::string tableName);
132+
Table(RedisPipeline *pipeline, std::string tableName, bool buffered);
120133
virtual ~Table();
121134

122135
/* Set an entry in the DB directly (op not in use) */

0 commit comments

Comments
 (0)