Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[dbconnect]: Support DPU database schema #845

Merged
merged 11 commits into from
Jan 27, 2024
+625 −135
Merged

[dbconnect]: Support DPU database schema #845

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f74928d
support DPU database schema
Pterosaur Dec 28, 2023
f4fa7ce
Add unittest
Pterosaur Dec 29, 2023
9a2424d
Fix comment
Pterosaur Jan 12, 2024
cd71777
Merge branch 'master' into smart_switch_db
Pterosaur Jan 12, 2024
4fe98b0
Rename schema field
Pterosaur Jan 12, 2024
57d09d3
Fix ut
Pterosaur Jan 15, 2024
bd95490
Fix comment
Pterosaur Jan 18, 2024
a9a2602
Merge branch 'master' into smart_switch_db
Pterosaur Jan 18, 2024
bf7af35
Merge branch 'master' into smart_switch_db
Pterosaur Jan 19, 2024
bb4f1df
Fix comment
Pterosaur Jan 24, 2024
ba372b0
Merge branch 'master' into smart_switch_db
qiluo-msft Jan 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
251 changes: 165 additions & 86 deletions common/dbconnector.cpp
View file
Open in desktop

Large diffs are not rendered by default.

96 changes: 78 additions & 18 deletions common/dbconnector.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
#include <map>
#include <memory>
#include <mutex>
#include <boost/functional/hash.hpp>
#include <boost/algorithm/string.hpp>

#include <hiredis/hiredis.h>
#include "rediscommand.h"
Expand Down Expand Up @@ -35,6 +37,52 @@ class SonicDBInfo
std::string separator;
};

struct SonicDBKey
{
std::string containerName;
r12f marked this conversation as resolved.
Show resolved Hide resolved
std::string netns;

SonicDBKey() = default;
SonicDBKey(const std::string &ns) : netns(ns) {}

bool operator==(const SonicDBKey& other) const
{
return containerName == other.containerName && netns == other.netns;
}

bool isEmpty() const
{
return containerName.empty() && netns.empty();
}

std::string toString() const
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

toString

If the name is flexible, use to_string instead? #WontFix

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it is a tradeoff, align to the STL convertion or align to this file's convertion .
So, I choose to align to this file.

{
std::vector<std::string> buffer;
if (!containerName.empty())
{
buffer.push_back(containerName);
}
if (!netns.empty())
{
buffer.push_back(netns);
}
return boost::algorithm::join(buffer, ":");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

join

If one has only containerName, and another has only netns (the same string), they will have the same toString. Is it going to be problematic? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's a problem. Fixed it.

}
};

struct SonicDBKeyHash
{
std::size_t operator()(const SonicDBKey& k) const
{
std::size_t seed = 0;
boost::hash_combine(seed, k.containerName);
boost::hash_combine(seed, k.netns);
return seed;
}
};

extern const SonicDBKey EMPTY_SONIC_DB_KEY;

class SonicDBConfig
{
public:
Expand Down Expand Up @@ -62,14 +110,21 @@ class SonicDBConfig
static void reset();

static void validateNamespace(const std::string &netns);
static std::string getDbInst(const std::string &dbName, const std::string &netns = EMPTY_NAMESPACE);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

netns

Is it easier just to add one more parameter containerName=DEFAULT_CONTAINERNAME to each function related ? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't feel that adding more parameters is better. Because I have introduced a new class SonicDBKey, I believe we should prefer to use this new class's object to identify a database instance instead of the old style API in the following code. This function is just for adapting the existing reference.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The suggested function is like

    static std::string getDbInst(const std::string &dbName, const std::string &netns = EMPTY_NAMESPACE, containerName=DEFAULT_CONTAINERNAME);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

static int getDbId(const std::string &dbName, const std::string &netns = EMPTY_NAMESPACE);
static std::string getSeparator(const std::string &dbName, const std::string &netns = EMPTY_NAMESPACE);
static std::string getSeparator(int dbId, const std::string &netns = EMPTY_NAMESPACE);
static std::string getDbInst(const std::string &dbName, const std::string &netns);
static std::string getDbInst(const std::string &dbName, const SonicDBKey &key = EMPTY_SONIC_DB_KEY);
static int getDbId(const std::string &dbName, const std::string &netns);
static int getDbId(const std::string &dbName, const SonicDBKey &key = EMPTY_SONIC_DB_KEY);
static std::string getSeparator(const std::string &dbName, const std::string &netns);
static std::string getSeparator(const std::string &dbName, const SonicDBKey &key = EMPTY_SONIC_DB_KEY);
static std::string getSeparator(int dbId, const std::string &netns);
static std::string getSeparator(int dbId, const SonicDBKey &key = EMPTY_SONIC_DB_KEY);
static std::string getSeparator(const DBConnector* db);
static std::string getDbSock(const std::string &dbName, const std::string &netns = EMPTY_NAMESPACE);
static std::string getDbHostname(const std::string &dbName, const std::string &netns = EMPTY_NAMESPACE);
static int getDbPort(const std::string &dbName, const std::string &netns = EMPTY_NAMESPACE);
static std::string getDbSock(const std::string &dbName, const std::string &netns);
static std::string getDbSock(const std::string &dbName, const SonicDBKey &key = EMPTY_SONIC_DB_KEY);
static std::string getDbHostname(const std::string &dbName, const std::string &netns);
static std::string getDbHostname(const std::string &dbName, const SonicDBKey &key = EMPTY_SONIC_DB_KEY);
static int getDbPort(const std::string &dbName, const std::string &netns);
static int getDbPort(const std::string &dbName, const SonicDBKey &key = EMPTY_SONIC_DB_KEY);
static std::vector<std::string> getNamespaces();
#if defined(SWIG) && defined(SWIGPYTHON)
%pythoncode %{
Expand All @@ -80,27 +135,29 @@ class SonicDBConfig
%}
#endif

static std::vector<std::string> getDbList(const std::string &netns = EMPTY_NAMESPACE);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getDbList

the same: add one more parameter containerName=DEFAULT_CONTAINERNAME #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reply as above.

static std::vector<std::string> getDbList(const std::string &netns);
static std::vector<std::string> getDbList(const SonicDBKey &key = EMPTY_SONIC_DB_KEY);
static bool isInit() { return m_init; };
static bool isGlobalInit() { return m_global_init; };
static std::map<std::string, RedisInstInfo> getInstanceList(const std::string &netns = EMPTY_NAMESPACE);
static std::map<std::string, RedisInstInfo> getInstanceList(const std::string &netns);
static std::map<std::string, RedisInstInfo> getInstanceList(const SonicDBKey &key = EMPTY_SONIC_DB_KEY);

private:
static std::recursive_mutex m_db_info_mutex;
// { namespace { instName, { unix_socket_path, hostname, port } } }
static std::unordered_map<std::string, std::map<std::string, RedisInstInfo>> m_inst_info;
// { namespace, { dbName, {instName, dbId, separator} } }
static std::unordered_map<std::string, std::unordered_map<std::string, SonicDBInfo>> m_db_info;
// { namespace, { dbId, separator } }
static std::unordered_map<std::string, std::unordered_map<int, std::string>> m_db_separator;
// { {containerName, namespace}, { instName, { unix_socket_path, hostname, port } } }
static std::unordered_map<SonicDBKey, std::map<std::string, RedisInstInfo>, SonicDBKeyHash> m_inst_info;
// { {containerName, namespace}, { dbName, {instName, dbId, separator} } }
static std::unordered_map<SonicDBKey, std::unordered_map<std::string, SonicDBInfo>, SonicDBKeyHash> m_db_info;
// { {containerName, namespace}, { dbId, separator } }
static std::unordered_map<SonicDBKey, std::unordered_map<int, std::string>, SonicDBKeyHash> m_db_separator;
static bool m_init;
static bool m_global_init;
static void parseDatabaseConfig(const std::string &file,
std::map<std::string, RedisInstInfo> &inst_entry,
std::unordered_map<std::string, SonicDBInfo> &db_entry,
std::unordered_map<int, std::string> &separator_entry);
static SonicDBInfo& getDbInfo(const std::string &dbName, const std::string &netns = EMPTY_NAMESPACE);
static RedisInstInfo& getRedisInfo(const std::string &dbName, const std::string &netns = EMPTY_NAMESPACE);
static RedisInstInfo& getRedisInfo(const std::string &dbName, const SonicDBKey &key);
static SonicDBInfo& getDbInfo(const std::string &dbName, const SonicDBKey &key);
};

class RedisContext
Expand Down Expand Up @@ -158,6 +215,7 @@ class DBConnector : public RedisContext
DBConnector(int dbId, const std::string &unixPath, unsigned int timeout);
DBConnector(const std::string &dbName, unsigned int timeout, bool isTcpConn = false);
DBConnector(const std::string &dbName, unsigned int timeout, bool isTcpConn, const std::string &netns);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

netns

the same: add one more parameter containerName=DEFAULT_CONTAINERNAME #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reply as above.

DBConnector(const std::string &dbName, unsigned int timeout, bool isTcpConn, const SonicDBKey &key);
DBConnector& operator=(const DBConnector&) = delete;

int getDbId() const;
Expand All @@ -169,6 +227,7 @@ class DBConnector : public RedisContext
namespace = property(getNamespace)
%}
#endif
SonicDBKey getDBKey() const;

static void select(DBConnector *db);

Expand Down Expand Up @@ -252,10 +311,11 @@ class DBConnector : public RedisContext
std::map<std::string, std::map<std::string, std::map<std::string, std::string>>> getall();
private:
void setNamespace(const std::string &netns);
void setDBKey(const SonicDBKey &key);

int m_dbId;
std::string m_dbName;
std::string m_namespace;
SonicDBKey m_key;
};

template <typename ReturnType>
Expand Down
1 change: 1 addition & 0 deletions tests/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ tests_tests_SOURCES = tests/redis_ut.cpp \
common/loglevel.cpp \
tests/loglevel_ut.cpp \
tests/redis_multi_ns_ut.cpp \
tests/redis_smartswitch_ut.cpp \
tests/fdb_flush.cpp \
tests/stringutility_ut.cpp \
tests/redisutility_ut.cpp \
Expand Down
2 changes: 1 addition & 1 deletion tests/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class SwsscommonEnvironment : public ::testing::Environment {
}
catch (exception &e)
{
EXPECT_TRUE(strstr(e.what(), "Namespace invalid is not a valid namespace name in config file"));
EXPECT_TRUE(strstr(e.what(), "Key invalid is not a valid key name in config file"));
}

// reset SonicDBConfig, init should be false
Expand Down
113 changes: 113 additions & 0 deletions tests/redis_multi_db_ut_config/database_config4.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
{
"INSTANCES": {
"redis":{
"hostname" : "127.0.0.1",
"port": 6379,
"unix_socket_path": "/var/run/redis/redis.sock"
}
},
"DATABASES" : {
"APPL_DB" : {
"id" : 0,
"separator": ":",
"instance" : "redis"
},
"ASIC_DB" : {
"id" : 1,
"separator": ":",
"instance" : "redis"
},
"COUNTERS_DB" : {
"id" : 2,
"separator": ":",
"instance" : "redis"
},
"LOGLEVEL_DB" : {
"id" : 3,
"separator": ":",
"instance" : "redis"
},
"CONFIG_DB" : {
"id" : 4,
"separator": "|",
"instance" : "redis"
},
"PFC_WD_DB" : {
"id" : 5,
"separator": ":",
"instance" : "redis"
},
"FLEX_COUNTER_DB" : {
"id" : 5,
"separator": ":",
"instance" : "redis"
},
"STATE_DB" : {
"id" : 6,
"separator": "|",
"instance" : "redis"
},
"SNMP_OVERLAY_DB" : {
"id" : 7,
"separator": "|",
"instance" : "redis"
},
"RESTAPI_DB": {
"id": 8,
"separator": "|",
"instance": "redis"
},
"GB_ASIC_DB": {
"id": 9,
"separator": ":",
"instance": "redis"
},
"GB_COUNTERS_DB": {
"id": 10,
"separator": ":",
"instance": "redis"
},
"GB_FLEX_COUNTER_DB": {
"id": 11,
"separator": ":",
"instance": "redis"
},
"PROFILE_DB" : {
"id" : 12,
"separator": "|",
"instance" : "redis"
},
"STATE_DB2" : {
"id" : 13,
"separator": "|",
"instance" : "redis"
},
"APPL_STATE_DB" : {
"id" : 14,
"separator": ":",
"instance" : "redis"
},
"DPU_APPL_DB" : {
"id" : 15,
"separator": ":",
"instance" : "redis",
"format": "proto"
},
"DPU_APPL_STATE_DB" : {
"id" : 16,
"separator": "|",
"instance" : "redis"
},
"DPU_STATE_DB" : {
"id" : 17,
"separator": "|",
"instance" : "redis"
},
"DPU_COUNTERS_DB" : {
"id" : 18,
"separator": ":",
"instance" : "redis"
}
},
"VERSION" : "1.0"
}