Skip to content

Commit

Permalink
[orchdaemon]: Fixed sairedis record file rotation (sonic-net#2299)
Browse files Browse the repository at this point in the history
* [orchdaemon]: Fixed sairedis record file rotation

* What I did
Fix sonic-net#8162
Moved sairedis record file rotation logic out of flush() to fix issue.

Why I did it
Sairedis record file was not releasing the file handle on rotation. This is because the file handle release was inside the flush() which was only being called if a select timeout was triggered. Moved the logic to its own function which is called in the start() loop.

How I verified it
Ran a script to fill log and verified that rotation was happening correctly.

Signed-off-by: Bryan Crossland bryan.crossland@target.com
  • Loading branch information
bacrossland authored Oct 3, 2022
1 parent b8ee07d commit 24d29f1
Show file tree
Hide file tree
Showing 8 changed files with 84 additions and 17 deletions.
1 change: 0 additions & 1 deletion orchagent/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ bool gSairedisRecord = true;
bool gSwssRecord = true;
bool gResponsePublisherRecord = false;
bool gLogRotate = false;
bool gSaiRedisLogRotate = false;
bool gResponsePublisherLogRotate = false;
bool gSyncMode = false;
sai_redis_communication_mode_t gRedisCommunicationMode = SAI_REDIS_COMMUNICATION_MODE_REDIS_ASYNC;
Expand Down
34 changes: 24 additions & 10 deletions orchagent/orchdaemon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ FlowCounterRouteOrch *gFlowCounterRouteOrch;
DebugCounterOrch *gDebugCounterOrch;

bool gIsNatSupported = false;
bool gSaiRedisLogRotate = false;
event_handle_t g_events_handle;

#define DEFAULT_MAX_BULK_SIZE 1000
Expand Down Expand Up @@ -676,24 +677,26 @@ void OrchDaemon::flush()
SWSS_LOG_ERROR("Failed to flush redis pipeline %d", status);
abort();
}
}

// check if logroate is requested
if (gSaiRedisLogRotate)
/* Release the file handle so the log can be rotated */
void OrchDaemon::logRotate() {
SWSS_LOG_ENTER();
sai_attribute_t attr;
attr.id = SAI_REDIS_SWITCH_ATTR_PERFORM_LOG_ROTATE;
attr.value.booldata = true;
sai_status_t status = sai_switch_api->set_switch_attribute(gSwitchId, &attr);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_NOTICE("performing log rotate");

gSaiRedisLogRotate = false;

attr.id = SAI_REDIS_SWITCH_ATTR_PERFORM_LOG_ROTATE;
attr.value.booldata = true;

sai_switch_api->set_switch_attribute(gSwitchId, &attr);
SWSS_LOG_ERROR("Failed to release the file handle on sairedis log %d", status);
}
}


void OrchDaemon::start()
{
SWSS_LOG_ENTER();
gSaiRedisLogRotate = false;

for (Orch *o : m_orchList)
{
Expand Down Expand Up @@ -737,6 +740,17 @@ void OrchDaemon::start()
continue;
}

// check if logroate is requested
if (gSaiRedisLogRotate)
{
SWSS_LOG_NOTICE("performing log rotate");

gSaiRedisLogRotate = false;

logRotate();
continue;
}

auto *c = (Executor *)s;
c->execute();

Expand Down
3 changes: 3 additions & 0 deletions orchagent/orchdaemon.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,10 @@
#include "bfdorch.h"
#include "srv6orch.h"
#include "nvgreorch.h"
#include <sairedis.h>

using namespace swss;
extern bool gSaiRedisLogRotate;

class OrchDaemon
{
Expand All @@ -67,6 +69,7 @@ class OrchDaemon
{
m_fabricEnabled = enabled;
}
void logRotate();
private:
DBConnector *m_applDb;
DBConnector *m_configDb;
Expand Down
1 change: 0 additions & 1 deletion orchagent/p4orch/tests/test_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ size_t gMaxBulkSize = DEFAULT_MAX_BULK_SIZE;
bool gSairedisRecord = true;
bool gSwssRecord = true;
bool gLogRotate = false;
bool gSaiRedisLogRotate = false;
bool gResponsePublisherRecord = false;
bool gResponsePublisherLogRotate = false;
bool gSyncMode = false;
Expand Down
8 changes: 5 additions & 3 deletions tests/mock_tests/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ LDADD_GTEST = -L/usr/src/gtest

## Orchagent Unit Tests

tests_INCLUDES = -I $(FLEX_CTR_DIR) -I $(DEBUG_CTR_DIR) -I $(top_srcdir)/lib -I$(top_srcdir)/cfgmgr -I$(top_srcdir)/orchagent
tests_INCLUDES = -I $(FLEX_CTR_DIR) -I $(DEBUG_CTR_DIR) -I $(top_srcdir)/lib -I$(top_srcdir)/cfgmgr -I$(top_srcdir)/orchagent -I$(P4_ORCH_DIR)/tests

tests_SOURCES = aclorch_ut.cpp \
portsorch_ut.cpp \
Expand All @@ -47,6 +47,7 @@ tests_SOURCES = aclorch_ut.cpp \
fake_response_publisher.cpp \
swssnet_ut.cpp \
flowcounterrouteorch_ut.cpp \
orchdaemon_ut.cpp \
$(top_srcdir)/lib/gearboxutils.cpp \
$(top_srcdir)/lib/subintf.cpp \
$(top_srcdir)/orchagent/orchdaemon.cpp \
Expand Down Expand Up @@ -120,12 +121,13 @@ tests_SOURCES += $(P4_ORCH_DIR)/p4orch.cpp \
$(P4_ORCH_DIR)/wcmp_manager.cpp \
$(P4_ORCH_DIR)/mirror_session_manager.cpp \
$(P4_ORCH_DIR)/gre_tunnel_manager.cpp \
$(P4_ORCH_DIR)/l3_admit_manager.cpp
$(P4_ORCH_DIR)/l3_admit_manager.cpp \
$(P4_ORCH_DIR)/tests/mock_sai_switch.cpp

tests_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI)
tests_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI) $(tests_INCLUDES)
tests_LDADD = $(LDADD_GTEST) $(LDADD_SAI) -lnl-genl-3 -lhiredis -lhiredis -lpthread \
-lswsscommon -lswsscommon -lgtest -lgtest_main -lzmq -lnl-3 -lnl-route-3
-lswsscommon -lswsscommon -lgtest -lgtest_main -lzmq -lnl-3 -lnl-route-3 -lgmock -lgmock_main

## portsyncd unit tests

Expand Down
1 change: 0 additions & 1 deletion tests/mock_tests/mock_orchagent_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ int gBatchSize = DEFAULT_BATCH_SIZE;
bool gSairedisRecord = true;
bool gSwssRecord = true;
bool gLogRotate = false;
bool gSaiRedisLogRotate = false;
ofstream gRecordOfs;
string gRecordFile;
string gMySwitchType = "switch";
Expand Down
1 change: 0 additions & 1 deletion tests/mock_tests/mock_orchagent_main.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ extern int gBatchSize;
extern bool gSwssRecord;
extern bool gSairedisRecord;
extern bool gLogRotate;
extern bool gSaiRedisLogRotate;
extern ofstream gRecordOfs;
extern string gRecordFile;

Expand Down
52 changes: 52 additions & 0 deletions tests/mock_tests/orchdaemon_ut.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
#include "orchdaemon.h"
#include "dbconnector.h"
#include <gtest/gtest.h>
#include <gmock/gmock.h>
#include "mock_sai_switch.h"

extern sai_switch_api_t* sai_switch_api;
sai_switch_api_t test_sai_switch;

namespace orchdaemon_test
{

using ::testing::_;
using ::testing::Return;
using ::testing::StrictMock;

DBConnector appl_db("APPL_DB", 0);
DBConnector state_db("STATE_DB", 0);
DBConnector config_db("CONFIG_DB", 0);
DBConnector counters_db("COUNTERS_DB", 0);

class OrchDaemonTest : public ::testing::Test
{
public:
StrictMock<MockSaiSwitch> mock_sai_switch_;

OrchDaemon* orchd;

OrchDaemonTest()
{
mock_sai_switch = &mock_sai_switch_;
sai_switch_api = &test_sai_switch;
sai_switch_api->get_switch_attribute = &mock_get_switch_attribute;
sai_switch_api->set_switch_attribute = &mock_set_switch_attribute;

orchd = new OrchDaemon(&appl_db, &config_db, &state_db, &counters_db);

};

~OrchDaemonTest()
{

};
};

TEST_F(OrchDaemonTest, logRotate)
{
EXPECT_CALL(mock_sai_switch_, set_switch_attribute( _, _)).WillOnce(Return(SAI_STATUS_SUCCESS));

orchd->logRotate();
}
}

0 comments on commit 24d29f1

Please sign in to comment.