[dbconnect]: Support DPU database schema #845

Pterosaur · 2023-12-28T15:11:22Z

This PR is for expanding the capability of dbconnect to adapt the new DPU DB schema introduced by PR: sonic-net/sonic-buildimage#17443

Signed-off-by: Ze Gan <ganze718@gmail.com>

qiluo-msft · 2024-01-11T21:26:32Z

common/dbconnector.h

+        return containerName.empty() && netns.empty();
+    }
+
+    std::string toString() const


toString

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

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.

qiluo-msft · 2024-01-11T21:29:19Z

common/dbconnector.h

+        {
+            buffer.push_back(netns);
+        }
+        return boost::algorithm::join(buffer, ":");


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

Yes, it's a problem. Fixed it.

qiluo-msft · 2024-01-11T21:34:59Z

common/dbconnector.h

@@ -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);


netns

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

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.

The suggested function is like

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

qiluo-msft · 2024-01-11T21:36:08Z

common/dbconnector.h

@@ -80,27 +135,29 @@ class SonicDBConfig
    %}
 #endif

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


getDbList

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

reply as above.

qiluo-msft · 2024-01-11T21:36:29Z

common/dbconnector.h

@@ -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);


netns

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

reply as above.

Signed-off-by: Ze Gan <ganze718@gmail.com>

r12f · 2024-01-11T01:45:41Z

common/dbconnector.cpp

+               string socket;
+               if (it.value().find("unix_socket_path") != it.value().end())
+               {
+                    socket = it.value().at("unix_socket_path");


we should be able to directly get the value from the iterator that returned by find(), according to nlohmann/json code here: https://github.com/nlohmann/json/blob/a259ecc51e1951e12f757ce17db958e9881e9c6c/include/nlohmann/detail/iterators/iter_impl.hpp#L280C21-L280C21.

r12f · 2024-01-11T01:48:36Z

common/dbconnector.cpp

-    m_db_info[EMPTY_NAMESPACE] = db_entry;
-    m_db_separator[EMPTY_NAMESPACE] = separator_entry;
+    m_inst_info[empty_key] = inst_entry;
+    m_db_info[empty_key] = db_entry;


use .emplace(empty_key, std::move(db_entry)) will be faster if it is supported in our dev env.

r12f · 2024-01-11T01:51:12Z

common/dbconnector.cpp


-                if(element["namespace"].empty())
+                if (!element["database_name"].empty())


have we decided the name to be database_name? I remember Qi had concern on this name, since redis also has a database concepts, but not sure if this is closed or not.

if not, maybe database_instance_name or something like this might help with the concern.

It has been changed to container_name, please check it again.

common/dbconnector.h

Signed-off-by: Ze Gan <ganze718@gmail.com>

Pterosaur · 2024-01-20T00:45:10Z

Hi @qiluo-msft Please help to review or approve/merge this PR?

Signed-off-by: Ze Gan <ganze718@gmail.com>

qiluo-msft · 2024-01-25T08:45:00Z

common/dbconnector.cpp

 {
    struct timeval tv = {0, (suseconds_t)timeout * 1000};
    struct timeval *ptv = timeout ? &tv : NULL;
    if (isTcpConn)
    {
-        initContext(SonicDBConfig::getDbHostname(dbName, netns).c_str(), SonicDBConfig::getDbPort(dbName, netns), ptv);
+        initContext(SonicDBConfig::getDbHostname(dbName, m_key).c_str(), SonicDBConfig::getDbPort(dbName, m_key), ptv);


SonicDBConfig

Future TODO: decouple DBConnector with SonicDBConfig. DBConnector only need to know its own dbName/index, port, hostname, unix socket. No need to know any other information such as namespace, containerName, sep, etc.

Pterosaur force-pushed the smart_switch_db branch from 5cd8953 to 3e8c1b5 Compare December 29, 2023 02:07

Pterosaur changed the title ~~support DPU database schema~~ [dbconnect]: Support DPU database schema Dec 29, 2023

support DPU database schema

f74928d

Signed-off-by: Ze Gan <ganze718@gmail.com>

Pterosaur force-pushed the smart_switch_db branch from 3e8c1b5 to f74928d Compare December 29, 2023 02:10

Add unittest

f4fa7ce

Signed-off-by: Ze Gan <ganze718@gmail.com>

Pterosaur marked this pull request as ready for review December 29, 2023 12:36

Pterosaur requested review from liuh-80, qiluo-msft and ganglyu December 29, 2023 12:36

qiluo-msft requested a review from judyjoseph December 29, 2023 23:05

qiluo-msft reviewed Jan 11, 2024

View reviewed changes

Pterosaur added 2 commits January 12, 2024 09:40

Fix comment

9a2424d

Signed-off-by: Ze Gan <ganze718@gmail.com>

Merge branch 'master' into smart_switch_db

cd71777

Pterosaur force-pushed the smart_switch_db branch from 2ebf48e to 314dbf5 Compare January 13, 2024 01:02

Rename schema field

4fe98b0

Signed-off-by: Ze Gan <ganze718@gmail.com>

Pterosaur force-pushed the smart_switch_db branch from 314dbf5 to 4fe98b0 Compare January 15, 2024 03:14

Fix ut

57d09d3

Signed-off-by: Ze Gan <ganze718@gmail.com>

Pterosaur force-pushed the smart_switch_db branch from bb54c55 to 57d09d3 Compare January 15, 2024 05:56

Pterosaur requested a review from qiluo-msft January 15, 2024 10:08

r12f reviewed Jan 18, 2024

View reviewed changes

Fix comment

bd95490

Signed-off-by: Ze Gan <ganze718@gmail.com>

Pterosaur requested a review from r12f January 18, 2024 04:46

Pterosaur added 2 commits January 18, 2024 15:02

Merge branch 'master' into smart_switch_db

a9a2602

Merge branch 'master' into smart_switch_db

bf7af35

Signed-off-by: Ze Gan <ganze718@gmail.com>

Pterosaur force-pushed the smart_switch_db branch from 2d9f885 to bf7af35 Compare January 19, 2024 11:43

Fix comment

bb4f1df

Signed-off-by: Ze Gan <ganze718@gmail.com>

qiluo-msft reviewed Jan 25, 2024

View reviewed changes

qiluo-msft mentioned this pull request Jan 25, 2024

>SonicDBConfig [](http://example.com/codeflow?start=20&length=13) #851

Open

qiluo-msft approved these changes Jan 25, 2024

View reviewed changes

r12f approved these changes Jan 26, 2024

View reviewed changes

Merge branch 'master' into smart_switch_db

ba372b0

qiluo-msft merged commit 41ee154 into sonic-net:master Jan 27, 2024
14 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[dbconnect]: Support DPU database schema #845

[dbconnect]: Support DPU database schema #845

Pterosaur commented Dec 28, 2023 •

edited

qiluo-msft Jan 11, 2024 •

edited

Pterosaur Jan 12, 2024

qiluo-msft Jan 11, 2024 •

edited

Pterosaur Jan 12, 2024

qiluo-msft Jan 11, 2024 •

edited

Pterosaur Jan 12, 2024

qiluo-msft Jan 20, 2024

Pterosaur Jan 24, 2024

qiluo-msft Jan 11, 2024 •

edited

Pterosaur Jan 12, 2024

qiluo-msft Jan 11, 2024 •

edited

Pterosaur Jan 12, 2024

r12f Jan 11, 2024

Pterosaur Jan 18, 2024

r12f Jan 11, 2024

Pterosaur Jan 18, 2024

r12f Jan 11, 2024

Pterosaur Jan 18, 2024

Pterosaur commented Jan 20, 2024

qiluo-msft Jan 25, 2024


		if(element["namespace"].empty())
		if (!element["database_name"].empty())

[dbconnect]: Support DPU database schema #845

[dbconnect]: Support DPU database schema #845

Conversation

Pterosaur commented Dec 28, 2023 • edited

qiluo-msft Jan 11, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jan 11, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jan 11, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jan 11, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jan 11, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Pterosaur commented Jan 20, 2024

Choose a reason for hiding this comment

Pterosaur commented Dec 28, 2023 •

edited

qiluo-msft Jan 11, 2024 •

edited

qiluo-msft Jan 11, 2024 •

edited

qiluo-msft Jan 11, 2024 •

edited

qiluo-msft Jan 11, 2024 •

edited

qiluo-msft Jan 11, 2024 •

edited