From 5cfc88648419f94e50e348f2313fe4a83e7fb69a Mon Sep 17 00:00:00 2001 From: Vivek Reddy Karri Date: Tue, 20 Jun 2023 22:53:24 +0000 Subject: [PATCH 1/9] Added new Recorder Interface and removed ResponsePublisher Global Variables Signed-off-by: Vivek Reddy Karri --- cfgmgr/buffermgrd.cpp | 6 - cfgmgr/coppmgrd.cpp | 6 - cfgmgr/intfmgrd.cpp | 6 - cfgmgr/macsecmgrd.cpp | 6 - cfgmgr/natmgrd.cpp | 5 - cfgmgr/nbrmgrd.cpp | 6 - cfgmgr/portmgrd.cpp | 6 - cfgmgr/sflowmgrd.cpp | 6 - cfgmgr/teammgrd.cpp | 4 - cfgmgr/tunnelmgrd.cpp | 6 - cfgmgr/vlanmgrd.cpp | 6 - cfgmgr/vrfmgrd.cpp | 7 +- cfgmgr/vxlanmgrd.cpp | 6 - orchagent/Makefile.am | 1 + orchagent/main.cpp | 30 ++--- orchagent/orch.cpp | 3 - orchagent/orch.h | 1 + orchagent/p4orch/tests/Makefile.am | 1 + orchagent/p4orch/tests/test_main.cpp | 2 - orchagent/recorder.cpp | 116 ++++++++++++++++++ orchagent/recorder.h | 75 +++++++++++ orchagent/response_publisher.cpp | 27 +--- orchagent/response_publisher.h | 1 + tests/mock_tests/Makefile.am | 1 + .../response_publisher_ut.cpp | 5 - 25 files changed, 210 insertions(+), 129 deletions(-) create mode 100644 orchagent/recorder.cpp create mode 100644 orchagent/recorder.h diff --git a/cfgmgr/buffermgrd.cpp b/cfgmgr/buffermgrd.cpp index 94176a54fd..580e765cc3 100644 --- a/cfgmgr/buffermgrd.cpp +++ b/cfgmgr/buffermgrd.cpp @@ -34,12 +34,6 @@ bool gSwssRecord = false; bool gLogRotate = false; ofstream gRecordOfs; string gRecordFile; -bool gResponsePublisherRecord = false; -bool gResponsePublisherLogRotate = false; -ofstream gResponsePublisherRecordOfs; -string gResponsePublisherRecordFile; -/* Global database mutex */ -mutex gDbMutex; void usage() { diff --git a/cfgmgr/coppmgrd.cpp b/cfgmgr/coppmgrd.cpp index 60b0a2442a..c537a585d5 100644 --- a/cfgmgr/coppmgrd.cpp +++ b/cfgmgr/coppmgrd.cpp @@ -29,12 +29,6 @@ bool gSwssRecord = false; bool gLogRotate = false; ofstream gRecordOfs; string gRecordFile; -bool gResponsePublisherRecord = false; -bool gResponsePublisherLogRotate = false; -ofstream gResponsePublisherRecordOfs; -string gResponsePublisherRecordFile; -/* Global database mutex */ -mutex gDbMutex; int main(int argc, char **argv) { diff --git a/cfgmgr/intfmgrd.cpp b/cfgmgr/intfmgrd.cpp index d07cb9af78..dcc4390ae5 100644 --- a/cfgmgr/intfmgrd.cpp +++ b/cfgmgr/intfmgrd.cpp @@ -29,12 +29,6 @@ bool gSwssRecord = false; bool gLogRotate = false; ofstream gRecordOfs; string gRecordFile; -bool gResponsePublisherRecord = false; -bool gResponsePublisherLogRotate = false; -ofstream gResponsePublisherRecordOfs; -string gResponsePublisherRecordFile; -/* Global database mutex */ -mutex gDbMutex; int main(int argc, char **argv) { diff --git a/cfgmgr/macsecmgrd.cpp b/cfgmgr/macsecmgrd.cpp index ff7bda9087..f575294512 100644 --- a/cfgmgr/macsecmgrd.cpp +++ b/cfgmgr/macsecmgrd.cpp @@ -39,12 +39,6 @@ bool gSwssRecord = false; bool gLogRotate = false; ofstream gRecordOfs; string gRecordFile; -bool gResponsePublisherRecord = false; -bool gResponsePublisherLogRotate = false; -ofstream gResponsePublisherRecordOfs; -string gResponsePublisherRecordFile; -/* Global database mutex */ -mutex gDbMutex; static bool received_sigterm = false; static struct sigaction old_sigaction; diff --git a/cfgmgr/natmgrd.cpp b/cfgmgr/natmgrd.cpp index db5a77f9a6..25a9e24773 100644 --- a/cfgmgr/natmgrd.cpp +++ b/cfgmgr/natmgrd.cpp @@ -52,11 +52,6 @@ bool gSwssRecord = false; bool gLogRotate = false; ofstream gRecordOfs; string gRecordFile; -bool gResponsePublisherRecord = false; -bool gResponsePublisherLogRotate = false; -ofstream gResponsePublisherRecordOfs; -string gResponsePublisherRecordFile; -mutex gDbMutex; NatMgr *natmgr = NULL; NotificationConsumer *timeoutNotificationsConsumer = NULL; diff --git a/cfgmgr/nbrmgrd.cpp b/cfgmgr/nbrmgrd.cpp index 338d8d9d0d..7cbc4ed938 100644 --- a/cfgmgr/nbrmgrd.cpp +++ b/cfgmgr/nbrmgrd.cpp @@ -33,12 +33,6 @@ bool gSwssRecord = false; bool gLogRotate = false; ofstream gRecordOfs; string gRecordFile; -bool gResponsePublisherRecord = false; -bool gResponsePublisherLogRotate = false; -ofstream gResponsePublisherRecordOfs; -string gResponsePublisherRecordFile; -/* Global database mutex */ -mutex gDbMutex; int main(int argc, char **argv) { diff --git a/cfgmgr/portmgrd.cpp b/cfgmgr/portmgrd.cpp index 944a881d06..1942b8a313 100644 --- a/cfgmgr/portmgrd.cpp +++ b/cfgmgr/portmgrd.cpp @@ -28,12 +28,6 @@ bool gSwssRecord = false; bool gLogRotate = false; ofstream gRecordOfs; string gRecordFile; -bool gResponsePublisherRecord = false; -bool gResponsePublisherLogRotate = false; -ofstream gResponsePublisherRecordOfs; -string gResponsePublisherRecordFile; -/* Global database mutex */ -mutex gDbMutex; int main(int argc, char **argv) { diff --git a/cfgmgr/sflowmgrd.cpp b/cfgmgr/sflowmgrd.cpp index 4672d04a42..1a96503ddb 100644 --- a/cfgmgr/sflowmgrd.cpp +++ b/cfgmgr/sflowmgrd.cpp @@ -28,12 +28,6 @@ bool gSwssRecord = false; bool gLogRotate = false; ofstream gRecordOfs; string gRecordFile; -bool gResponsePublisherRecord = false; -bool gResponsePublisherLogRotate = false; -ofstream gResponsePublisherRecordOfs; -string gResponsePublisherRecordFile; -/* Global database mutex */ -mutex gDbMutex; int main(int argc, char **argv) { diff --git a/cfgmgr/teammgrd.cpp b/cfgmgr/teammgrd.cpp index ff4151c921..35ed35309b 100644 --- a/cfgmgr/teammgrd.cpp +++ b/cfgmgr/teammgrd.cpp @@ -17,10 +17,6 @@ bool gSwssRecord = false; bool gLogRotate = false; ofstream gRecordOfs; string gRecordFile; -bool gResponsePublisherRecord = false; -bool gResponsePublisherLogRotate = false; -ofstream gResponsePublisherRecordOfs; -string gResponsePublisherRecordFile; bool received_sigterm = false; static struct sigaction old_sigaction; diff --git a/cfgmgr/tunnelmgrd.cpp b/cfgmgr/tunnelmgrd.cpp index 0a6a84eaeb..5cbf6e7a97 100644 --- a/cfgmgr/tunnelmgrd.cpp +++ b/cfgmgr/tunnelmgrd.cpp @@ -32,12 +32,6 @@ bool gSwssRecord = false; bool gLogRotate = false; ofstream gRecordOfs; string gRecordFile; -bool gResponsePublisherRecord = false; -bool gResponsePublisherLogRotate = false; -ofstream gResponsePublisherRecordOfs; -string gResponsePublisherRecordFile; -/* Global database mutex */ -mutex gDbMutex; int main(int argc, char **argv) { diff --git a/cfgmgr/vlanmgrd.cpp b/cfgmgr/vlanmgrd.cpp index b69dc78122..89931f57d2 100644 --- a/cfgmgr/vlanmgrd.cpp +++ b/cfgmgr/vlanmgrd.cpp @@ -36,12 +36,6 @@ bool gSwssRecord = false; bool gLogRotate = false; ofstream gRecordOfs; string gRecordFile; -bool gResponsePublisherRecord = false; -bool gResponsePublisherLogRotate = false; -ofstream gResponsePublisherRecordOfs; -string gResponsePublisherRecordFile; -/* Global database mutex */ -mutex gDbMutex; int main(int argc, char **argv) { diff --git a/cfgmgr/vrfmgrd.cpp b/cfgmgr/vrfmgrd.cpp index c7ca49b6bc..44f9292741 100644 --- a/cfgmgr/vrfmgrd.cpp +++ b/cfgmgr/vrfmgrd.cpp @@ -29,12 +29,7 @@ bool gSwssRecord = false; bool gLogRotate = false; ofstream gRecordOfs; string gRecordFile; -bool gResponsePublisherRecord = false; -bool gResponsePublisherLogRotate = false; -ofstream gResponsePublisherRecordOfs; -string gResponsePublisherRecordFile; -/* Global database mutex */ -mutex gDbMutex; + int main(int argc, char **argv) { diff --git a/cfgmgr/vxlanmgrd.cpp b/cfgmgr/vxlanmgrd.cpp index d47893a614..b6ec40e7d4 100644 --- a/cfgmgr/vxlanmgrd.cpp +++ b/cfgmgr/vxlanmgrd.cpp @@ -34,12 +34,6 @@ bool gSwssRecord = false; bool gLogRotate = false; ofstream gRecordOfs; string gRecordFile; -bool gResponsePublisherRecord = false; -bool gResponsePublisherLogRotate = false; -ofstream gResponsePublisherRecordOfs; -string gResponsePublisherRecordFile; -/* Global database mutex */ -mutex gDbMutex; MacAddress gMacAddress; int main(int argc, char **argv) diff --git a/orchagent/Makefile.am b/orchagent/Makefile.am index 5e3cd741cc..36d659071f 100644 --- a/orchagent/Makefile.am +++ b/orchagent/Makefile.am @@ -98,6 +98,7 @@ orchagent_SOURCES = \ bfdorch.cpp \ srv6orch.cpp \ response_publisher.cpp \ + recorder.cpp \ nvgreorch.cpp orchagent_SOURCES += flex_counter/flex_counter_manager.cpp flex_counter/flex_counter_stat_manager.cpp flex_counter/flow_counter_handler.cpp flex_counter/flowcounterrouteorch.cpp diff --git a/orchagent/main.cpp b/orchagent/main.cpp index 3e938e01f1..e71373f087 100644 --- a/orchagent/main.cpp +++ b/orchagent/main.cpp @@ -56,9 +56,7 @@ int gBatchSize = DEFAULT_BATCH_SIZE; bool gSairedisRecord = true; bool gSwssRecord = true; -bool gResponsePublisherRecord = false; bool gLogRotate = false; -bool gResponsePublisherLogRotate = false; bool gSyncMode = false; sai_redis_communication_mode_t gRedisCommunicationMode = SAI_REDIS_COMMUNICATION_MODE_REDIS_ASYNC; string gAsicInstance; @@ -67,8 +65,6 @@ extern bool gIsNatSupported; ofstream gRecordOfs; string gRecordFile; -ofstream gResponsePublisherRecordOfs; -string gResponsePublisherRecordFile; #define SAIREDIS_RECORD_ENABLE 0x1 #define SWSS_RECORD_ENABLE (0x1 << 1) @@ -110,7 +106,7 @@ void sighup_handler(int signo) */ gLogRotate = true; gSaiRedisLogRotate = true; - gResponsePublisherLogRotate = true; + Recorder::respub.setRotate(true); } void syncd_apply_view() @@ -454,9 +450,10 @@ int main(int argc, char **argv) gSairedisRecord = (record_type & SAIREDIS_RECORD_ENABLE) == SAIREDIS_RECORD_ENABLE; gSwssRecord = (record_type & SWSS_RECORD_ENABLE) == SWSS_RECORD_ENABLE; - gResponsePublisherRecord = + Recorder::respub.enable( (record_type & RESPONSE_PUBLISHER_RECORD_ENABLE) == - RESPONSE_PUBLISHER_RECORD_ENABLE; + RESPONSE_PUBLISHER_RECORD_ENABLE + ); /* Disable/enable SwSS recording */ if (gSwssRecord) @@ -471,22 +468,11 @@ int main(int argc, char **argv) gRecordOfs << getTimestamp() << "|recording started" << endl; } - // Disable/Enable response publisher recording. - if (gResponsePublisherRecord) + if (Recorder::respub.isRecord()) { - gResponsePublisherRecordFile = record_location + "/" + responsepublisher_rec_filename; - gResponsePublisherRecordOfs.open(gResponsePublisherRecordFile, std::ofstream::out | std::ofstream::app); - if (!gResponsePublisherRecordOfs.is_open()) - { - SWSS_LOG_ERROR("Failed to open Response Publisher recording file %s", - gResponsePublisherRecordFile.c_str()); - gResponsePublisherRecord = false; - } - else - { - gResponsePublisherRecordOfs << getTimestamp() << "|recording started" - << endl; - } + Recorder::respub.setLocation(record_location); + Recorder::respub.setLocation(responsepublisher_rec_filename); + Recorder::respub.startRec(false); } attr.id = SAI_SWITCH_ATTR_PORT_STATE_CHANGE_NOTIFY; diff --git a/orchagent/orch.cpp b/orchagent/orch.cpp index 5607d5c027..be73795fae 100644 --- a/orchagent/orch.cpp +++ b/orchagent/orch.cpp @@ -1,7 +1,4 @@ -#include -#include #include -#include #include #include #include "timestamp.h" diff --git a/orchagent/orch.h b/orchagent/orch.h index bb39d34589..35bcb25bc6 100644 --- a/orchagent/orch.h +++ b/orchagent/orch.h @@ -21,6 +21,7 @@ extern "C" { #include "selectabletimer.h" #include "macaddress.h" #include "response_publisher.h" +#include "recorder.h" const char delimiter = ':'; const char list_item_delimiter = ','; diff --git a/orchagent/p4orch/tests/Makefile.am b/orchagent/p4orch/tests/Makefile.am index d759033c68..2bf6dae443 100644 --- a/orchagent/p4orch/tests/Makefile.am +++ b/orchagent/p4orch/tests/Makefile.am @@ -28,6 +28,7 @@ p4orch_tests_SOURCES = $(ORCHAGENT_DIR)/orch.cpp \ $(ORCHAGENT_DIR)/copporch.cpp \ $(ORCHAGENT_DIR)/switchorch.cpp \ $(ORCHAGENT_DIR)/request_parser.cpp \ + $(ORCHAGENT_DIR)/recorder.cpp \ $(ORCHAGENT_DIR)/flex_counter/flex_counter_manager.cpp \ $(ORCHAGENT_DIR)/flex_counter/flow_counter_handler.cpp \ $(P4ORCH_DIR)/p4oidmapper.cpp \ diff --git a/orchagent/p4orch/tests/test_main.cpp b/orchagent/p4orch/tests/test_main.cpp index 08b72a9377..7863433faa 100644 --- a/orchagent/p4orch/tests/test_main.cpp +++ b/orchagent/p4orch/tests/test_main.cpp @@ -43,8 +43,6 @@ size_t gMaxBulkSize = DEFAULT_MAX_BULK_SIZE; bool gSairedisRecord = true; bool gSwssRecord = true; bool gLogRotate = false; -bool gResponsePublisherRecord = false; -bool gResponsePublisherLogRotate = false; bool gSyncMode = false; bool gIsNatSupported = false; diff --git a/orchagent/recorder.cpp b/orchagent/recorder.cpp new file mode 100644 index 0000000000..7985f8f648 --- /dev/null +++ b/orchagent/recorder.cpp @@ -0,0 +1,116 @@ +#include "recorder.h" +#include "logger.h" +#include + +using namespace swss; + +const std::string Recorder::DEFAULT_DIR = "."; +const std::string Recorder::REC_START = "|recording started"; +const std::string Recorder::SWSS_FNAME = "swss.rec"; +const std::string Recorder::SAIREDIS_FNAME = "sairedis.rec"; +const std::string Recorder::RESPPUB_FNAME = "responsepublisher.rec"; + + +SwSSRec::SwSSRec() +{ + /* Set Default values */ + setRecord(true); + setRotate(false); + setLocation(Recorder::DEFAULT_DIR); + setFileName(Recorder::SWSS_FNAME); +} + + +ResPubRec::ResPubRec() +{ + /* Set Default values */ + setRecord(false); + setRotate(false); + setLocation(Recorder::DEFAULT_DIR); + setFileName(Recorder::RESPPUB_FNAME); +} + + +SaiRedisRec::SaiRedisRec() +{ + /* Set Default values */ + setRecord(true); + setRotate(false); + setLocation(Recorder::DEFAULT_DIR); + setFileName(Recorder::SAIREDIS_FNAME); +} + + +void RecWriter::startRec(bool exit_if_failure) +{ + if (!isRecord()) + { + return ; + } + + if (record_ofs.is_open()) + { + SWSS_LOG_ERROR("Record File %s is already open", fname.c_str()); + } + + fname = getLoc() + "/" + getFile(); + record_ofs.open(fname, std::ofstream::out | std::ofstream::app); + if (!record_ofs.is_open()) + { + SWSS_LOG_ERROR("Failed to open recording file %s: %s", fname.c_str(), strerror(errno)); + if (exit_if_failure) + { + exit(EXIT_FAILURE); + } + else + { + setRecord(false); + } + } + record_ofs << getTimestamp() << Recorder::REC_START << std::endl; +} + + +RecWriter::~RecWriter() +{ + if (record_ofs.is_open()) + { + record_ofs.close(); + } +} + + +void RecWriter::record(const std::string& val) +{ + record_ofs << getTimestamp() << "|" << val << std::endl; + if (isRotate()) + { + setRotate(false); + logfileReopen(); + } +} + + +void RecWriter::logfileReopen() +{ + if (!isRecord()) + { + return ; + } + + record_ofs.close(); + + /* + * On log rotate we will use the same file name, we are assuming that + * logrotate daemon move filename to filename.1 and we will create new + * empty file here. + */ + + record_ofs.open(fname, std::ofstream::out | std::ofstream::app); + + if (!record_ofs.is_open()) + { + SWSS_LOG_ERROR("Failed to open recording file %s: %s", fname.c_str(), strerror(errno)); + return; + } +} \ No newline at end of file diff --git a/orchagent/recorder.h b/orchagent/recorder.h new file mode 100644 index 0000000000..8f5dc6bef5 --- /dev/null +++ b/orchagent/recorder.h @@ -0,0 +1,75 @@ +#pragma once + +#include +#include +#include +#include +#include + +namespace swss { + +class Recorder { +public: + static const std::string DEFAULT_DIR; + static const std::string REC_START; + static const std::string SWSS_FNAME; + static const std::string SAIREDIS_FNAME; + static const std::string RESPPUB_FNAME; + + static SwSSRec swss; + static SaiRedisRec sairedis; + static ResPubRec respub; +}; + +class RecBase { +public: + RecBase() {} + /* Setters */ + void setRecord(bool record) { enable_rec = record; } + void setRotate(bool rotate) { log_rotate = rotate; } + void setLocation(const std::string& loc) { location = loc; } + void setFileName(const std::string& name) { filename = name; } + + /* getters */ + bool isRecord() { return enable_rec; } + bool isRotate() { return log_rotate; } + std::string getLoc() { return location; } + std::string getFile() { return filename; } + +private: + bool enable_rec; + std::string location; + std::string filename; + bool log_rotate; +}; + +class RecWriter : public RecBase { +public: + RecWriter() : RecBase() {} + virtual ~RecWriter(); + void startRec(bool exit_if_failure); + void record(const std::string& val); + void logfileReopen(); + +private: + std::ofstream record_ofs; + std::string fname; +}; + +class SwSSRec : public RecWriter { +public: + SwSSRec(); +}; + +/* Record Handler for Response Publisher Class */ +class ResPubRec : public RecWriter { +public: + ResPubRec(); +}; + +class SaiRedisRec : public RecBase { +public: + SaiRedisRec(); +}; + +} \ No newline at end of file diff --git a/orchagent/response_publisher.cpp b/orchagent/response_publisher.cpp index 169075faa4..6e1cd64273 100644 --- a/orchagent/response_publisher.cpp +++ b/orchagent/response_publisher.cpp @@ -7,11 +7,6 @@ #include "timestamp.h" -extern bool gResponsePublisherRecord; -extern bool gResponsePublisherLogRotate; -extern std::ofstream gResponsePublisherRecordOfs; -extern std::string gResponsePublisherRecordFile; - namespace { @@ -37,25 +32,13 @@ std::string PrependedComponent(const ReturnCode &status) void PerformLogRotate() { - if (!gResponsePublisherLogRotate) - { - return; - } - gResponsePublisherLogRotate = false; - - gResponsePublisherRecordOfs.close(); - gResponsePublisherRecordOfs.open(gResponsePublisherRecordFile); - if (!gResponsePublisherRecordOfs.is_open()) - { - SWSS_LOG_ERROR("Failed to reopen Response Publisher record file %s: %s", gResponsePublisherRecordFile.c_str(), - strerror(errno)); - } + Recorder::respub.logfileReopen(); } void RecordDBWrite(const std::string &table, const std::string &key, const std::vector &attrs, const std::string &op) { - if (!gResponsePublisherRecord) + if (!Recorder::respub.isRecord()) { return; } @@ -67,13 +50,13 @@ void RecordDBWrite(const std::string &table, const std::string &key, const std:: } PerformLogRotate(); - gResponsePublisherRecordOfs << swss::getTimestamp() << "|" << s << std::endl; + Recorder::respub.record(s); } void RecordResponse(const std::string &response_channel, const std::string &key, const std::vector &attrs, const std::string &status) { - if (!gResponsePublisherRecord) + if (!Recorder::respub.isRecord()) { return; } @@ -85,7 +68,7 @@ void RecordResponse(const std::string &response_channel, const std::string &key, } PerformLogRotate(); - gResponsePublisherRecordOfs << swss::getTimestamp() << "|" << s << std::endl; + Recorder::respub.record(s); } } // namespace diff --git a/orchagent/response_publisher.h b/orchagent/response_publisher.h index db882d9c70..ff7bd291e4 100644 --- a/orchagent/response_publisher.h +++ b/orchagent/response_publisher.h @@ -9,6 +9,7 @@ #include "notificationproducer.h" #include "response_publisher_interface.h" #include "table.h" +#include "recorder.h" // This class performs two tasks when publish is called: // 1. Sends a notification into the redis channel. diff --git a/tests/mock_tests/Makefile.am b/tests/mock_tests/Makefile.am index fd8982eae8..e06d75f2cc 100644 --- a/tests/mock_tests/Makefile.am +++ b/tests/mock_tests/Makefile.am @@ -56,6 +56,7 @@ tests_SOURCES = aclorch_ut.cpp \ test_failure_handling.cpp \ $(top_srcdir)/lib/gearboxutils.cpp \ $(top_srcdir)/lib/subintf.cpp \ + $(top_srcdir)/orchagent/recorder.cpp \ $(top_srcdir)/orchagent/orchdaemon.cpp \ $(top_srcdir)/orchagent/orch.cpp \ $(top_srcdir)/orchagent/notifications.cpp \ diff --git a/tests/mock_tests/response_publisher/response_publisher_ut.cpp b/tests/mock_tests/response_publisher/response_publisher_ut.cpp index 3738ac6d87..9e836bad04 100644 --- a/tests/mock_tests/response_publisher/response_publisher_ut.cpp +++ b/tests/mock_tests/response_publisher/response_publisher_ut.cpp @@ -2,11 +2,6 @@ #include -bool gResponsePublisherRecord{false}; -bool gResponsePublisherLogRotate{false}; -std::ofstream gResponsePublisherRecordOfs; -std::string gResponsePublisherRecordFile; - using namespace swss; TEST(ResponsePublisher, TestPublish) From 3f604328fd036c35527c1245b93606c8f41eb67e Mon Sep 17 00:00:00 2001 From: Vivek Reddy Karri Date: Tue, 20 Jun 2023 23:18:09 +0000 Subject: [PATCH 2/9] Removed sairedis.rec global variables Signed-off-by: Vivek Reddy Karri --- orchagent/main.cpp | 20 +++++++++++++------- orchagent/p4orch/tests/test_main.cpp | 1 - orchagent/saihelper.cpp | 16 +++++++--------- orchagent/saihelper.h | 2 +- tests/mock_tests/mock_orchagent_main.cpp | 1 - tests/mock_tests/mock_orchagent_main.h | 1 - 6 files changed, 21 insertions(+), 20 deletions(-) diff --git a/orchagent/main.cpp b/orchagent/main.cpp index e71373f087..f5d3edf997 100644 --- a/orchagent/main.cpp +++ b/orchagent/main.cpp @@ -20,7 +20,7 @@ extern "C" { #include #include "timestamp.h" - +s #include #include @@ -54,7 +54,6 @@ extern size_t gMaxBulkSize; #define DEFAULT_BATCH_SIZE 128 int gBatchSize = DEFAULT_BATCH_SIZE; -bool gSairedisRecord = true; bool gSwssRecord = true; bool gLogRotate = false; bool gSyncMode = false; @@ -105,7 +104,7 @@ void sighup_handler(int signo) * Don't do any logging since they are using mutexes. */ gLogRotate = true; - gSaiRedisLogRotate = true; + Recorder::sairedis.setRotate(true); Recorder::respub.setRotate(true); } @@ -433,6 +432,12 @@ int main(int argc, char **argv) SWSS_LOG_NOTICE("--- Starting Orchestration Agent ---"); + Recorder::sairedis.enable( + (record_type & SAIREDIS_RECORD_ENABLE) == SAIREDIS_RECORD_ENABLE + ); + Recorder::sairedis.setLocation(record_location); + Recorder::sairedis.setFileName(sairedis_rec_filename); + initSaiApi(); initSaiRedis(record_location, sairedis_rec_filename); @@ -447,13 +452,14 @@ int main(int argc, char **argv) attrs.push_back(attr); // Initialize recording parameters. - gSairedisRecord = - (record_type & SAIREDIS_RECORD_ENABLE) == SAIREDIS_RECORD_ENABLE; - gSwssRecord = (record_type & SWSS_RECORD_ENABLE) == SWSS_RECORD_ENABLE; + Recorder::sairedis.enable( + (record_type & SAIREDIS_RECORD_ENABLE) == SAIREDIS_RECORD_ENABLE + ); Recorder::respub.enable( (record_type & RESPONSE_PUBLISHER_RECORD_ENABLE) == RESPONSE_PUBLISHER_RECORD_ENABLE ); + gSwssRecord = (record_type & SWSS_RECORD_ENABLE) == SWSS_RECORD_ENABLE; /* Disable/enable SwSS recording */ if (gSwssRecord) @@ -471,7 +477,7 @@ int main(int argc, char **argv) if (Recorder::respub.isRecord()) { Recorder::respub.setLocation(record_location); - Recorder::respub.setLocation(responsepublisher_rec_filename); + Recorder::respub.setFileName(responsepublisher_rec_filename); Recorder::respub.startRec(false); } diff --git a/orchagent/p4orch/tests/test_main.cpp b/orchagent/p4orch/tests/test_main.cpp index 7863433faa..cb6e27cdb2 100644 --- a/orchagent/p4orch/tests/test_main.cpp +++ b/orchagent/p4orch/tests/test_main.cpp @@ -40,7 +40,6 @@ sai_object_id_t gUnderlayIfId; int gBatchSize = DEFAULT_BATCH_SIZE; #define DEFAULT_MAX_BULK_SIZE 1000 size_t gMaxBulkSize = DEFAULT_MAX_BULK_SIZE; -bool gSairedisRecord = true; bool gSwssRecord = true; bool gLogRotate = false; bool gSyncMode = false; diff --git a/orchagent/saihelper.cpp b/orchagent/saihelper.cpp index 3f3e5ba5cd..c4861f8e39 100644 --- a/orchagent/saihelper.cpp +++ b/orchagent/saihelper.cpp @@ -75,10 +75,6 @@ sai_my_mac_api_t* sai_my_mac_api; sai_generic_programmable_api_t* sai_generic_programmable_api; extern sai_object_id_t gSwitchId; -extern bool gSairedisRecord; -extern bool gSwssRecord; -extern ofstream gRecordOfs; -extern string gRecordFile; static map hardware_access_map = { @@ -244,7 +240,7 @@ void initSaiApi() sai_log_set(SAI_API_GENERIC_PROGRAMMABLE, SAI_LOG_LEVEL_NOTICE); } -void initSaiRedis(const string &record_location, const std::string &record_filename) +void initSaiRedis() { /** * NOTE: Notice that all Redis attributes here are using SAI_NULL_OBJECT_ID @@ -255,9 +251,11 @@ void initSaiRedis(const string &record_location, const std::string &record_filen sai_attribute_t attr; sai_status_t status; - /* set recording dir before enable recording */ + auto record_filename = Recorder::sairedis.getFile(); + auto record_location = Recorder::sairedis.getLoc(); - if (gSairedisRecord) + /* set recording dir before enable recording */ + if (Recorder::sairedis.isRecord()) { attr.id = SAI_REDIS_SWITCH_ATTR_RECORDING_OUTPUT_DIR; attr.value.s8list.count = (uint32_t)record_location.size(); @@ -288,13 +286,13 @@ void initSaiRedis(const string &record_location, const std::string &record_filen /* Disable/enable SAI Redis recording */ attr.id = SAI_REDIS_SWITCH_ATTR_RECORD; - attr.value.booldata = gSairedisRecord; + attr.value.booldata = Recorder::sairedis.isRecord(); status = sai_switch_api->set_switch_attribute(gSwitchId, &attr); if (status != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("Failed to %s SAI Redis recording, rv:%d", - gSairedisRecord ? "enable" : "disable", status); + Recorder::sairedis.isRecord() ? "enable" : "disable", status); exit(EXIT_FAILURE); } diff --git a/orchagent/saihelper.h b/orchagent/saihelper.h index c7ff8d23ea..b83f894c2e 100644 --- a/orchagent/saihelper.h +++ b/orchagent/saihelper.h @@ -9,7 +9,7 @@ ((attrId) >= SAI_ ## objectType ## _ATTR_ ## attrPrefix ## _START && (attrId) <= SAI_ ## objectType ## _ATTR_ ## attrPrefix ## _END) void initSaiApi(); -void initSaiRedis(const std::string &record_location, const std::string &record_filename); +void initSaiRedis(); sai_status_t initSaiPhyApi(swss::gearbox_phy_t *phy); /* Handling SAI status*/ diff --git a/tests/mock_tests/mock_orchagent_main.cpp b/tests/mock_tests/mock_orchagent_main.cpp index ff2f902f39..cd278fdfe1 100644 --- a/tests/mock_tests/mock_orchagent_main.cpp +++ b/tests/mock_tests/mock_orchagent_main.cpp @@ -15,7 +15,6 @@ MacAddress gVxlanMacAddress; #define DEFAULT_BATCH_SIZE 128 int gBatchSize = DEFAULT_BATCH_SIZE; -bool gSairedisRecord = true; bool gSwssRecord = true; bool gLogRotate = false; ofstream gRecordOfs; diff --git a/tests/mock_tests/mock_orchagent_main.h b/tests/mock_tests/mock_orchagent_main.h index 4fd00d41fc..dc94c502f5 100644 --- a/tests/mock_tests/mock_orchagent_main.h +++ b/tests/mock_tests/mock_orchagent_main.h @@ -31,7 +31,6 @@ extern int gBatchSize; extern bool gSwssRecord; -extern bool gSairedisRecord; extern bool gLogRotate; extern ofstream gRecordOfs; extern string gRecordFile; From b9140490f8fced861d00ff5a87ceac2930083c59 Mon Sep 17 00:00:00 2001 From: Vivek Reddy Karri Date: Wed, 21 Jun 2023 04:00:31 +0000 Subject: [PATCH 3/9] Removed all recorder related global variable and used Recorder interface Signed-off-by: Vivek Reddy Karri --- cfgmgr/Makefile.am | 31 +++++++----- cfgmgr/buffermgrd.cpp | 12 ----- cfgmgr/coppmgrd.cpp | 12 ----- cfgmgr/intfmgrd.cpp | 12 ----- cfgmgr/macsecmgrd.cpp | 15 +----- cfgmgr/natmgr.cpp | 12 ++--- cfgmgr/natmgrd.cpp | 14 +----- cfgmgr/nbrmgrd.cpp | 12 ----- cfgmgr/portmgrd.cpp | 12 ----- cfgmgr/sflowmgrd.cpp | 12 ----- cfgmgr/teammgrd.cpp | 5 -- cfgmgr/tunnelmgrd.cpp | 12 ----- cfgmgr/vlanmgr.cpp | 8 ++-- cfgmgr/vlanmgrd.cpp | 15 +----- cfgmgr/vrfmgrd.cpp | 13 ----- cfgmgr/vxlanmgrd.cpp | 12 ----- {orchagent => lib}/recorder.cpp | 20 +++++--- {orchagent => lib}/recorder.h | 29 +++++------ orchagent/Makefile.am | 2 +- orchagent/main.cpp | 72 ++++++++++++---------------- orchagent/orch.cpp | 62 +++--------------------- orchagent/orch.h | 10 ++-- orchagent/orchdaemon.cpp | 11 ++--- orchagent/orchdaemon.h | 1 - orchagent/p4orch/tests/Makefile.am | 2 +- orchagent/p4orch/tests/test_main.cpp | 4 -- orchagent/response_publisher.cpp | 10 ++-- orchagent/saihelper.cpp | 11 ++--- tests/mock_tests/Makefile.am | 7 ++- 29 files changed, 122 insertions(+), 328 deletions(-) rename {orchagent => lib}/recorder.cpp (83%) rename {orchagent => lib}/recorder.h (88%) diff --git a/cfgmgr/Makefile.am b/cfgmgr/Makefile.am index 69cefc8052..685ab04407 100644 --- a/cfgmgr/Makefile.am +++ b/cfgmgr/Makefile.am @@ -26,67 +26,72 @@ else DBGFLAGS = -g endif -vlanmgrd_SOURCES = vlanmgrd.cpp vlanmgr.cpp $(top_srcdir)/orchagent/orch.cpp $(top_srcdir)/orchagent/request_parser.cpp $(top_srcdir)/orchagent/response_publisher.cpp shellcmd.h +COMMON_ORCH_SOURCE = $(top_srcdir)/orchagent/orch.cpp \ + $(top_srcdir)/orchagent/request_parser.cpp \ + $(top_srcdir)/orchagent/response_publisher.cpp \ + $(top_srcdir)/lib/recorder.cpp + +vlanmgrd_SOURCES = vlanmgrd.cpp vlanmgr.cpp $(COMMON_ORCH_SOURCE) shellcmd.h vlanmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN) vlanmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN) vlanmgrd_LDADD = $(LDFLAGS_ASAN) $(COMMON_LIBS) $(SAIMETA_LIBS) -teammgrd_SOURCES = teammgrd.cpp teammgr.cpp $(top_srcdir)/orchagent/orch.cpp $(top_srcdir)/orchagent/request_parser.cpp $(top_srcdir)/orchagent/response_publisher.cpp shellcmd.h +teammgrd_SOURCES = teammgrd.cpp teammgr.cpp $(COMMON_ORCH_SOURCE) shellcmd.h teammgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN) teammgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN) teammgrd_LDADD = $(LDFLAGS_ASAN) $(COMMON_LIBS) $(SAIMETA_LIBS) -portmgrd_SOURCES = portmgrd.cpp portmgr.cpp $(top_srcdir)/orchagent/orch.cpp $(top_srcdir)/orchagent/request_parser.cpp $(top_srcdir)/orchagent/response_publisher.cpp shellcmd.h +portmgrd_SOURCES = portmgrd.cpp portmgr.cpp $(COMMON_ORCH_SOURCE) shellcmd.h portmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN) portmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN) portmgrd_LDADD = $(LDFLAGS_ASAN) $(COMMON_LIBS) $(SAIMETA_LIBS) -intfmgrd_SOURCES = intfmgrd.cpp intfmgr.cpp $(top_srcdir)/orchagent/orch.cpp $(top_srcdir)/orchagent/request_parser.cpp $(top_srcdir)/lib/subintf.cpp $(top_srcdir)/orchagent/response_publisher.cpp shellcmd.h +intfmgrd_SOURCES = intfmgrd.cpp intfmgr.cpp $(top_srcdir)/lib/subintf.cpp $(COMMON_ORCH_SOURCE) shellcmd.h intfmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN) intfmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN) intfmgrd_LDADD = $(LDFLAGS_ASAN) $(COMMON_LIBS) $(SAIMETA_LIBS) -buffermgrd_SOURCES = buffermgrd.cpp buffermgr.cpp buffermgrdyn.cpp $(top_srcdir)/orchagent/orch.cpp $(top_srcdir)/orchagent/request_parser.cpp $(top_srcdir)/orchagent/response_publisher.cpp shellcmd.h +buffermgrd_SOURCES = buffermgrd.cpp buffermgr.cpp buffermgrdyn.cpp $(COMMON_ORCH_SOURCE) shellcmd.h buffermgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN) buffermgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN) buffermgrd_LDADD = $(LDFLAGS_ASAN) $(COMMON_LIBS) $(SAIMETA_LIBS) -vrfmgrd_SOURCES = vrfmgrd.cpp vrfmgr.cpp $(top_srcdir)/orchagent/orch.cpp $(top_srcdir)/orchagent/request_parser.cpp $(top_srcdir)/orchagent/response_publisher.cpp shellcmd.h +vrfmgrd_SOURCES = vrfmgrd.cpp vrfmgr.cpp $(COMMON_ORCH_SOURCE) shellcmd.h vrfmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN) vrfmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN) vrfmgrd_LDADD = $(LDFLAGS_ASAN) $(COMMON_LIBS) $(SAIMETA_LIBS) -nbrmgrd_SOURCES = nbrmgrd.cpp nbrmgr.cpp $(top_srcdir)/orchagent/orch.cpp $(top_srcdir)/orchagent/request_parser.cpp $(top_srcdir)/orchagent/response_publisher.cpp shellcmd.h +nbrmgrd_SOURCES = nbrmgrd.cpp nbrmgr.cpp $(COMMON_ORCH_SOURCE) shellcmd.h nbrmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(LIBNL_CFLAGS) $(CFLAGS_ASAN) nbrmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(LIBNL_CPPFLAGS) $(CFLAGS_ASAN) nbrmgrd_LDADD = $(LDFLAGS_ASAN) $(COMMON_LIBS) $(SAIMETA_LIBS) $(LIBNL_LIBS) -vxlanmgrd_SOURCES = vxlanmgrd.cpp vxlanmgr.cpp $(top_srcdir)/orchagent/orch.cpp $(top_srcdir)/orchagent/request_parser.cpp $(top_srcdir)/orchagent/response_publisher.cpp shellcmd.h +vxlanmgrd_SOURCES = vxlanmgrd.cpp vxlanmgr.cpp $(COMMON_ORCH_SOURCE) shellcmd.h vxlanmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN) vxlanmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN) vxlanmgrd_LDADD = $(LDFLAGS_ASAN) $(COMMON_LIBS) $(SAIMETA_LIBS) -sflowmgrd_SOURCES = sflowmgrd.cpp sflowmgr.cpp $(top_srcdir)/orchagent/orch.cpp $(top_srcdir)/orchagent/request_parser.cpp $(top_srcdir)/orchagent/response_publisher.cpp shellcmd.h +sflowmgrd_SOURCES = sflowmgrd.cpp sflowmgr.cpp $(COMMON_ORCH_SOURCE) shellcmd.h sflowmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN) sflowmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN) sflowmgrd_LDADD = $(LDFLAGS_ASAN) $(COMMON_LIBS) $(SAIMETA_LIBS) -natmgrd_SOURCES = natmgrd.cpp natmgr.cpp $(top_srcdir)/orchagent/orch.cpp $(top_srcdir)/orchagent/request_parser.cpp $(top_srcdir)/orchagent/response_publisher.cpp shellcmd.h +natmgrd_SOURCES = natmgrd.cpp natmgr.cpp $(COMMON_ORCH_SOURCE) shellcmd.h natmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN) natmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN) natmgrd_LDADD = $(LDFLAGS_ASAN) $(COMMON_LIBS) $(SAIMETA_LIBS) -coppmgrd_SOURCES = coppmgrd.cpp coppmgr.cpp $(top_srcdir)/orchagent/orch.cpp $(top_srcdir)/orchagent/request_parser.cpp $(top_srcdir)/orchagent/response_publisher.cpp shellcmd.h +coppmgrd_SOURCES = coppmgrd.cpp coppmgr.cpp $(COMMON_ORCH_SOURCE) shellcmd.h coppmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN) coppmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN) coppmgrd_LDADD = $(LDFLAGS_ASAN) $(COMMON_LIBS) $(SAIMETA_LIBS) -tunnelmgrd_SOURCES = tunnelmgrd.cpp tunnelmgr.cpp $(top_srcdir)/orchagent/orch.cpp $(top_srcdir)/orchagent/request_parser.cpp $(top_srcdir)/orchagent/response_publisher.cpp shellcmd.h +tunnelmgrd_SOURCES = tunnelmgrd.cpp tunnelmgr.cpp $(COMMON_ORCH_SOURCE) shellcmd.h tunnelmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN) tunnelmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN) tunnelmgrd_LDADD = $(LDFLAGS_ASAN) $(COMMON_LIBS) $(SAIMETA_LIBS) -macsecmgrd_SOURCES = macsecmgrd.cpp macsecmgr.cpp $(top_srcdir)/orchagent/orch.cpp $(top_srcdir)/orchagent/request_parser.cpp $(top_srcdir)/orchagent/response_publisher.cpp shellcmd.h +macsecmgrd_SOURCES = macsecmgrd.cpp macsecmgr.cpp $(COMMON_ORCH_SOURCE) shellcmd.h macsecmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN) macsecmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN) macsecmgrd_LDADD = $(LDFLAGS_ASAN) $(COMMON_LIBS) $(SAIMETA_LIBS) diff --git a/cfgmgr/buffermgrd.cpp b/cfgmgr/buffermgrd.cpp index 580e765cc3..7032b2939e 100644 --- a/cfgmgr/buffermgrd.cpp +++ b/cfgmgr/buffermgrd.cpp @@ -21,19 +21,7 @@ using json = nlohmann::json; /* SELECT() function timeout retry time, in millisecond */ #define SELECT_TIMEOUT 1000 -/* - * Following global variables are defined here for the purpose of - * using existing Orch class which is to be refactored soon to - * eliminate the direct exposure of the global variables. - * - * Once Orch class refactoring is done, these global variables - * should be removed from here. - */ int gBatchSize = 0; -bool gSwssRecord = false; -bool gLogRotate = false; -ofstream gRecordOfs; -string gRecordFile; void usage() { diff --git a/cfgmgr/coppmgrd.cpp b/cfgmgr/coppmgrd.cpp index c537a585d5..fbe66aa548 100644 --- a/cfgmgr/coppmgrd.cpp +++ b/cfgmgr/coppmgrd.cpp @@ -16,19 +16,7 @@ using namespace swss; /* select() function timeout retry time, in millisecond */ #define SELECT_TIMEOUT 1000 -/* - * Following global variables are defined here for the purpose of - * using existing Orch class which is to be refactored soon to - * eliminate the direct exposure of the global variables. - * - * Once Orch class refactoring is done, these global variables - * should be removed from here. - */ int gBatchSize = 0; -bool gSwssRecord = false; -bool gLogRotate = false; -ofstream gRecordOfs; -string gRecordFile; int main(int argc, char **argv) { diff --git a/cfgmgr/intfmgrd.cpp b/cfgmgr/intfmgrd.cpp index dcc4390ae5..bf13d61991 100644 --- a/cfgmgr/intfmgrd.cpp +++ b/cfgmgr/intfmgrd.cpp @@ -16,19 +16,7 @@ using namespace swss; /* select() function timeout retry time, in millisecond */ #define SELECT_TIMEOUT 1000 -/* - * Following global variables are defined here for the purpose of - * using existing Orch class which is to be refactored soon to - * eliminate the direct exposure of the global variables. - * - * Once Orch class refactoring is done, these global variables - * should be removed from here. - */ int gBatchSize = 0; -bool gSwssRecord = false; -bool gLogRotate = false; -ofstream gRecordOfs; -string gRecordFile; int main(int argc, char **argv) { diff --git a/cfgmgr/macsecmgrd.cpp b/cfgmgr/macsecmgrd.cpp index f575294512..055beb3bdb 100644 --- a/cfgmgr/macsecmgrd.cpp +++ b/cfgmgr/macsecmgrd.cpp @@ -24,21 +24,8 @@ using namespace swss; /* select() function timeout retry time, in millisecond */ #define SELECT_TIMEOUT 1000 -MacAddress gMacAddress; - -/* - * Following global variables are defined here for the purpose of - * using existing Orch class which is to be refactored soon to - * eliminate the direct exposure of the global variables. - * - * Once Orch class refactoring is done, these global variables - * should be removed from here. - */ int gBatchSize = 0; -bool gSwssRecord = false; -bool gLogRotate = false; -ofstream gRecordOfs; -string gRecordFile; +MacAddress gMacAddress; static bool received_sigterm = false; static struct sigaction old_sigaction; diff --git a/cfgmgr/natmgr.cpp b/cfgmgr/natmgr.cpp index 43077fbe32..d903544d9b 100644 --- a/cfgmgr/natmgr.cpp +++ b/cfgmgr/natmgr.cpp @@ -6129,7 +6129,7 @@ void NatMgr::doStaticNatTask(Consumer &consumer) else { SWSS_LOG_ERROR("Unknown operation type %s", op.c_str()); - SWSS_LOG_DEBUG("%s", (dumpTuple(consumer, t)).c_str()); + SWSS_LOG_DEBUG("%s", (consumer.dumpTuple(t)).c_str()); it = consumer.m_toSync.erase(it); } } @@ -6472,7 +6472,7 @@ void NatMgr::doStaticNaptTask(Consumer &consumer) else { SWSS_LOG_ERROR("Unknown operation type %s", op.c_str()); - SWSS_LOG_DEBUG("%s", (dumpTuple(consumer, t)).c_str()); + SWSS_LOG_DEBUG("%s", (consumer.dumpTuple(t)).c_str()); it = consumer.m_toSync.erase(it); } } @@ -6859,7 +6859,7 @@ void NatMgr::doNatPoolTask(Consumer &consumer) else { SWSS_LOG_ERROR("Unknown operation type %s", op.c_str()); - SWSS_LOG_DEBUG("%s", (dumpTuple(consumer, t)).c_str()); + SWSS_LOG_DEBUG("%s", (consumer.dumpTuple(t)).c_str()); it = consumer.m_toSync.erase(it); } } @@ -7095,7 +7095,7 @@ void NatMgr::doNatBindingTask(Consumer &consumer) else { SWSS_LOG_ERROR("Unknown operation type %s", op.c_str()); - SWSS_LOG_DEBUG("%s", (dumpTuple(consumer, t)).c_str()); + SWSS_LOG_DEBUG("%s", (consumer.dumpTuple(t)).c_str()); it = consumer.m_toSync.erase(it); } } @@ -7873,7 +7873,7 @@ void NatMgr::doNatAclTableTask(Consumer &consumer) else { SWSS_LOG_INFO("Unknown operation type %s", op.c_str()); - SWSS_LOG_DEBUG("%s", (dumpTuple(consumer, t)).c_str()); + SWSS_LOG_DEBUG("%s", (consumer.dumpTuple(t)).c_str()); it = consumer.m_toSync.erase(it); } } @@ -8137,7 +8137,7 @@ void NatMgr::doNatAclRuleTask(Consumer &consumer) else { SWSS_LOG_INFO("Unknown operation type %s", op.c_str()); - SWSS_LOG_DEBUG("%s", (dumpTuple(consumer, t)).c_str()); + SWSS_LOG_DEBUG("%s", (consumer.dumpTuple(t)).c_str()); it = consumer.m_toSync.erase(it); } } diff --git a/cfgmgr/natmgrd.cpp b/cfgmgr/natmgrd.cpp index 25a9e24773..421000d40c 100644 --- a/cfgmgr/natmgrd.cpp +++ b/cfgmgr/natmgrd.cpp @@ -39,19 +39,7 @@ using namespace swss; /* select() function timeout retry time, in millisecond */ #define SELECT_TIMEOUT 1000 -/* - * Following global variables are defined here for the purpose of - * using existing Orch class which is to be refactored soon to - * eliminate the direct exposure of the global variables. - * - * Once Orch class refactoring is done, these global variables - * should be removed from here. - */ -int gBatchSize = 0; -bool gSwssRecord = false; -bool gLogRotate = false; -ofstream gRecordOfs; -string gRecordFile; +int gBatchSize = 0; NatMgr *natmgr = NULL; NotificationConsumer *timeoutNotificationsConsumer = NULL; diff --git a/cfgmgr/nbrmgrd.cpp b/cfgmgr/nbrmgrd.cpp index 7cbc4ed938..7659367de3 100644 --- a/cfgmgr/nbrmgrd.cpp +++ b/cfgmgr/nbrmgrd.cpp @@ -20,19 +20,7 @@ using namespace swss; /* select() function timeout retry time, in millisecond */ #define SELECT_TIMEOUT 1000 -/* - * Following global variables are defined here for the purpose of - * using existing Orch class which is to be refactored soon to - * eliminate the direct exposure of the global variables. - * - * Once Orch class refactoring is done, these global variables - * should be removed from here. - */ int gBatchSize = 0; -bool gSwssRecord = false; -bool gLogRotate = false; -ofstream gRecordOfs; -string gRecordFile; int main(int argc, char **argv) { diff --git a/cfgmgr/portmgrd.cpp b/cfgmgr/portmgrd.cpp index 1942b8a313..7bfc969f88 100644 --- a/cfgmgr/portmgrd.cpp +++ b/cfgmgr/portmgrd.cpp @@ -15,19 +15,7 @@ using namespace swss; /* select() function timeout retry time, in millisecond */ #define SELECT_TIMEOUT 1000 -/* - * Following global variables are defined here for the purpose of - * using existing Orch class which is to be refactored soon to - * eliminate the direct exposure of the global variables. - * - * Once Orch class refactoring is done, these global variables - * should be removed from here. - */ int gBatchSize = 0; -bool gSwssRecord = false; -bool gLogRotate = false; -ofstream gRecordOfs; -string gRecordFile; int main(int argc, char **argv) { diff --git a/cfgmgr/sflowmgrd.cpp b/cfgmgr/sflowmgrd.cpp index 1a96503ddb..dd2b19c09e 100644 --- a/cfgmgr/sflowmgrd.cpp +++ b/cfgmgr/sflowmgrd.cpp @@ -15,19 +15,7 @@ using namespace swss; /* select() function timeout retry time, in millisecond */ #define SELECT_TIMEOUT 1000 -/* - * Following global variables are defined here for the purpose of - * using existing Orch class which is to be refactored soon to - * eliminate the direct exposure of the global variables. - * - * Once Orch class refactoring is done, these global variables - * should be removed from here. - */ int gBatchSize = 0; -bool gSwssRecord = false; -bool gLogRotate = false; -ofstream gRecordOfs; -string gRecordFile; int main(int argc, char **argv) { diff --git a/cfgmgr/teammgrd.cpp b/cfgmgr/teammgrd.cpp index 35ed35309b..e48ecf6063 100644 --- a/cfgmgr/teammgrd.cpp +++ b/cfgmgr/teammgrd.cpp @@ -13,11 +13,6 @@ using namespace swss; #define SELECT_TIMEOUT 1000 int gBatchSize = 0; -bool gSwssRecord = false; -bool gLogRotate = false; -ofstream gRecordOfs; -string gRecordFile; - bool received_sigterm = false; static struct sigaction old_sigaction; diff --git a/cfgmgr/tunnelmgrd.cpp b/cfgmgr/tunnelmgrd.cpp index 5cbf6e7a97..0990d492b8 100644 --- a/cfgmgr/tunnelmgrd.cpp +++ b/cfgmgr/tunnelmgrd.cpp @@ -19,19 +19,7 @@ using namespace swss; /* select() function timeout retry time, in millisecond */ #define SELECT_TIMEOUT 1000 -/* - * Following global variables are defined here for the purpose of - * using existing Orch class which is to be refactored soon to - * eliminate the direct exposure of the global variables. - * - * Once Orch class refactoring is done, these global variables - * should be removed from here. - */ int gBatchSize = 0; -bool gSwssRecord = false; -bool gLogRotate = false; -ofstream gRecordOfs; -string gRecordFile; int main(int argc, char **argv) { diff --git a/cfgmgr/vlanmgr.cpp b/cfgmgr/vlanmgr.cpp index e74582ce29..ee5b7a7067 100644 --- a/cfgmgr/vlanmgr.cpp +++ b/cfgmgr/vlanmgr.cpp @@ -431,13 +431,13 @@ void VlanMgr::doVlanTask(Consumer &consumer) { SWSS_LOG_ERROR("%s doesn't exist", key.c_str()); } - SWSS_LOG_DEBUG("%s", (dumpTuple(consumer, t)).c_str()); + SWSS_LOG_DEBUG("%s", (consumer.dumpTuple(t)).c_str()); it = consumer.m_toSync.erase(it); } else { SWSS_LOG_ERROR("Unknown operation type %s", op.c_str()); - SWSS_LOG_DEBUG("%s", (dumpTuple(consumer, t)).c_str()); + SWSS_LOG_DEBUG("%s", (consumer.dumpTuple(t)).c_str()); it = consumer.m_toSync.erase(it); } } @@ -539,7 +539,7 @@ void VlanMgr::processUntaggedVlanMembers(string vlan, const string &members) fvVector.push_back(t); KeyOpFieldsValuesTuple tuple = make_tuple(member_key, SET_COMMAND, fvVector); consumer.addToSync(tuple); - SWSS_LOG_DEBUG("%s", (dumpTuple(consumer, tuple)).c_str()); + SWSS_LOG_DEBUG("%s", (consumer.dumpTuple(tuple)).c_str()); } /* * There is pending task from consumer pipe, in this case just skip it. @@ -659,7 +659,7 @@ void VlanMgr::doVlanMemberTask(Consumer &consumer) { SWSS_LOG_DEBUG("%s doesn't exist", kfvKey(t).c_str()); } - SWSS_LOG_DEBUG("%s", (dumpTuple(consumer, t)).c_str()); + SWSS_LOG_DEBUG("%s", (consumer.dumpTuple(t)).c_str()); } else { diff --git a/cfgmgr/vlanmgrd.cpp b/cfgmgr/vlanmgrd.cpp index 89931f57d2..55f19d69ea 100644 --- a/cfgmgr/vlanmgrd.cpp +++ b/cfgmgr/vlanmgrd.cpp @@ -21,21 +21,8 @@ using namespace swss; /* select() function timeout retry time, in millisecond */ #define SELECT_TIMEOUT 1000 -MacAddress gMacAddress; - -/* - * Following global variables are defined here for the purpose of - * using existing Orch class which is to be refactored soon to - * eliminate the direct exposure of the global variables. - * - * Once Orch class refactoring is done, these global variables - * should be removed from here. - */ int gBatchSize = 0; -bool gSwssRecord = false; -bool gLogRotate = false; -ofstream gRecordOfs; -string gRecordFile; +MacAddress gMacAddress; int main(int argc, char **argv) { diff --git a/cfgmgr/vrfmgrd.cpp b/cfgmgr/vrfmgrd.cpp index 44f9292741..ed4db3a12d 100644 --- a/cfgmgr/vrfmgrd.cpp +++ b/cfgmgr/vrfmgrd.cpp @@ -16,20 +16,7 @@ using namespace swss; /* select() function timeout retry time, in millisecond */ #define SELECT_TIMEOUT 1000 -/* - * Following global variables are defined here for the purpose of - * using existing Orch class which is to be refactored soon to - * eliminate the direct exposure of the global variables. - * - * Once Orch class refactoring is done, these global variables - * should be removed from here. - */ int gBatchSize = 0; -bool gSwssRecord = false; -bool gLogRotate = false; -ofstream gRecordOfs; -string gRecordFile; - int main(int argc, char **argv) { diff --git a/cfgmgr/vxlanmgrd.cpp b/cfgmgr/vxlanmgrd.cpp index b6ec40e7d4..404aa386a2 100644 --- a/cfgmgr/vxlanmgrd.cpp +++ b/cfgmgr/vxlanmgrd.cpp @@ -21,19 +21,7 @@ using namespace swss; /* select() function timeout retry time, in millisecond */ #define SELECT_TIMEOUT 1000 -/* - * Following global variables are defined here for the purpose of - * using existing Orch class which is to be refactored soon to - * eliminate the direct exposure of the global variables. - * - * Once Orch class refactoring is done, these global variables - * should be removed from here. - */ int gBatchSize = 0; -bool gSwssRecord = false; -bool gLogRotate = false; -ofstream gRecordOfs; -string gRecordFile; MacAddress gMacAddress; int main(int argc, char **argv) diff --git a/orchagent/recorder.cpp b/lib/recorder.cpp similarity index 83% rename from orchagent/recorder.cpp rename to lib/recorder.cpp index 7985f8f648..4735aae4eb 100644 --- a/orchagent/recorder.cpp +++ b/lib/recorder.cpp @@ -1,6 +1,7 @@ #include "recorder.h" +#include "timestamp.h" #include "logger.h" -#include +#include using namespace swss; @@ -10,6 +11,9 @@ const std::string Recorder::SWSS_FNAME = "swss.rec"; const std::string Recorder::SAIREDIS_FNAME = "sairedis.rec"; const std::string Recorder::RESPPUB_FNAME = "responsepublisher.rec"; +std::unique_ptr Recorder::swss = std::make_unique(); +std::unique_ptr Recorder::sairedis = std::make_unique(); +std::unique_ptr Recorder::respub = std::make_unique(); SwSSRec::SwSSRec() { @@ -67,7 +71,7 @@ void RecWriter::startRec(bool exit_if_failure) setRecord(false); } } - record_ofs << getTimestamp() << Recorder::REC_START << std::endl; + record_ofs << swss::getTimestamp() << Recorder::REC_START << std::endl; } @@ -82,7 +86,11 @@ RecWriter::~RecWriter() void RecWriter::record(const std::string& val) { - record_ofs << getTimestamp() << "|" << val << std::endl; + if (!isRecord()) + { + return ; + } + record_ofs << swss::getTimestamp() << "|" << val << std::endl; if (isRotate()) { setRotate(false); @@ -98,14 +106,12 @@ void RecWriter::logfileReopen() return ; } - record_ofs.close(); - /* * On log rotate we will use the same file name, we are assuming that * logrotate daemon move filename to filename.1 and we will create new * empty file here. */ - + record_ofs.close(); record_ofs.open(fname, std::ofstream::out | std::ofstream::app); if (!record_ofs.is_open()) @@ -113,4 +119,4 @@ void RecWriter::logfileReopen() SWSS_LOG_ERROR("Failed to open recording file %s: %s", fname.c_str(), strerror(errno)); return; } -} \ No newline at end of file +} diff --git a/orchagent/recorder.h b/lib/recorder.h similarity index 88% rename from orchagent/recorder.h rename to lib/recorder.h index 8f5dc6bef5..51e4843a8e 100644 --- a/orchagent/recorder.h +++ b/lib/recorder.h @@ -4,23 +4,10 @@ #include #include #include -#include +#include namespace swss { -class Recorder { -public: - static const std::string DEFAULT_DIR; - static const std::string REC_START; - static const std::string SWSS_FNAME; - static const std::string SAIREDIS_FNAME; - static const std::string RESPPUB_FNAME; - - static SwSSRec swss; - static SaiRedisRec sairedis; - static ResPubRec respub; -}; - class RecBase { public: RecBase() {} @@ -72,4 +59,18 @@ class SaiRedisRec : public RecBase { SaiRedisRec(); }; +/* Interface to access recorder classes */ +class Recorder { +public: + static const std::string DEFAULT_DIR; + static const std::string REC_START; + static const std::string SWSS_FNAME; + static const std::string SAIREDIS_FNAME; + static const std::string RESPPUB_FNAME; + + static std::unique_ptr swss; + static std::unique_ptr sairedis; + static std::unique_ptr respub; +}; + } \ No newline at end of file diff --git a/orchagent/Makefile.am b/orchagent/Makefile.am index 36d659071f..4a065289fb 100644 --- a/orchagent/Makefile.am +++ b/orchagent/Makefile.am @@ -45,6 +45,7 @@ orchagent_SOURCES = \ main.cpp \ $(top_srcdir)/lib/gearboxutils.cpp \ $(top_srcdir)/lib/subintf.cpp \ + $(top_srcdir)/lib/recorder.cpp \ orchdaemon.cpp \ orch.cpp \ notifications.cpp \ @@ -98,7 +99,6 @@ orchagent_SOURCES = \ bfdorch.cpp \ srv6orch.cpp \ response_publisher.cpp \ - recorder.cpp \ nvgreorch.cpp orchagent_SOURCES += flex_counter/flex_counter_manager.cpp flex_counter/flex_counter_stat_manager.cpp flex_counter/flow_counter_handler.cpp flex_counter/flowcounterrouteorch.cpp diff --git a/orchagent/main.cpp b/orchagent/main.cpp index f5d3edf997..2213fddfe3 100644 --- a/orchagent/main.cpp +++ b/orchagent/main.cpp @@ -19,8 +19,6 @@ extern "C" { #include #include -#include "timestamp.h" -s #include #include @@ -54,17 +52,12 @@ extern size_t gMaxBulkSize; #define DEFAULT_BATCH_SIZE 128 int gBatchSize = DEFAULT_BATCH_SIZE; -bool gSwssRecord = true; -bool gLogRotate = false; bool gSyncMode = false; sai_redis_communication_mode_t gRedisCommunicationMode = SAI_REDIS_COMMUNICATION_MODE_REDIS_ASYNC; string gAsicInstance; extern bool gIsNatSupported; -ofstream gRecordOfs; -string gRecordFile; - #define SAIREDIS_RECORD_ENABLE 0x1 #define SWSS_RECORD_ENABLE (0x1 << 1) #define RESPONSE_PUBLISHER_RECORD_ENABLE (0x1 << 2) @@ -103,9 +96,9 @@ void sighup_handler(int signo) /* * Don't do any logging since they are using mutexes. */ - gLogRotate = true; - Recorder::sairedis.setRotate(true); - Recorder::respub.setRotate(true); + Recorder::swss->setRotate(true); + Recorder::sairedis->setRotate(true); + Recorder::respub->setRotate(true); } void syncd_apply_view() @@ -341,10 +334,10 @@ int main(int argc, char **argv) int opt; sai_status_t status; - string record_location = "."; - string swss_rec_filename = "swss.rec"; - string sairedis_rec_filename = "sairedis.rec"; - string responsepublisher_rec_filename = "responsepublisher.rec"; + string record_location = Recorder::DEFAULT_DIR; + string swss_rec_filename = Recorder::SWSS_FNAME; + string sairedis_rec_filename = Recorder::SAIREDIS_FNAME; + string responsepublisher_rec_filename = Recorder::RESPPUB_FNAME; int record_type = 3; // Only swss and sairedis recordings enabled by default. while ((opt = getopt(argc, argv, "b:m:r:f:j:d:i:hsz:k:")) != -1) @@ -432,14 +425,30 @@ int main(int argc, char **argv) SWSS_LOG_NOTICE("--- Starting Orchestration Agent ---"); - Recorder::sairedis.enable( + /* Initialize sairedis recording parameters */ + Recorder::sairedis->setRecord( (record_type & SAIREDIS_RECORD_ENABLE) == SAIREDIS_RECORD_ENABLE ); - Recorder::sairedis.setLocation(record_location); - Recorder::sairedis.setFileName(sairedis_rec_filename); + Recorder::sairedis->setLocation(record_location); + Recorder::sairedis->setFileName(sairedis_rec_filename); + /* Initialize sairedis */ initSaiApi(); - initSaiRedis(record_location, sairedis_rec_filename); + initSaiRedis(); + + /* Initialize remaining recorder parameters */ + Recorder::swss->setRecord( + (record_type & SWSS_RECORD_ENABLE) == SWSS_RECORD_ENABLE + ); + Recorder::swss->setLocation(record_location); + Recorder::swss->setFileName(swss_rec_filename); + + Recorder::respub->setRecord( + (record_type & RESPONSE_PUBLISHER_RECORD_ENABLE) == + RESPONSE_PUBLISHER_RECORD_ENABLE + ); + Recorder::respub->setLocation(record_location); + Recorder::respub->setFileName(responsepublisher_rec_filename); sai_attribute_t attr; vector attrs; @@ -451,34 +460,15 @@ int main(int argc, char **argv) attr.value.ptr = (void *)on_fdb_event; attrs.push_back(attr); - // Initialize recording parameters. - Recorder::sairedis.enable( - (record_type & SAIREDIS_RECORD_ENABLE) == SAIREDIS_RECORD_ENABLE - ); - Recorder::respub.enable( - (record_type & RESPONSE_PUBLISHER_RECORD_ENABLE) == - RESPONSE_PUBLISHER_RECORD_ENABLE - ); - gSwssRecord = (record_type & SWSS_RECORD_ENABLE) == SWSS_RECORD_ENABLE; - /* Disable/enable SwSS recording */ - if (gSwssRecord) + if (Recorder::swss->isRecord()) { - gRecordFile = record_location + "/" + swss_rec_filename; - gRecordOfs.open(gRecordFile, std::ofstream::out | std::ofstream::app); - if (!gRecordOfs.is_open()) - { - SWSS_LOG_ERROR("Failed to open SwSS recording file %s", gRecordFile.c_str()); - exit(EXIT_FAILURE); - } - gRecordOfs << getTimestamp() << "|recording started" << endl; + Recorder::swss->startRec(true); } - if (Recorder::respub.isRecord()) + if (Recorder::respub->isRecord()) { - Recorder::respub.setLocation(record_location); - Recorder::respub.setFileName(responsepublisher_rec_filename); - Recorder::respub.startRec(false); + Recorder::respub->startRec(false); } attr.id = SAI_SWITCH_ATTR_PORT_STATE_CHANGE_NOTIFY; diff --git a/orchagent/orch.cpp b/orchagent/orch.cpp index be73795fae..7c913a13c2 100644 --- a/orchagent/orch.cpp +++ b/orchagent/orch.cpp @@ -15,11 +15,6 @@ using namespace swss; extern int gBatchSize; -extern bool gSwssRecord; -extern ofstream gRecordOfs; -extern bool gLogRotate; -extern string gRecordFile; - Orch::Orch(DBConnector *db, const string tableName, int pri) { addConsumer(db, tableName, pri); @@ -49,14 +44,6 @@ Orch::Orch(const vector& tables) } } -Orch::~Orch() -{ - if (gRecordOfs.is_open()) - { - gRecordOfs.close(); - } -} - vector Orch::getSelectables() { vector selectables; @@ -71,14 +58,13 @@ void ConsumerBase::addToSync(const KeyOpFieldsValuesTuple &entry) { SWSS_LOG_ENTER(); - string key = kfvKey(entry); string op = kfvOp(entry); /* Record incoming tasks */ - if (gSwssRecord) + if (Recorder::swss->isRecord()) { - Orch::recordTuple(*this, entry); + recordTuple(entry); } /* @@ -215,6 +201,11 @@ size_t Consumer::refillToSync() } } +void ConsumerBase::recordTuple(const KeyOpFieldsValuesTuple &tuple) +{ + Recorder::swss->record(this->dumpTuple(tuple)); +} + string ConsumerBase::dumpTuple(const KeyOpFieldsValuesTuple &tuple) { string s = getTableName() + getConsumerTable()->getTableNameSeparator() + kfvKey(tuple) @@ -563,45 +554,6 @@ void Orch::flushResponses() m_publisher.flush(); } -void Orch::logfileReopen() -{ - gRecordOfs.close(); - - /* - * On log rotate we will use the same file name, we are assuming that - * logrotate daemon move filename to filename.1 and we will create new - * empty file here. - */ - - gRecordOfs.open(gRecordFile, std::ofstream::out | std::ofstream::app); - - if (!gRecordOfs.is_open()) - { - SWSS_LOG_ERROR("failed to open gRecordOfs file %s: %s", gRecordFile.c_str(), strerror(errno)); - return; - } -} - -void Orch::recordTuple(ConsumerBase &consumer, const KeyOpFieldsValuesTuple &tuple) -{ - string s = consumer.dumpTuple(tuple); - - gRecordOfs << getTimestamp() << "|" << s << endl; - - if (gLogRotate) - { - gLogRotate = false; - - logfileReopen(); - } -} - -string Orch::dumpTuple(Consumer &consumer, const KeyOpFieldsValuesTuple &tuple) -{ - string s = consumer.dumpTuple(tuple); - return s; -} - ref_resolve_status Orch::resolveFieldRefArray( type_map &type_maps, const string &field_name, diff --git a/orchagent/orch.h b/orchagent/orch.h index 35bcb25bc6..628764c3f9 100644 --- a/orchagent/orch.h +++ b/orchagent/orch.h @@ -154,6 +154,9 @@ class ConsumerBase : public Executor { // TODO: hide? SyncMap m_toSync; + /* record the tuple */ + void recordTuple(const swss::KeyOpFieldsValuesTuple &tuple); + void addToSync(const swss::KeyOpFieldsValuesTuple &entry); // Returns: the number of entries added to m_toSync @@ -217,7 +220,7 @@ class Orch Orch(swss::DBConnector *db, const std::vector &tableNames); Orch(swss::DBConnector *db, const std::vector &tableNameWithPri); Orch(const std::vector& tables); - virtual ~Orch(); + virtual ~Orch() {}; std::vector getSelectables(); @@ -237,9 +240,6 @@ class Orch virtual void doTask(swss::NotificationConsumer &consumer) { } virtual void doTask(swss::SelectableTimer &timer) { } - /* TODO: refactor recording */ - static void recordTuple(ConsumerBase &consumer, const swss::KeyOpFieldsValuesTuple &tuple); - void dumpPendingTasks(std::vector &ts); /** @@ -249,8 +249,6 @@ class Orch protected: ConsumerMap m_consumerMap; - static void logfileReopen(); - std::string dumpTuple(Consumer &consumer, const swss::KeyOpFieldsValuesTuple &tuple); ref_resolve_status resolveFieldRefValue(type_map&, const std::string&, const std::string&, swss::KeyOpFieldsValuesTuple&, sai_object_id_t&, std::string&); std::set generateIdListFromMap(unsigned long idsMap, sai_uint32_t maxId); unsigned long generateBitMapFromIdsStr(const std::string &idsStr); diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index 36f293ce88..251c99d75b 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -24,7 +24,6 @@ using namespace swss; extern sai_switch_api_t* sai_switch_api; extern sai_object_id_t gSwitchId; -extern bool gSaiRedisLogRotate; extern void syncd_apply_view(); /* @@ -62,7 +61,6 @@ DebugCounterOrch *gDebugCounterOrch; MonitorOrch *gMonitorOrch; bool gIsNatSupported = false; -bool gSaiRedisLogRotate = false; event_handle_t g_events_handle; #define DEFAULT_MAX_BULK_SIZE 1000 @@ -710,7 +708,8 @@ void OrchDaemon::logRotate() { void OrchDaemon::start() { SWSS_LOG_ENTER(); - gSaiRedisLogRotate = false; + + Recorder::sairedis->setRotate(false); for (Orch *o : m_orchList) { @@ -756,12 +755,10 @@ void OrchDaemon::start() } // check if logroate is requested - if (gSaiRedisLogRotate) + if (Recorder::sairedis->isRotate()) { SWSS_LOG_NOTICE("performing log rotate"); - - gSaiRedisLogRotate = false; - + Recorder::sairedis->setRotate(false); logRotate(); } diff --git a/orchagent/orchdaemon.h b/orchagent/orchdaemon.h index 6092be26b4..32fab2b9b0 100644 --- a/orchagent/orchdaemon.h +++ b/orchagent/orchdaemon.h @@ -48,7 +48,6 @@ #include using namespace swss; -extern bool gSaiRedisLogRotate; class OrchDaemon { diff --git a/orchagent/p4orch/tests/Makefile.am b/orchagent/p4orch/tests/Makefile.am index 2bf6dae443..aee7afe603 100644 --- a/orchagent/p4orch/tests/Makefile.am +++ b/orchagent/p4orch/tests/Makefile.am @@ -28,7 +28,7 @@ p4orch_tests_SOURCES = $(ORCHAGENT_DIR)/orch.cpp \ $(ORCHAGENT_DIR)/copporch.cpp \ $(ORCHAGENT_DIR)/switchorch.cpp \ $(ORCHAGENT_DIR)/request_parser.cpp \ - $(ORCHAGENT_DIR)/recorder.cpp \ + $(top_srcdir)/lib/recorder.cpp \ $(ORCHAGENT_DIR)/flex_counter/flex_counter_manager.cpp \ $(ORCHAGENT_DIR)/flex_counter/flow_counter_handler.cpp \ $(P4ORCH_DIR)/p4oidmapper.cpp \ diff --git a/orchagent/p4orch/tests/test_main.cpp b/orchagent/p4orch/tests/test_main.cpp index cb6e27cdb2..f780dcdae6 100644 --- a/orchagent/p4orch/tests/test_main.cpp +++ b/orchagent/p4orch/tests/test_main.cpp @@ -40,8 +40,6 @@ sai_object_id_t gUnderlayIfId; int gBatchSize = DEFAULT_BATCH_SIZE; #define DEFAULT_MAX_BULK_SIZE 1000 size_t gMaxBulkSize = DEFAULT_MAX_BULK_SIZE; -bool gSwssRecord = true; -bool gLogRotate = false; bool gSyncMode = false; bool gIsNatSupported = false; @@ -52,8 +50,6 @@ VRFOrch *gVrfOrch; FlowCounterRouteOrch *gFlowCounterRouteOrch; SwitchOrch *gSwitchOrch; Directory gDirectory; -ofstream gRecordOfs; -string gRecordFile; swss::DBConnector *gAppDb; swss::DBConnector *gStateDb; swss::DBConnector *gConfigDb; diff --git a/orchagent/response_publisher.cpp b/orchagent/response_publisher.cpp index 6e1cd64273..a2f9089d77 100644 --- a/orchagent/response_publisher.cpp +++ b/orchagent/response_publisher.cpp @@ -32,13 +32,13 @@ std::string PrependedComponent(const ReturnCode &status) void PerformLogRotate() { - Recorder::respub.logfileReopen(); + swss::Recorder::respub->logfileReopen(); } void RecordDBWrite(const std::string &table, const std::string &key, const std::vector &attrs, const std::string &op) { - if (!Recorder::respub.isRecord()) + if (!swss::Recorder::respub->isRecord()) { return; } @@ -50,13 +50,13 @@ void RecordDBWrite(const std::string &table, const std::string &key, const std:: } PerformLogRotate(); - Recorder::respub.record(s); + swss::Recorder::respub->record(s); } void RecordResponse(const std::string &response_channel, const std::string &key, const std::vector &attrs, const std::string &status) { - if (!Recorder::respub.isRecord()) + if (!swss::Recorder::respub->isRecord()) { return; } @@ -68,7 +68,7 @@ void RecordResponse(const std::string &response_channel, const std::string &key, } PerformLogRotate(); - Recorder::respub.record(s); + swss::Recorder::respub->record(s); } } // namespace diff --git a/orchagent/saihelper.cpp b/orchagent/saihelper.cpp index c4861f8e39..3fcc27a56d 100644 --- a/orchagent/saihelper.cpp +++ b/orchagent/saihelper.cpp @@ -251,11 +251,11 @@ void initSaiRedis() sai_attribute_t attr; sai_status_t status; - auto record_filename = Recorder::sairedis.getFile(); - auto record_location = Recorder::sairedis.getLoc(); + auto record_filename = Recorder::sairedis->getFile(); + auto record_location = Recorder::sairedis->getLoc(); /* set recording dir before enable recording */ - if (Recorder::sairedis.isRecord()) + if (Recorder::sairedis->isRecord()) { attr.id = SAI_REDIS_SWITCH_ATTR_RECORDING_OUTPUT_DIR; attr.value.s8list.count = (uint32_t)record_location.size(); @@ -284,15 +284,14 @@ void initSaiRedis() } /* Disable/enable SAI Redis recording */ - attr.id = SAI_REDIS_SWITCH_ATTR_RECORD; - attr.value.booldata = Recorder::sairedis.isRecord(); + attr.value.booldata = Recorder::sairedis->isRecord(); status = sai_switch_api->set_switch_attribute(gSwitchId, &attr); if (status != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("Failed to %s SAI Redis recording, rv:%d", - Recorder::sairedis.isRecord() ? "enable" : "disable", status); + Recorder::sairedis->isRecord() ? "enable" : "disable", status); exit(EXIT_FAILURE); } diff --git a/tests/mock_tests/Makefile.am b/tests/mock_tests/Makefile.am index e06d75f2cc..55b6116730 100644 --- a/tests/mock_tests/Makefile.am +++ b/tests/mock_tests/Makefile.am @@ -56,7 +56,7 @@ tests_SOURCES = aclorch_ut.cpp \ test_failure_handling.cpp \ $(top_srcdir)/lib/gearboxutils.cpp \ $(top_srcdir)/lib/subintf.cpp \ - $(top_srcdir)/orchagent/recorder.cpp \ + $(top_srcdir)/lib/recorder.cpp \ $(top_srcdir)/orchagent/orchdaemon.cpp \ $(top_srcdir)/orchagent/orch.cpp \ $(top_srcdir)/orchagent/notifications.cpp \ @@ -143,6 +143,7 @@ tests_LDADD = $(LDADD_GTEST) $(LDADD_SAI) -lnl-genl-3 -lhiredis -lhiredis -lpthr ## portsyncd unit tests tests_portsyncd_SOURCES = portsyncd/portsyncd_ut.cpp \ + $(top_srcdir)/lib/recorder.cpp \ $(top_srcdir)/portsyncd/linksync.cpp \ mock_dbconnector.cpp \ common/mock_shell_command.cpp \ @@ -150,7 +151,7 @@ tests_portsyncd_SOURCES = portsyncd/portsyncd_ut.cpp \ mock_hiredis.cpp \ mock_redisreply.cpp -tests_portsyncd_INCLUDES = -I $(top_srcdir)/portsyncd -I $(top_srcdir)/cfgmgr +tests_portsyncd_INCLUDES = -I $(top_srcdir)/portsyncd -I $(top_srcdir)/cfgmgr -I $(top_srcdir)/lib tests_portsyncd_CXXFLAGS = -Wl,-wrap,if_nameindex -Wl,-wrap,if_freenameindex tests_portsyncd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) tests_portsyncd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(tests_portsyncd_INCLUDES) @@ -162,6 +163,7 @@ tests_portsyncd_LDADD = $(LDADD_GTEST) -lnl-genl-3 -lhiredis -lhiredis \ tests_intfmgrd_SOURCES = intfmgrd/intfmgr_ut.cpp \ $(top_srcdir)/cfgmgr/intfmgr.cpp \ $(top_srcdir)/lib/subintf.cpp \ + $(top_srcdir)/lib/recorder.cpp \ $(top_srcdir)/orchagent/orch.cpp \ $(top_srcdir)/orchagent/request_parser.cpp \ mock_orchagent_main.cpp \ @@ -202,6 +204,7 @@ tests_fpmsyncd_LDADD = $(LDADD_GTEST) $(LDADD_SAI) -lnl-genl-3 -lhiredis -lhired tests_response_publisher_SOURCES = response_publisher/response_publisher_ut.cpp \ $(top_srcdir)/orchagent/response_publisher.cpp \ + $(top_srcdir)/lib/recorder.cpp \ mock_orchagent_main.cpp \ mock_dbconnector.cpp \ mock_table.cpp \ From aefdb0baf2f680e7dbc3205e602ce852c9a1203f Mon Sep 17 00:00:00 2001 From: Vivek Reddy Karri Date: Wed, 21 Jun 2023 06:13:19 +0000 Subject: [PATCH 4/9] Handle build failures Signed-off-by: Vivek Reddy Karri --- lib/recorder.h | 2 +- orchagent/response_publisher.cpp | 2 -- tests/Makefile.am | 4 +++- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/recorder.h b/lib/recorder.h index 51e4843a8e..0253e7b870 100644 --- a/lib/recorder.h +++ b/lib/recorder.h @@ -73,4 +73,4 @@ class Recorder { static std::unique_ptr respub; }; -} \ No newline at end of file +} diff --git a/orchagent/response_publisher.cpp b/orchagent/response_publisher.cpp index a2f9089d77..ee17742878 100644 --- a/orchagent/response_publisher.cpp +++ b/orchagent/response_publisher.cpp @@ -5,8 +5,6 @@ #include #include -#include "timestamp.h" - namespace { diff --git a/tests/Makefile.am b/tests/Makefile.am index 0b6831be97..8f2aa131c4 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1,3 +1,5 @@ +INCLUDES = -I $(top_srcdir)/lib + CFLAGS_SAI = -I /usr/include/sai TESTS = tests @@ -18,7 +20,7 @@ CFLAGS_GTEST = LDADD_GTEST = -L/usr/src/gtest tests_SOURCES = swssnet_ut.cpp request_parser_ut.cpp ../orchagent/request_parser.cpp \ - quoted_ut.cpp + quoted_ut.cpp ../lib/recorder.cpp tests_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI) tests_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI) -I../orchagent From 9500bc86db7e0bdf1e8c55270af02cbeba326ce8 Mon Sep 17 00:00:00 2001 From: Vivek Reddy Karri Date: Wed, 21 Jun 2023 23:02:14 +0000 Subject: [PATCH 5/9] Refactoring recorder Signed-off-by: Vivek Reddy Karri --- lib/recorder.cpp | 20 ++++++++------------ lib/recorder.h | 7 ++++++- orchagent/main.cpp | 13 ++----------- orchagent/orch.cpp | 10 +--------- orchagent/response_publisher.cpp | 7 ------- 5 files changed, 17 insertions(+), 40 deletions(-) diff --git a/lib/recorder.cpp b/lib/recorder.cpp index 4735aae4eb..87903be38d 100644 --- a/lib/recorder.cpp +++ b/lib/recorder.cpp @@ -22,6 +22,7 @@ SwSSRec::SwSSRec() setRotate(false); setLocation(Recorder::DEFAULT_DIR); setFileName(Recorder::SWSS_FNAME); + setName("SwSS"); } @@ -32,6 +33,7 @@ ResPubRec::ResPubRec() setRotate(false); setLocation(Recorder::DEFAULT_DIR); setFileName(Recorder::RESPPUB_FNAME); + setName("Response Publisher"); } @@ -42,6 +44,7 @@ SaiRedisRec::SaiRedisRec() setRotate(false); setLocation(Recorder::DEFAULT_DIR); setFileName(Recorder::SAIREDIS_FNAME); + setName("SaiRedis"); } @@ -52,16 +55,11 @@ void RecWriter::startRec(bool exit_if_failure) return ; } - if (record_ofs.is_open()) - { - SWSS_LOG_ERROR("Record File %s is already open", fname.c_str()); - } - fname = getLoc() + "/" + getFile(); record_ofs.open(fname, std::ofstream::out | std::ofstream::app); if (!record_ofs.is_open()) { - SWSS_LOG_ERROR("Failed to open recording file %s: %s", fname.c_str(), strerror(errno)); + SWSS_LOG_ERROR("%s Recorder: Failed to open recording file %s: error %s", getName().c_str(), fname.c_str(), strerror(errno)); if (exit_if_failure) { exit(EXIT_FAILURE); @@ -72,6 +70,7 @@ void RecWriter::startRec(bool exit_if_failure) } } record_ofs << swss::getTimestamp() << Recorder::REC_START << std::endl; + SWSS_LOG_NOTICE("%s Recorder: Recording started at %s", getName().c_str(), fname.c_str()); } @@ -93,6 +92,7 @@ void RecWriter::record(const std::string& val) record_ofs << swss::getTimestamp() << "|" << val << std::endl; if (isRotate()) { + /* If the log rotate is set by sighup handler, peform the following actions */ setRotate(false); logfileReopen(); } @@ -101,11 +101,6 @@ void RecWriter::record(const std::string& val) void RecWriter::logfileReopen() { - if (!isRecord()) - { - return ; - } - /* * On log rotate we will use the same file name, we are assuming that * logrotate daemon move filename to filename.1 and we will create new @@ -116,7 +111,8 @@ void RecWriter::logfileReopen() if (!record_ofs.is_open()) { - SWSS_LOG_ERROR("Failed to open recording file %s: %s", fname.c_str(), strerror(errno)); + SWSS_LOG_ERROR("%s Recorder: Failed to open file %s: %s", getName().c_str(), fname.c_str(), strerror(errno)); return; } + SWSS_LOG_INFO("%s Recorder: LogRotate request handled", getName().c_str()); } diff --git a/lib/recorder.h b/lib/recorder.h index 0253e7b870..104dfa9956 100644 --- a/lib/recorder.h +++ b/lib/recorder.h @@ -16,18 +16,21 @@ class RecBase { void setRotate(bool rotate) { log_rotate = rotate; } void setLocation(const std::string& loc) { location = loc; } void setFileName(const std::string& name) { filename = name; } + void setName(const std::string& name) { m_name = name; } /* getters */ bool isRecord() { return enable_rec; } bool isRotate() { return log_rotate; } std::string getLoc() { return location; } std::string getFile() { return filename; } + std::string getName() { return m_name; } private: bool enable_rec; + bool log_rotate; std::string location; std::string filename; - bool log_rotate; + std::string m_name; }; class RecWriter : public RecBase { @@ -36,6 +39,8 @@ class RecWriter : public RecBase { virtual ~RecWriter(); void startRec(bool exit_if_failure); void record(const std::string& val); + +protected: void logfileReopen(); private: diff --git a/orchagent/main.cpp b/orchagent/main.cpp index 2213fddfe3..70e0d1042c 100644 --- a/orchagent/main.cpp +++ b/orchagent/main.cpp @@ -442,6 +442,7 @@ int main(int argc, char **argv) ); Recorder::swss->setLocation(record_location); Recorder::swss->setFileName(swss_rec_filename); + Recorder::swss->startRec(true); Recorder::respub->setRecord( (record_type & RESPONSE_PUBLISHER_RECORD_ENABLE) == @@ -449,6 +450,7 @@ int main(int argc, char **argv) ); Recorder::respub->setLocation(record_location); Recorder::respub->setFileName(responsepublisher_rec_filename); + Recorder::respub->startRec(false); sai_attribute_t attr; vector attrs; @@ -460,17 +462,6 @@ int main(int argc, char **argv) attr.value.ptr = (void *)on_fdb_event; attrs.push_back(attr); - /* Disable/enable SwSS recording */ - if (Recorder::swss->isRecord()) - { - Recorder::swss->startRec(true); - } - - if (Recorder::respub->isRecord()) - { - Recorder::respub->startRec(false); - } - attr.id = SAI_SWITCH_ATTR_PORT_STATE_CHANGE_NOTIFY; attr.value.ptr = (void *)on_port_state_change; attrs.push_back(attr); diff --git a/orchagent/orch.cpp b/orchagent/orch.cpp index 7c913a13c2..d74b1d4342 100644 --- a/orchagent/orch.cpp +++ b/orchagent/orch.cpp @@ -62,10 +62,7 @@ void ConsumerBase::addToSync(const KeyOpFieldsValuesTuple &entry) string op = kfvOp(entry); /* Record incoming tasks */ - if (Recorder::swss->isRecord()) - { - recordTuple(entry); - } + Recorder::swss->record(dumpTuple(entry)); /* * m_toSync is a multimap which will allow one key with multiple values, @@ -201,11 +198,6 @@ size_t Consumer::refillToSync() } } -void ConsumerBase::recordTuple(const KeyOpFieldsValuesTuple &tuple) -{ - Recorder::swss->record(this->dumpTuple(tuple)); -} - string ConsumerBase::dumpTuple(const KeyOpFieldsValuesTuple &tuple) { string s = getTableName() + getConsumerTable()->getTableNameSeparator() + kfvKey(tuple) diff --git a/orchagent/response_publisher.cpp b/orchagent/response_publisher.cpp index ee17742878..2b7fafa50a 100644 --- a/orchagent/response_publisher.cpp +++ b/orchagent/response_publisher.cpp @@ -28,11 +28,6 @@ std::string PrependedComponent(const ReturnCode &status) return kOrchagentComponent; } -void PerformLogRotate() -{ - swss::Recorder::respub->logfileReopen(); -} - void RecordDBWrite(const std::string &table, const std::string &key, const std::vector &attrs, const std::string &op) { @@ -47,7 +42,6 @@ void RecordDBWrite(const std::string &table, const std::string &key, const std:: s += "|" + fvField(attr) + ":" + fvValue(attr); } - PerformLogRotate(); swss::Recorder::respub->record(s); } @@ -65,7 +59,6 @@ void RecordResponse(const std::string &response_channel, const std::string &key, s += "|" + fvField(attr) + ":" + fvValue(attr); } - PerformLogRotate(); swss::Recorder::respub->record(s); } From 11089de246ac217d72d52de910b4d6e4ddef0d19 Mon Sep 17 00:00:00 2001 From: Vivek Reddy Karri Date: Fri, 23 Jun 2023 01:29:57 +0000 Subject: [PATCH 6/9] Removed the explicit definiton of gBatchSize from the cfgmgrs Signed-off-by: Vivek Reddy Karri --- cfgmgr/buffermgrd.cpp | 2 -- cfgmgr/coppmgrd.cpp | 2 -- cfgmgr/intfmgrd.cpp | 2 -- cfgmgr/macsecmgrd.cpp | 1 - cfgmgr/natmgrd.cpp | 1 - cfgmgr/nbrmgrd.cpp | 2 -- cfgmgr/portmgrd.cpp | 2 -- cfgmgr/sflowmgrd.cpp | 2 -- cfgmgr/teammgrd.cpp | 1 - cfgmgr/tunnelmgrd.cpp | 2 -- cfgmgr/vlanmgrd.cpp | 1 - cfgmgr/vrfmgrd.cpp | 2 -- cfgmgr/vxlanmgrd.cpp | 1 - orchagent/main.cpp | 3 ++- orchagent/orch.cpp | 2 +- orchagent/p4orch/tests/test_main.cpp | 4 ++-- tests/mock_tests/mock_orchagent_main.cpp | 7 ------- tests/mock_tests/mock_orchagent_main.h | 4 ---- 18 files changed, 5 insertions(+), 36 deletions(-) diff --git a/cfgmgr/buffermgrd.cpp b/cfgmgr/buffermgrd.cpp index 7032b2939e..fddaac930b 100644 --- a/cfgmgr/buffermgrd.cpp +++ b/cfgmgr/buffermgrd.cpp @@ -21,8 +21,6 @@ using json = nlohmann::json; /* SELECT() function timeout retry time, in millisecond */ #define SELECT_TIMEOUT 1000 -int gBatchSize = 0; - void usage() { cout << "Usage: buffermgrd <-l pg_lookup.ini|-a asic_table.json [-p peripheral_table.json] [-z zero_profiles.json]>" << endl; diff --git a/cfgmgr/coppmgrd.cpp b/cfgmgr/coppmgrd.cpp index fbe66aa548..16c15c1238 100644 --- a/cfgmgr/coppmgrd.cpp +++ b/cfgmgr/coppmgrd.cpp @@ -16,8 +16,6 @@ using namespace swss; /* select() function timeout retry time, in millisecond */ #define SELECT_TIMEOUT 1000 -int gBatchSize = 0; - int main(int argc, char **argv) { Logger::linkToDbNative("coppmgrd"); diff --git a/cfgmgr/intfmgrd.cpp b/cfgmgr/intfmgrd.cpp index bf13d61991..e414590920 100644 --- a/cfgmgr/intfmgrd.cpp +++ b/cfgmgr/intfmgrd.cpp @@ -16,8 +16,6 @@ using namespace swss; /* select() function timeout retry time, in millisecond */ #define SELECT_TIMEOUT 1000 -int gBatchSize = 0; - int main(int argc, char **argv) { Logger::linkToDbNative("intfmgrd"); diff --git a/cfgmgr/macsecmgrd.cpp b/cfgmgr/macsecmgrd.cpp index 055beb3bdb..263c5b4395 100644 --- a/cfgmgr/macsecmgrd.cpp +++ b/cfgmgr/macsecmgrd.cpp @@ -24,7 +24,6 @@ using namespace swss; /* select() function timeout retry time, in millisecond */ #define SELECT_TIMEOUT 1000 -int gBatchSize = 0; MacAddress gMacAddress; static bool received_sigterm = false; diff --git a/cfgmgr/natmgrd.cpp b/cfgmgr/natmgrd.cpp index 421000d40c..0e3a52fadc 100644 --- a/cfgmgr/natmgrd.cpp +++ b/cfgmgr/natmgrd.cpp @@ -39,7 +39,6 @@ using namespace swss; /* select() function timeout retry time, in millisecond */ #define SELECT_TIMEOUT 1000 -int gBatchSize = 0; NatMgr *natmgr = NULL; NotificationConsumer *timeoutNotificationsConsumer = NULL; diff --git a/cfgmgr/nbrmgrd.cpp b/cfgmgr/nbrmgrd.cpp index 7659367de3..2d325551a2 100644 --- a/cfgmgr/nbrmgrd.cpp +++ b/cfgmgr/nbrmgrd.cpp @@ -20,8 +20,6 @@ using namespace swss; /* select() function timeout retry time, in millisecond */ #define SELECT_TIMEOUT 1000 -int gBatchSize = 0; - int main(int argc, char **argv) { Logger::linkToDbNative("nbrmgrd"); diff --git a/cfgmgr/portmgrd.cpp b/cfgmgr/portmgrd.cpp index 7bfc969f88..99c7974559 100644 --- a/cfgmgr/portmgrd.cpp +++ b/cfgmgr/portmgrd.cpp @@ -15,8 +15,6 @@ using namespace swss; /* select() function timeout retry time, in millisecond */ #define SELECT_TIMEOUT 1000 -int gBatchSize = 0; - int main(int argc, char **argv) { Logger::linkToDbNative("portmgrd"); diff --git a/cfgmgr/sflowmgrd.cpp b/cfgmgr/sflowmgrd.cpp index dd2b19c09e..2eef82bac7 100644 --- a/cfgmgr/sflowmgrd.cpp +++ b/cfgmgr/sflowmgrd.cpp @@ -15,8 +15,6 @@ using namespace swss; /* select() function timeout retry time, in millisecond */ #define SELECT_TIMEOUT 1000 -int gBatchSize = 0; - int main(int argc, char **argv) { Logger::linkToDbNative("sflowmgrd"); diff --git a/cfgmgr/teammgrd.cpp b/cfgmgr/teammgrd.cpp index e48ecf6063..a18838c959 100644 --- a/cfgmgr/teammgrd.cpp +++ b/cfgmgr/teammgrd.cpp @@ -12,7 +12,6 @@ using namespace swss; #define SELECT_TIMEOUT 1000 -int gBatchSize = 0; bool received_sigterm = false; static struct sigaction old_sigaction; diff --git a/cfgmgr/tunnelmgrd.cpp b/cfgmgr/tunnelmgrd.cpp index 0990d492b8..69157ba051 100644 --- a/cfgmgr/tunnelmgrd.cpp +++ b/cfgmgr/tunnelmgrd.cpp @@ -19,8 +19,6 @@ using namespace swss; /* select() function timeout retry time, in millisecond */ #define SELECT_TIMEOUT 1000 -int gBatchSize = 0; - int main(int argc, char **argv) { Logger::linkToDbNative("tunnelmgrd"); diff --git a/cfgmgr/vlanmgrd.cpp b/cfgmgr/vlanmgrd.cpp index 55f19d69ea..84bc19cf08 100644 --- a/cfgmgr/vlanmgrd.cpp +++ b/cfgmgr/vlanmgrd.cpp @@ -21,7 +21,6 @@ using namespace swss; /* select() function timeout retry time, in millisecond */ #define SELECT_TIMEOUT 1000 -int gBatchSize = 0; MacAddress gMacAddress; int main(int argc, char **argv) diff --git a/cfgmgr/vrfmgrd.cpp b/cfgmgr/vrfmgrd.cpp index ed4db3a12d..3dbc7e447e 100644 --- a/cfgmgr/vrfmgrd.cpp +++ b/cfgmgr/vrfmgrd.cpp @@ -16,8 +16,6 @@ using namespace swss; /* select() function timeout retry time, in millisecond */ #define SELECT_TIMEOUT 1000 -int gBatchSize = 0; - int main(int argc, char **argv) { Logger::linkToDbNative("vrfmgrd"); diff --git a/cfgmgr/vxlanmgrd.cpp b/cfgmgr/vxlanmgrd.cpp index 404aa386a2..c992233c86 100644 --- a/cfgmgr/vxlanmgrd.cpp +++ b/cfgmgr/vxlanmgrd.cpp @@ -21,7 +21,6 @@ using namespace swss; /* select() function timeout retry time, in millisecond */ #define SELECT_TIMEOUT 1000 -int gBatchSize = 0; MacAddress gMacAddress; int main(int argc, char **argv) diff --git a/orchagent/main.cpp b/orchagent/main.cpp index 70e0d1042c..be581bfed8 100644 --- a/orchagent/main.cpp +++ b/orchagent/main.cpp @@ -50,7 +50,7 @@ MacAddress gVxlanMacAddress; extern size_t gMaxBulkSize; #define DEFAULT_BATCH_SIZE 128 -int gBatchSize = DEFAULT_BATCH_SIZE; +extern int gBatchSize; bool gSyncMode = false; sai_redis_communication_mode_t gRedisCommunicationMode = SAI_REDIS_COMMUNICATION_MODE_REDIS_ASYNC; @@ -334,6 +334,7 @@ int main(int argc, char **argv) int opt; sai_status_t status; + gBatchSize = DEFAULT_BATCH_SIZE; string record_location = Recorder::DEFAULT_DIR; string swss_rec_filename = Recorder::SWSS_FNAME; string sairedis_rec_filename = Recorder::SAIREDIS_FNAME; diff --git a/orchagent/orch.cpp b/orchagent/orch.cpp index d74b1d4342..98f0632d54 100644 --- a/orchagent/orch.cpp +++ b/orchagent/orch.cpp @@ -13,7 +13,7 @@ using namespace swss; -extern int gBatchSize; +int gBatchSize = 0; Orch::Orch(DBConnector *db, const string tableName, int pri) { diff --git a/orchagent/p4orch/tests/test_main.cpp b/orchagent/p4orch/tests/test_main.cpp index f780dcdae6..09b7a7fa83 100644 --- a/orchagent/p4orch/tests/test_main.cpp +++ b/orchagent/p4orch/tests/test_main.cpp @@ -37,7 +37,6 @@ sai_object_id_t kMirrorSessionOid2 = 9002; sai_object_id_t gUnderlayIfId; #define DEFAULT_BATCH_SIZE 128 -int gBatchSize = DEFAULT_BATCH_SIZE; #define DEFAULT_MAX_BULK_SIZE 1000 size_t gMaxBulkSize = DEFAULT_MAX_BULK_SIZE; bool gSyncMode = false; @@ -172,7 +171,8 @@ void AddVrf() } // namespace int main(int argc, char *argv[]) -{ +{ + gBatchSize = DEFAULT_BATCH_SIZE; testing::InitGoogleTest(&argc, argv); sai_router_interface_api_t router_intfs_api; diff --git a/tests/mock_tests/mock_orchagent_main.cpp b/tests/mock_tests/mock_orchagent_main.cpp index cd278fdfe1..542e5f3e36 100644 --- a/tests/mock_tests/mock_orchagent_main.cpp +++ b/tests/mock_tests/mock_orchagent_main.cpp @@ -12,13 +12,6 @@ sai_object_id_t gSwitchId = SAI_NULL_OBJECT_ID; MacAddress gMacAddress; MacAddress gVxlanMacAddress; -#define DEFAULT_BATCH_SIZE 128 -int gBatchSize = DEFAULT_BATCH_SIZE; - -bool gSwssRecord = true; -bool gLogRotate = false; -ofstream gRecordOfs; -string gRecordFile; string gMySwitchType = "switch"; int32_t gVoqMySwitchId = 0; string gMyHostName = "Linecard1"; diff --git a/tests/mock_tests/mock_orchagent_main.h b/tests/mock_tests/mock_orchagent_main.h index dc94c502f5..925e39186f 100644 --- a/tests/mock_tests/mock_orchagent_main.h +++ b/tests/mock_tests/mock_orchagent_main.h @@ -30,10 +30,6 @@ #include "directory.h" extern int gBatchSize; -extern bool gSwssRecord; -extern bool gLogRotate; -extern ofstream gRecordOfs; -extern string gRecordFile; extern MacAddress gMacAddress; extern MacAddress gVxlanMacAddress; From 0899623f054094bf14b6122c852522f1852632a2 Mon Sep 17 00:00:00 2001 From: Vivek Reddy Karri Date: Fri, 23 Jun 2023 01:53:02 +0000 Subject: [PATCH 7/9] handle spell check failures Signed-off-by: Vivek Reddy Karri --- lib/recorder.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/recorder.cpp b/lib/recorder.cpp index 87903be38d..8a51780ece 100644 --- a/lib/recorder.cpp +++ b/lib/recorder.cpp @@ -92,7 +92,6 @@ void RecWriter::record(const std::string& val) record_ofs << swss::getTimestamp() << "|" << val << std::endl; if (isRotate()) { - /* If the log rotate is set by sighup handler, peform the following actions */ setRotate(false); logfileReopen(); } From d8588e9ae4d8baa3d6202302636de5424a0afa89 Mon Sep 17 00:00:00 2001 From: Vivek Reddy Karri Date: Tue, 27 Jun 2023 01:43:19 +0000 Subject: [PATCH 8/9] Add missing declaration Signed-off-by: Vivek Reddy Karri --- orchagent/p4orch/tests/test_main.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/orchagent/p4orch/tests/test_main.cpp b/orchagent/p4orch/tests/test_main.cpp index c4dbd7f66b..787e0622f4 100644 --- a/orchagent/p4orch/tests/test_main.cpp +++ b/orchagent/p4orch/tests/test_main.cpp @@ -38,6 +38,7 @@ sai_object_id_t gUnderlayIfId; #define DEFAULT_BATCH_SIZE 128 #define DEFAULT_MAX_BULK_SIZE 1000 +extern int gBatchSize; size_t gMaxBulkSize = DEFAULT_MAX_BULK_SIZE; bool gSyncMode = false; bool gIsNatSupported = false; From 5895a14baa611b329b1440fe72e9314d2d0236f9 Mon Sep 17 00:00:00 2001 From: Vivek Reddy Karri Date: Mon, 3 Jul 2023 23:25:08 +0000 Subject: [PATCH 9/9] Converted Recorder into a singleton and handled other comments Signed-off-by: Vivek Reddy Karri --- lib/recorder.cpp | 10 +++++--- lib/recorder.h | 39 +++++++++++++++++--------------- orchagent/main.cpp | 28 +++++++++++------------ orchagent/orch.cpp | 2 +- orchagent/orch.h | 2 +- orchagent/orchdaemon.cpp | 8 +++---- orchagent/response_publisher.cpp | 10 ++++---- orchagent/saihelper.cpp | 10 ++++---- 8 files changed, 58 insertions(+), 51 deletions(-) diff --git a/lib/recorder.cpp b/lib/recorder.cpp index 8a51780ece..449039adff 100644 --- a/lib/recorder.cpp +++ b/lib/recorder.cpp @@ -11,9 +11,13 @@ const std::string Recorder::SWSS_FNAME = "swss.rec"; const std::string Recorder::SAIREDIS_FNAME = "sairedis.rec"; const std::string Recorder::RESPPUB_FNAME = "responsepublisher.rec"; -std::unique_ptr Recorder::swss = std::make_unique(); -std::unique_ptr Recorder::sairedis = std::make_unique(); -std::unique_ptr Recorder::respub = std::make_unique(); + +Recorder& Recorder::Instance() +{ + static Recorder m_recorder; + return m_recorder; +} + SwSSRec::SwSSRec() { diff --git a/lib/recorder.h b/lib/recorder.h index 104dfa9956..971c3a2bb7 100644 --- a/lib/recorder.h +++ b/lib/recorder.h @@ -10,32 +10,32 @@ namespace swss { class RecBase { public: - RecBase() {} + RecBase() = default; /* Setters */ - void setRecord(bool record) { enable_rec = record; } - void setRotate(bool rotate) { log_rotate = rotate; } - void setLocation(const std::string& loc) { location = loc; } - void setFileName(const std::string& name) { filename = name; } + void setRecord(bool record) { m_recording = record; } + void setRotate(bool rotate) { m_rotate = rotate; } + void setLocation(const std::string& loc) { m_location = loc; } + void setFileName(const std::string& name) { m_filename = name; } void setName(const std::string& name) { m_name = name; } /* getters */ - bool isRecord() { return enable_rec; } - bool isRotate() { return log_rotate; } - std::string getLoc() { return location; } - std::string getFile() { return filename; } + bool isRecord() { return m_recording; } + bool isRotate() { return m_rotate; } + std::string getLoc() { return m_location; } + std::string getFile() { return m_filename; } std::string getName() { return m_name; } private: - bool enable_rec; - bool log_rotate; - std::string location; - std::string filename; + bool m_recording; + bool m_rotate; + std::string m_location; + std::string m_filename; std::string m_name; }; class RecWriter : public RecBase { public: - RecWriter() : RecBase() {} + RecWriter() = default; virtual ~RecWriter(); void startRec(bool exit_if_failure); void record(const std::string& val); @@ -67,15 +67,18 @@ class SaiRedisRec : public RecBase { /* Interface to access recorder classes */ class Recorder { public: + static Recorder& Instance(); static const std::string DEFAULT_DIR; static const std::string REC_START; static const std::string SWSS_FNAME; static const std::string SAIREDIS_FNAME; static const std::string RESPPUB_FNAME; - - static std::unique_ptr swss; - static std::unique_ptr sairedis; - static std::unique_ptr respub; + + Recorder() = default; + /* Individual Handlers */ + SwSSRec swss; + SaiRedisRec sairedis; + ResPubRec respub; }; } diff --git a/orchagent/main.cpp b/orchagent/main.cpp index be581bfed8..daccbc1b27 100644 --- a/orchagent/main.cpp +++ b/orchagent/main.cpp @@ -96,9 +96,9 @@ void sighup_handler(int signo) /* * Don't do any logging since they are using mutexes. */ - Recorder::swss->setRotate(true); - Recorder::sairedis->setRotate(true); - Recorder::respub->setRotate(true); + Recorder::Instance().swss.setRotate(true); + Recorder::Instance().sairedis.setRotate(true); + Recorder::Instance().respub.setRotate(true); } void syncd_apply_view() @@ -427,31 +427,31 @@ int main(int argc, char **argv) SWSS_LOG_NOTICE("--- Starting Orchestration Agent ---"); /* Initialize sairedis recording parameters */ - Recorder::sairedis->setRecord( + Recorder::Instance().sairedis.setRecord( (record_type & SAIREDIS_RECORD_ENABLE) == SAIREDIS_RECORD_ENABLE ); - Recorder::sairedis->setLocation(record_location); - Recorder::sairedis->setFileName(sairedis_rec_filename); + Recorder::Instance().sairedis.setLocation(record_location); + Recorder::Instance().sairedis.setFileName(sairedis_rec_filename); /* Initialize sairedis */ initSaiApi(); initSaiRedis(); /* Initialize remaining recorder parameters */ - Recorder::swss->setRecord( + Recorder::Instance().swss.setRecord( (record_type & SWSS_RECORD_ENABLE) == SWSS_RECORD_ENABLE ); - Recorder::swss->setLocation(record_location); - Recorder::swss->setFileName(swss_rec_filename); - Recorder::swss->startRec(true); + Recorder::Instance().swss.setLocation(record_location); + Recorder::Instance().swss.setFileName(swss_rec_filename); + Recorder::Instance().swss.startRec(true); - Recorder::respub->setRecord( + Recorder::Instance().respub.setRecord( (record_type & RESPONSE_PUBLISHER_RECORD_ENABLE) == RESPONSE_PUBLISHER_RECORD_ENABLE ); - Recorder::respub->setLocation(record_location); - Recorder::respub->setFileName(responsepublisher_rec_filename); - Recorder::respub->startRec(false); + Recorder::Instance().respub.setLocation(record_location); + Recorder::Instance().respub.setFileName(responsepublisher_rec_filename); + Recorder::Instance().respub.startRec(false); sai_attribute_t attr; vector attrs; diff --git a/orchagent/orch.cpp b/orchagent/orch.cpp index 98f0632d54..3ba2908c93 100644 --- a/orchagent/orch.cpp +++ b/orchagent/orch.cpp @@ -62,7 +62,7 @@ void ConsumerBase::addToSync(const KeyOpFieldsValuesTuple &entry) string op = kfvOp(entry); /* Record incoming tasks */ - Recorder::swss->record(dumpTuple(entry)); + Recorder::Instance().swss.record(dumpTuple(entry)); /* * m_toSync is a multimap which will allow one key with multiple values, diff --git a/orchagent/orch.h b/orchagent/orch.h index 628764c3f9..3b72931d99 100644 --- a/orchagent/orch.h +++ b/orchagent/orch.h @@ -220,7 +220,7 @@ class Orch Orch(swss::DBConnector *db, const std::vector &tableNames); Orch(swss::DBConnector *db, const std::vector &tableNameWithPri); Orch(const std::vector& tables); - virtual ~Orch() {}; + virtual ~Orch() = default; std::vector getSelectables(); diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index 6370f9ad23..a931e9a594 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -711,7 +711,7 @@ void OrchDaemon::start() { SWSS_LOG_ENTER(); - Recorder::sairedis->setRotate(false); + Recorder::Instance().sairedis.setRotate(false); for (Orch *o : m_orchList) { @@ -757,10 +757,10 @@ void OrchDaemon::start() } // check if logroate is requested - if (Recorder::sairedis->isRotate()) + if (Recorder::Instance().sairedis.isRotate()) { - SWSS_LOG_NOTICE("performing log rotate"); - Recorder::sairedis->setRotate(false); + SWSS_LOG_NOTICE("Performing %s log rotate", Recorder::Instance().sairedis.getName().c_str()); + Recorder::Instance().sairedis.setRotate(false); logRotate(); } diff --git a/orchagent/response_publisher.cpp b/orchagent/response_publisher.cpp index 2b7fafa50a..031f1aefef 100644 --- a/orchagent/response_publisher.cpp +++ b/orchagent/response_publisher.cpp @@ -31,7 +31,7 @@ std::string PrependedComponent(const ReturnCode &status) void RecordDBWrite(const std::string &table, const std::string &key, const std::vector &attrs, const std::string &op) { - if (!swss::Recorder::respub->isRecord()) + if (!swss::Recorder::Instance().respub.isRecord()) { return; } @@ -42,15 +42,15 @@ void RecordDBWrite(const std::string &table, const std::string &key, const std:: s += "|" + fvField(attr) + ":" + fvValue(attr); } - swss::Recorder::respub->record(s); + swss::Recorder::Instance().respub.record(s); } void RecordResponse(const std::string &response_channel, const std::string &key, const std::vector &attrs, const std::string &status) { - if (!swss::Recorder::respub->isRecord()) + if (!swss::Recorder::Instance().respub.isRecord()) { - return; + return; } std::string s = response_channel + ":" + key + "|" + status; @@ -59,7 +59,7 @@ void RecordResponse(const std::string &response_channel, const std::string &key, s += "|" + fvField(attr) + ":" + fvValue(attr); } - swss::Recorder::respub->record(s); + swss::Recorder::Instance().respub.record(s); } } // namespace diff --git a/orchagent/saihelper.cpp b/orchagent/saihelper.cpp index 3fcc27a56d..d1dc472d7d 100644 --- a/orchagent/saihelper.cpp +++ b/orchagent/saihelper.cpp @@ -251,11 +251,11 @@ void initSaiRedis() sai_attribute_t attr; sai_status_t status; - auto record_filename = Recorder::sairedis->getFile(); - auto record_location = Recorder::sairedis->getLoc(); + auto record_filename = Recorder::Instance().sairedis.getFile(); + auto record_location = Recorder::Instance().sairedis.getLoc(); /* set recording dir before enable recording */ - if (Recorder::sairedis->isRecord()) + if (Recorder::Instance().sairedis.isRecord()) { attr.id = SAI_REDIS_SWITCH_ATTR_RECORDING_OUTPUT_DIR; attr.value.s8list.count = (uint32_t)record_location.size(); @@ -285,13 +285,13 @@ void initSaiRedis() /* Disable/enable SAI Redis recording */ attr.id = SAI_REDIS_SWITCH_ATTR_RECORD; - attr.value.booldata = Recorder::sairedis->isRecord(); + attr.value.booldata = Recorder::Instance().sairedis.isRecord(); status = sai_switch_api->set_switch_attribute(gSwitchId, &attr); if (status != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("Failed to %s SAI Redis recording, rv:%d", - Recorder::sairedis->isRecord() ? "enable" : "disable", status); + Recorder::Instance().sairedis.isRecord() ? "enable" : "disable", status); exit(EXIT_FAILURE); }