sonic-net / sonic-sairedis

SAI object interface to Redis database, as used in the SONiC project
Other
56 stars 263 forks source link

Skip FABRIC PORT Attributes from sairedis logging (Issue 17583 ) #1339

Closed saksarav-nokia closed 7 months ago

saksarav-nokia commented 7 months ago

Orchagent polls SAI_PORT_ATTR_FABRIC_ATTACHED, SAI_PORT_ATTR_FABRIC_ATTACHED_SWITCH_ID & SAI_PORT_ATTR_FABRIC_ATTACHED_PORT_INDEX attributes every 30 secs for all the fabric ports in Supervisor and Linecards. If there are many asics in SUP and LC, these logs fill up the sairedis records very quickly and take up the disk space. Since these logs don't give much useful information, added these attributes SkipRecordAttrContainer to skip them from sairedis logging.

Fix for https://github.com/sonic-net/sonic-buildimage/issues/17583

judyjoseph commented 7 months ago

@saksarav-nokia Please add the test details too .. To confirm the logs are no more coming in the sairedis logs

saksarav-nokia commented 7 months ago

@judyjoseph , i tested the fix in both SUP and LC and monitored the sairedis logs, these logs are not getting logged. Without this change, the sairedis logs used to fill very quickly and log rotate rotates the sairedis files and many files will be present with in few hours. But with this fix, only very few sairedis log files are present and the events are created only during syncd init and not the regular polling events.

StormLiangMS commented 7 months ago

@judyjoseph could we have an ADO for cherry pick?

judyjoseph commented 7 months ago

@kcudnik I have a quick question here -- these attributes SAI_PORT_ATTR_FABRIC_ATTACHED, SAI_PORT_ATTR_FABRIC_ATTACHED_SWITCH_ID & SAI_PORT_ATTR_FABRIC_ATTACHED_PORT_INDEX -- are specific to T2 not applicable to T0/T1. Do we need to add any T2 specific check for these attributes inSkipRecordAttrContainer::SkipRecordAttrContainer() ?

kcudnik commented 7 months ago

im not ware of any specific platform attributes, and i dont have enough knowledge here, can you ask @saksarav-nokia

saksarav-nokia commented 7 months ago

@judyjoseph @kcudnik , The daemon FabricPortsOrch is started only if m_fabricEnabled is set and this flag is set only if the switch is voq. Do we still need to add T2 only check in SkipRecordAttrContainer? if (gMySwitchType != "fabric") { orchDaemon = make_shared(&appl_db, &config_db, &state_db, chassis_app_db.get()); if (gMySwitchType == "voq") { orchDaemon->setFabricEnabled(true); orchDaemon->setFabricPortStatEnabled(true); orchDaemon->setFabricQueueStatEnabled(false); } }

if (m_fabricEnabled) { vector fabric_port_tables = { // empty for now }; gFabricPortsOrch = new FabricPortsOrch(m_applDb, fabric_port_tables, m_fabricPortStatEnabled, m_fabricQueueStatEnabled); m_orchList.push_back(gFabricPortsOrch); }

kcudnik commented 7 months ago

Not sure about that, could you ask author of that enable fabric code ? I didn't put that there

gechiang commented 7 months ago

@StormLiangMS , @yxieca , MSFT ADO: 26734247 Please help approve backport to 202205, 202305, and 202311. Thanks!

mssonicbld commented 7 months ago

Cherry-pick PR to 202311: https://github.com/sonic-net/sonic-sairedis/pull/1353

mssonicbld commented 7 months ago

Cherry-pick PR to 202305: https://github.com/sonic-net/sonic-sairedis/pull/1354

gechiang commented 6 months ago

@yxieca this PR was approved for 202205 but I don't see this ever made it into 202205. Did this missed by auto-cherry-pick somehow? Can you help check what happened? Thanks!

yxieca commented 6 months ago

@yxieca this PR was approved for 202205 but I don't see this ever made it into 202205. Did this missed by auto-cherry-pick somehow? Can you help check what happened? Thanks!

@liushilongbuaa can you take a look?

liushilongbuaa commented 6 months ago

@yxieca this PR was approved for 202205 but I don't see this ever made it into 202205. Did this missed by auto-cherry-pick somehow? Can you help check what happened? Thanks!

@liushilongbuaa can you take a look?

tests/TestClient.cpp and tests/testclient.cpp breaks our workflow. I'm still working on Root Cause.

liushilongbuaa commented 6 months ago

@gechiang , @yxieca please create separate PR to cherry pick if urgent.