Sequencer class with lambda response message #2

DanielaMurin · 2024-08-21T15:54:30Z

New Sequencer Class
Sequence number allocated according to the order that the events are received in Syncd
Passed sequence index in Syncd to function logic
The response is turned into a lambda and executed with the correct sequence order
Only added the sequence index to create/remove/set/get/bulk operations

Change-Id: Ic684138907605a2d79db9c85b84c425a5c5b0c59

Change-Id: I647dc326aac995785ebc1b2d882f85fd2b3d484d

…e second thread pop from the ring buffer and executes the function

Change-Id: I001bdc0f32d81eb847157a2f7ec8bc0faa5de1a8

Change-Id: Ie53906422adfc2027710ef9835dafe141448b84f

Change-Id: I90427922c574e9a5edaa722727878febe6c4d792

Change-Id: I9b7e9de507fb3926aa2c663548e43c97d0b99daa

Change-Id: I380534a136ced86bb0e3e3f06f31be8cc389ec74

Change-Id: I861379abf3db49fbe3488a874f092fbeb136491d

…ing/sonic-sairedis into syncd_multithread

Change-Id: Ia215d224868aef2a5767d7b03082398ffa38e562

…ing/sonic-sairedis into syncd_multithread

Change-Id: I24f0acbea04a99109371705b9529a0461779bfa6

…ing/sonic-sairedis into syncd_multithread

Change-Id: I90ef1f366c6b5f13c856d82f4aa9579c6fa9ff5b

Change-Id: I0e663cd3b7ce155298efd4f74fde86130ccf3b01

Change-Id: Id77121b6a52a535ffda7b31c699e0b6dc9a75ae1

Change-Id: I31bc11dd855c80ae551c1cdd6b750f9c06a24947

Fixed Problems/New Changes: Should be Tested:

Should be Tested:

marvellgit · 2024-08-29T21:03:44Z

syncd/Makefile.am

@@ -56,7 +56,8 @@ libSyncd_a_SOURCES = \
 				WatchdogScope.cpp \
 				Workaround.cpp \
 				ZeroMQNotificationProducer.cpp \
-				syncd_main.cpp
+				syncd_main.cpp \
+				Sequencer.cpp


why is ring buffer not here ?

marvellgit · 2024-08-29T21:05:40Z

syncd/Sequencer.cpp

@@ -0,0 +1,175 @@
+#include "Sequencer.h"
+#include "swss/logger.h"


add under ifdef

marvellgit · 2024-08-29T21:06:19Z

syncd/Sequencer.cpp

+#include "Sequencer.h"
+#include "swss/logger.h"
+
+using namespace syncd;


why is this needed ?

marvellgit · 2024-08-29T21:06:42Z

syncd/Sequencer.cpp

+    std::string logMsg;
+    SequenceStatus status = FAILURE;
+
+    logMsg = "multithreaded: Checking for ready responses in queue... \n";


add under debug flag or under if def

marvellgit · 2024-08-29T21:08:25Z

syncd/Sequencer.cpp

+        // Check if the next sequence number is in the map
+        auto seq_data = responses.find(next_seq_to_send);
+        if (seq_data == responses.end()) {
+            logMsg += "multithreaded: No next sequence found in queue \n";


add under debug flag or under if def

marvellgit · 2024-08-29T22:09:04Z

syncd/Syncd.cpp

+    if (!ringBuffer || ringBuffer->Started)
+	{
+		LogToModuleFile(getNameByRingBuffer(ringBuffer), "popRingBuffer return");
+        return;


add counter stats ?

marvellgit · 2024-08-29T22:12:48Z

syncd/Sequencer.cpp

+
+    logMsg = "multithreaded: Checking for ready responses in queue... \n";
+
+    while (true) {


condeider adding while not exit thread like in ring buffer

marvellgit · 2024-08-29T22:26:55Z

syncd/Syncd.cpp

@@ -298,39 +383,74 @@ bool Syncd::isInitViewMode() const
    return m_asicInitViewMode && m_commandLineOptions->m_enableTempView;
 }

+void point(_In_ const swss::KeyOpFieldsValuesTuple &kco){


?still here

marvellgit · 2024-08-29T22:28:31Z

syncd/Syncd.cpp

+        getApiRingBuffer(kco, ringBuffer);
+
+        int sequenceNumber;
+        if(int seq_status = m_sequencer->allocateSequenceNumber(&sequenceNumber) != Sequencer::SUCCESS)


no need for if just log return status

marvellgit · 2024-08-29T22:29:10Z

syncd/Syncd.cpp

+        int sequenceNumber;
+        if(int seq_status = m_sequencer->allocateSequenceNumber(&sequenceNumber) != Sequencer::SUCCESS)
+        {
+            std::string logMsg = "Failed to allocate sequence number: " + std::to_string(seq_status);


add sww log with warning

marvellgit · 2024-08-29T22:50:03Z

syncd/Sequencer.cpp

+
+// Helper function to execute all ready responses in order
+// check how many consecutive responses were executed in sucession and log it
+Sequencer::SequenceStatus Sequencer::executeReadyResponses() {


should not run under lock since block new ecxcute ... ?
but need to think if we lock each iteration - two treads can run in parallel ...

Fixed Problems/New Changes: Should be Tested:

marvellgit

need to reduce changes in compare
replace one line with one line ... so compare is better
also all debug crate string shpul dbe done in macro .....

marvellgit · 2024-08-30T10:43:27Z

syncd/Syncd.cpp

@@ -446,13 +567,14 @@ sai_status_t Syncd::processAttrCapabilityQuery(
            capability.create_implemented, capability.set_implemented, capability.get_implemented);
    }

-    m_selectableChannel->set(sai_serialize_status(status), entry, REDIS_ASIC_STATE_COMMAND_ATTR_CAPABILITY_RESPONSE);
+    sendStausResponseSequence(status, REDIS_ASIC_STATE_COMMAND_ATTR_CAPABILITY_RESPONSE, entry, sequenceNumber);


diff order than m_selectableChannel->need to verify

marvellgit · 2024-08-30T10:46:03Z

syncd/Syncd.cpp

-
-        attr_list = vidlist.get_attr_list();
-        attr_count = vidlist.get_attr_count();
+


why is this needed - in original code this was defined before ?

also vidlist was initialized in previous function and not moved as parameter
now reinitilized why ?

marvellgit · 2024-08-30T10:57:09Z

syncd/Syncd.cpp

@@ -5025,6 +5407,108 @@ static void timerWatchdogCallback(
    SWSS_LOG_ERROR("main loop execution exceeded %ld ms", span/1000);
 }

+void Syncd::getObjectTypeByOperation(


very complicated :-(

marvellgit · 2024-08-30T10:57:37Z

syncd/Syncd.cpp

@@ -5076,19 +5563,22 @@ void Syncd::run()
        SWSS_LOG_NOTICE("starting main loop, ONLY restart query");

        if (m_commandLineOptions->m_disableExitSleep)
-            runMainLoop = false;
+            runMainLoop = true;


false became true ?

marvellgit · 2024-08-30T10:58:54Z

syncd/Syncd.cpp

@@ -5182,11 +5672,59 @@ void Syncd::run()
            }
            else if (sel == m_flexCounter.get())
            {
-                processFlexCounterEvent(*(swss::ConsumerTable*)sel);
+                int sequenceNumber;


really need to have a s ervice or macro - too much code

marvellgit · 2024-08-30T10:59:51Z

syncd/Syncd.cpp

@@ -5296,3 +5836,73 @@ syncd_restart_type_t Syncd::handleRestartQuery(

    return RequestShutdownCommandLineOptions::stringToRestartType(op);
 }
+
+void Syncd::sendStatusAndEntryResponse(


see priviouse note, in should be status mybe alreadt serialized entry and than command type

…ewMode Fixed Problems/New Changes: Should be Tested:

Fixed Problems/New Changes: Should be Tested:

Change namespace Add is_exit param Fix logs Move statistics to public Add new statistics parmas Change-Id: I36f9d45babc7d4ee024aa8afde44edf1615da582

Change-Id: Ib5453d04e8d1f32a6114a32df9149df11c3bb75f

Change-Id: I7aad3550f334ae674eb93dfd66608fefb7aa0add

Sequencer class with lambda response message

b30be88

Change-Id: Ic684138907605a2d79db9c85b84c425a5c5b0c59

DanielaMurin requested review from marvellgit, matiAlfaro and shiraez August 21, 2024 15:54

DanielaMurin and others added 26 commits August 21, 2024 19:08

Add Sequencer to sendGetResponse

d0369a2

Change-Id: I647dc326aac995785ebc1b2d882f85fd2b3d484d

Ring Buffer - Phase 1: The main thread push to the ring buffer and th…

2caeedc

…e second thread pop from the ring buffer and executes the function

Remove singleton & fix compilation

98c6c89

Change-Id: I001bdc0f32d81eb847157a2f7ec8bc0faa5de1a8

Fix merge issues

78ec4a7

Change-Id: Ie53906422adfc2027710ef9835dafe141448b84f

add debug info + enable thread

65f94df

add delay

8523a2a

debug single ring buffer & enable sequencer

0ddc1ec

Change-Id: I90427922c574e9a5edaa722727878febe6c4d792

Add new debug file & move sequencer alloc to processEvent

cc73b2e

Change-Id: I9b7e9de507fb3926aa2c663548e43c97d0b99daa

fix reset sequencer logic

c1b916a

Change-Id: I380534a136ced86bb0e3e3f06f31be8cc389ec74

Take out lambda execute from within lock

8d0feae

Change-Id: I861379abf3db49fbe3488a874f092fbeb136491d

add loger

efc1bc9

Merge branch 'syncd_multithread' of https://github.com/Marvell-switch…

da6d6b9

…ing/sonic-sairedis into syncd_multithread

Working single ring buffer & sequencer

5e2e15c

Change-Id: Ia215d224868aef2a5767d7b03082398ffa38e562

Merge branch 'syncd_multithread' of https://github.com/Marvell-switch…

fdf56c9

…ing/sonic-sairedis into syncd_multithread

add logger to syncd + sequencer

d3fddf3

Return m_switches and m_mdioIpcServer logic

9b01706

Change-Id: I24f0acbea04a99109371705b9529a0461779bfa6

fix Logger

7a14462

Merge branch 'syncd_multithread' of https://github.com/Marvell-switch…

b4d0a06

…ing/sonic-sairedis into syncd_multithread

add logger.h file

04eb362

Update sequencer statistics/mux/logging

00b8ade

Change-Id: I90ef1f366c6b5f13c856d82f4aa9579c6fa9ff5b

Merge conflict fix, remove old logger code

94005f8

Change-Id: I0e663cd3b7ce155298efd4f74fde86130ccf3b01

enable flag to the logger

3f121a3

Return diag shell thread, improve lambda func readability

7a661ce

Change-Id: Id77121b6a52a535ffda7b31c699e0b6dc9a75ae1

Allocate sequence number inside functions

8ea0eb6

Change-Id: I31bc11dd855c80ae551c1cdd6b750f9c06a24947

add multiRing buffer support & send response for all ops

9908e23

Fixed Problems/New Changes: Should be Tested:

fix find operation group by string

328867e

Should be Tested:

marvellgit reviewed Aug 29, 2024

View reviewed changes

fix find ring buffer by op or objectType

762ae3b

Fixed Problems/New Changes: Should be Tested:

marvellgit reviewed Aug 30, 2024

View reviewed changes

aviramd and others added 10 commits August 31, 2024 13:17

fix sendApiResponse objectCount param on processBulkQuadEventInInitVi…

a5d4802

…ewMode Fixed Problems/New Changes: Should be Tested:

removing unnecessary code

b10bf7d

Fix send responde sync for nom get actions

0225abc

Fixed Problems/New Changes: Should be Tested:

create muliple threads, cleanup, change response seq order

6cf0e13

Fixed Problems/New Changes: Should be Tested:

Sequencer update & Cleanup

a2d711e

Change namespace Add is_exit param Fix logs Move statistics to public Add new statistics parmas Change-Id: I36f9d45babc7d4ee024aa8afde44edf1615da582

Sequencer update - ring buffer instead of map

4c72076

Change-Id: Ib5453d04e8d1f32a6114a32df9149df11c3bb75f

fix bug when redis reads empty key

c5e511a

REVERT - Sequencer update - ring buffer instead of map

db71e99

Change-Id: I7aad3550f334ae674eb93dfd66608fefb7aa0add

protected RedisClient + RedisSelectableChannel

d52b04e

remove LOCK from loop wrap it instead , add thread id prints, 3 threads

cad2dcb

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sequencer class with lambda response message #2

Sequencer class with lambda response message #2

DanielaMurin commented Aug 21, 2024

marvellgit Aug 29, 2024

marvellgit Aug 29, 2024

marvellgit Aug 29, 2024

marvellgit Aug 29, 2024

marvellgit Aug 29, 2024

marvellgit Aug 29, 2024

marvellgit Aug 29, 2024

marvellgit Aug 29, 2024

marvellgit Aug 29, 2024

marvellgit Aug 29, 2024

marvellgit Aug 29, 2024

marvellgit left a comment

marvellgit Aug 30, 2024

marvellgit Aug 30, 2024

marvellgit Aug 30, 2024

marvellgit Aug 30, 2024

marvellgit Aug 30, 2024

marvellgit Aug 30, 2024

marvellgit Aug 30, 2024

		@@ -0,0 +1,175 @@
		#include "Sequencer.h"
		#include "swss/logger.h"


		logMsg = "multithreaded: Checking for ready responses in queue... \n";

		while (true) {


		attr_list = vidlist.get_attr_list();
		attr_count = vidlist.get_attr_count();

Sequencer class with lambda response message #2

Are you sure you want to change the base?

Sequencer class with lambda response message #2

Conversation

DanielaMurin commented Aug 21, 2024

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marvellgit left a 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

Choose a reason for hiding this comment