Add PublisherEventTable/SubscriberEventTable classes #1020

qiluo-msft · 2025-05-29T23:08:01Z

TBD

mssonicbld · 2025-05-29T23:08:08Z

/azp run

azure-pipelines · 2025-05-29T23:08:19Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Qi Luo <[email protected]>

mssonicbld · 2025-05-29T23:09:28Z

/azp run

azure-pipelines · 2025-05-29T23:09:39Z

Azure Pipelines successfully started running 1 pipeline(s).

Copilot

Pull Request Overview

Adds event-driven pub/sub tables and integrates them into tests and Python bindings

Introduces PublisherEventTable and SubscriberEventTable for JSON-based Redis pub/sub
Exposes new classes in SWIG (swsscommon.i) for Python support
Adds C++ and Python unit tests and updates build scripts to compile new sources

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
common/publishereventtable.h/.cpp	Implements `PublisherEventTable` with JSON message building and publish
common/subscribereventtable.h/.cpp	Implements `SubscriberEventTable` buffering, parsing, and pops APIs
pyext/swsscommon.i	Adds SWIG bindings for the new event table classes
tests/test_redis_ut.py	Adds Python tests for state and event table pub/sub behavior
tests/redis_subscriber_state_ut.cpp	Adds C++ tests for `SubscriberEventTable`
common/Makefile.am	Includes new source files in the build

Comments suppressed due to low confidence (4)

tests/redis_subscriber_state_ut.cpp:227

Test suite name SubscribeEventTable doesn’t match the class SubscriberEventTable; rename for consistency.

TEST(SubscribeEventTable, set)

common/subscribereventtable.cpp:127

No existing unit tests cover the pops() batch API; consider adding tests for multiple queued events to verify correct behavior.

void SubscriberEventTable::pops(deque<KeyOpFieldsValuesTuple> &vkco, const string& /*prefix*/)

tests/test_redis_ut.py:90

Undefined variable t in test_SubscriberStateTable. You should instantiate a PublisherStateTable (or the appropriate table) before calling set.

t.set("aaa", fvs)

pyext/swsscommon.i:301

[nitpick] Duplicate %include "publishereventtable.h"; remove the redundant inclusion to clean up the bindings file.

%include "publishereventtable.h"

tests/test_redis_ut.py

Copilot · 2025-05-29T23:11:28Z

common/subscribereventtable.cpp

+namespace swss {
+
+// TODO: reuse
+#define REDIS_PUBLISH_MESSAGE_ELEMNTS (3)


Typo in macro name REDIS_PUBLISH_MESSAGE_ELEMNTS; consider renaming to REDIS_PUBLISH_MESSAGE_ELEMENTS.

Suggested change

#define REDIS_PUBLISH_MESSAGE_ELEMNTS (3)

#define REDIS_PUBLISH_MESSAGE_ELEMENTS (3)

Copilot · 2025-05-29T23:11:28Z

common/publishereventtable.h

+
+namespace swss {
+
+class PublisherEventTable : public Table { // public TableBase, public TableEntryEnumerable {


[nitpick] Outdated comment // public TableBase, public TableEntryEnumerable; update or remove it to reflect that this class now inherits directly from Table.

common/subscribereventtable.cpp

common/publishereventtable.h

+
+    /* Read a value from the DB directly */
+    /* Returns false if the key doesn't exists */
+    // virtual bool get(const std::string &key, std::vector<FieldValueTuple> &ovalues);


common/publishereventtable.h

+    /* Returns false if the key doesn't exists */
+    // virtual bool get(const std::string &key, std::vector<FieldValueTuple> &ovalues);
+
+    // void getKeys(std::vector<std::string> &keys);


common/publishereventtable.h

+
+    // void getKeys(std::vector<std::string> &keys);
+
+    // void setBuffered(bool buffered);


common/publishereventtable.h

+
+    // void setBuffered(bool buffered);
+
+    // void flush();


common/publishereventtable.h

+
+    // void flush();
+
+    // void dump(TableDump &tableDump);


common/subscribereventtable.cpp

+
+    if (reply->type != REDIS_REPLY_ARRAY)
+    {
+        // SWSS_LOG_ERROR("expected ARRAY redis reply on channel %s, got: %d", m_channel.c_str(), reply->type);


Co-authored-by: Copilot <[email protected]>

mssonicbld · 2025-05-29T23:41:49Z

/azp run

azure-pipelines · 2025-05-29T23:42:01Z

Azure Pipelines successfully started running 1 pipeline(s).

Co-authored-by: Copilot <[email protected]>

mssonicbld · 2025-05-29T23:42:09Z

/azp run

azure-pipelines · 2025-05-29T23:42:20Z

Azure Pipelines successfully started running 1 pipeline(s).

Co-authored-by: Copilot <[email protected]>

mssonicbld · 2025-05-29T23:43:30Z

/azp run

azure-pipelines · 2025-05-29T23:43:42Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-05-29T23:46:03Z

/azp run

azure-pipelines · 2025-05-29T23:46:15Z

Azure Pipelines successfully started running 1 pipeline(s).

confliction in one redis instance across multiple databases

Signed-off-by: Qi Luo <[email protected]>

mssonicbld · 2025-06-07T23:55:28Z

/azp run

azure-pipelines · 2025-06-07T23:55:39Z

Azure Pipelines successfully started running 1 pipeline(s).

common/dbconnector.cpp

+                    for (auto& eventTable: it.value()["event_tables"])
+                    {
+                        eventTables.emplace(eventTable);
+                        // printf("Event table: %s %s\n", dbName.c_str(), eventTable.get<string>().c_str());


common/dbconnector.cpp

+                }
+                else
+                {
+                    // printf("Warning: event_tables not found for database %s, using empty set\n", dbName.c_str());


common/dbconnector.cpp

+           // inst_entry.size(), db_entry.size(), separator_entry.size(), db_entry["CONFIG_DB"].eventTables.size());
    m_inst_info.emplace(empty_key, std::move(inst_entry));
    m_db_info.emplace(empty_key, std::move(db_entry));
+    // printf("m_db_info %lu\n", m_db_info[empty_key]["CONFIG_DB"].eventTables.size());


common/dbconnector.cpp

+    // printf("SonicDBConfig::getEventTables: dbName=%s, netns=%s, containerName=%s, %lu\n",
+    //        dbName.c_str(), netns.c_str(), containerName.c_str(), m_db_info[key][dbName].eventTables.size());
+    auto ret = getEventTables(dbName, key);
+    // printf("SonicDBConfig::getEventTables: %lu\n", ret.size());


common/dbconnector.cpp

+    // printf("SonicDBConfig::getEventTables2: dbName=%s, %lu\n",
+    //        dbName.c_str(), m_db_info[key][dbName].eventTables.size());
+    auto ret = getDbInfo(dbName, key).eventTables;
+    // printf("SonicDBConfig::getEventTables2: %lu\n", ret.size());


tests/redis_subscriber_state_ut.cpp

+    // for (auto &table: eventTables)
+    // {
+    //     cout << "Event table: " << table << endl;
+    // }


bhouse-nexthop · 2025-06-15T14:57:10Z

Can this effort possibly be used to address sonic-net/sonic-buildimage#22255 ?

qiluo-msft added 9 commits May 29, 2025 23:08

(temp)

172c072

Signed-off-by: Qi Luo <[email protected]>

Add testcase in py

031aba1

Fix channel name for PublisherEventTable/SubscriberEventTable

798b86e

Add UT in c++

e15f7f9

Signed-off-by: Qi Luo <[email protected]>

Add testcase: test_PublisherEventTable_SubscriberStateTable

a3f474a

test_SubscriberEventTable: add test for accurate events

f8452f6

test_SubscriberStateTable: add test for final state convergence

57eea2b

(reformat)

b44cde6

(comment)

e306266

qiluo-msft force-pushed the qiluo/pubsubchannel branch from 349bab8 to e306266 Compare May 29, 2025 23:09

qiluo-msft requested a review from Copilot May 29, 2025 23:09

Copilot AI reviewed May 29, 2025

View reviewed changes

github-advanced-security bot found potential problems May 29, 2025

View reviewed changes

Update tests/test_redis_ut.py

712dab7

Co-authored-by: Copilot <[email protected]>

Update tests/test_redis_ut.py

0cdfcc5

Co-authored-by: Copilot <[email protected]>

Update common/subscribereventtable.cpp

f01201d

Co-authored-by: Copilot <[email protected]>

(comment)

d94f3f2

Add testcase in c++: Set operation before any subscriber is created

fe9a204

qiluo-msft added 5 commits June 1, 2025 19:15

refactor: implement mod_entry with Table

9849e30

Implement PublisherEventTable::hdel()

a5cd031

Use channel name with DbID as tag, in order to prevent naming

8e6c52d

confliction in one redis instance across multiple databases

ConfigDBConnector_Native adds support of PublisherEventTable

1397c0f

Signed-off-by: Qi Luo <[email protected]>

Merge branch 'master' into qiluo/pubsubchannel

6602971

github-advanced-security bot found potential problems Jun 8, 2025

View reviewed changes

	#define REDIS_PUBLISH_MESSAGE_ELEMNTS (3)
	#define REDIS_PUBLISH_MESSAGE_ELEMENTS (3)


		namespace swss {

		class PublisherEventTable : public Table { // public TableBase, public TableEntryEnumerable {


		// void getKeys(std::vector<std::string> &keys);

		// void setBuffered(bool buffered);

Add PublisherEventTable/SubscriberEventTable classes #1020

Are you sure you want to change the base?

Add PublisherEventTable/SubscriberEventTable classes #1020

Uh oh!

Conversation

qiluo-msft commented May 29, 2025

Uh oh!

mssonicbld commented May 29, 2025

Uh oh!

azure-pipelines bot commented May 29, 2025

Uh oh!

mssonicbld commented May 29, 2025

Uh oh!

azure-pipelines bot commented May 29, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI May 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Check notice

Check notice

Check notice

Check notice

Check notice

Check notice

mssonicbld commented May 29, 2025

Uh oh!

azure-pipelines bot commented May 29, 2025

Uh oh!

mssonicbld commented May 29, 2025

Uh oh!

azure-pipelines bot commented May 29, 2025

Uh oh!

mssonicbld commented May 29, 2025

Uh oh!

azure-pipelines bot commented May 29, 2025

Uh oh!

mssonicbld commented May 29, 2025

Uh oh!

azure-pipelines bot commented May 29, 2025

Uh oh!

mssonicbld commented Jun 7, 2025

Uh oh!

azure-pipelines bot commented Jun 7, 2025

Uh oh!

Check notice

Check notice

Check notice

Check notice

Check notice

Check notice

bhouse-nexthop commented Jun 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants