Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix snmp agent not-responding issue when high CPU utilization #345

Merged
merged 5 commits into from
Jan 14, 2025
Merged

Fix snmp agent not-responding issue when high CPU utilization #345

merged 5 commits into from
Jan 14, 2025

Conversation

yejianquan
Copy link
Contributor

@yejianquan yejianquan commented Jan 2, 2025 •
edited
Loading

Fix sonic-net/sonic-buildimage#21314
The SNMP agent and MIB updaters are basic on Asyncio/Coroutine,
the mib updaters share the same Asyncio event loop with the SNMP agent client.
Hence during the updaters executing, the agent client can't receive/respond to new requests.

When the CPU utilization is high (In some stress test we make CPU 100% utilization), the updates are slow, and this causes the snmpd request to be timeout because the agent got suspended during updating.

- What I did
Decrease the MIB update frequency when the update execution is slow.
pros:

  1. The snmp request can success even if 100% CPU utilization.
  2. The snmpd request seldomly fails due to timeout, combined with Update snmpd.conf.j2 prolong agentXTimeout to avoid timeout failure in high CPU utilization scenario sonic-buildimage#21316 , we have 4*5 = 20s time windows for the SNMP agent to wait for the MIB updates finish and respond to snmpd request.
  3. Relief the CPU cost when CPU is high, the can avoid CPU becomes more crowded.

cons:

  1. Tested on pizzabox (4600c), the updaters are very fast, generally finished within 0.001~0.02s, the chagne won't actually affect the frequency and interval.
  2. On Cisco chassis, the update of SNMP data could be delayed for 10~20s(at most 60s in extreme situation). Per my oberservation, most of the updater finishes within 0.5s. But for
    1.a ciscoSwitchQosMIB.QueueStatUpdater, generally finishs in 0.5-2s, expected to be delayed to 5-20s
    1.b PhysicalTableMIBUpdater, generally finishs in 0.5-1.5s, expected to be delayed to 5-1.5s
    1.c ciscoPfcExtMIB.PfcPrioUpdater, generally finishs in 0.5-3s, expected to be delayed to 5-30s

- How I did it
In get_next_update_interval, we compute the interval based on current execution time.
Roughly, we make the 'update interval'/'update execution time' >= UPDATE_FREQUENCY_RATE(10)
More specifically, if the execution time is 2.000001s, we sleep 21s before next update round.
And the max interval won't be longer than MAX_UPDATE_INTERVAL(60s).

- How to verify it
Test on Cisco chassis, test_snmp_cpu.py which triggers 100% CPU utilization test whether snmp requests work well.

- Description for the changelog

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yejianquan
Copy link
Contributor Author

Hi @SuvarnaMeenakshi , could you help to review this PR?
The only concern is the potential delay of SNMP data, described in cons.
Do you think the delay would result in risk in production env?

@yejianquan
Copy link
Contributor Author

FYI @abdosi @vperumal @anamehra

@yejianquan yejianquan mentioned this pull request Jan 2, 2025
Closed
@rlhui rlhui added the P0 label Jan 4, 2025
@yejianquan yejianquan requested a review from qiluo-msft January 4, 2025 04:33
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yejianquan yejianquan mentioned this pull request Jan 8, 2025
Merged
12 tasks
@yejianquan
Copy link
Contributor Author

Hi @qiluo-msft @SuvarnaMeenakshi
Limited the scope of dynamic frequency, please help to review again, thanks!

@yejianquan yejianquan merged commit 9e2c50a into sonic-net:master Jan 14, 2025
5 checks passed
This was referenced Jan 14, 2025
Merged
Merged
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202411: #346

@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202405: #347

mssonicbld added a commit to mssonicbld/sonic-buildimage that referenced this pull request Jan 14, 2025
@mssonicbld
[chassis-packet][SNMP] Enable SNMP dynamic frequency on packet chassis
f9b677d 
<!--
     Please make sure you've read and understood our contributing guidelines:
     https://github.com/Azure/SONiC/blob/gh-pages/CONTRIBUTING.md

     ** Make sure all your commits include a signature generated with `git commit -s` **

     If this is a bug fix, make sure your description includes "fixes #xxxx", or
     "closes #xxxx" or "resolves #xxxx"

     Please provide the following information:
-->

#### Why I did it
It's one part of the fixes of sonic-net#21314
SNMP walker request will always timeout when 100% CPU utilization.
##### Work item tracking
- Microsoft ADO **30112399**:

#### How I did it
Enable SNMP dynamic frequency on packet chassis.
#### How to verify it
snmp/test_snmp_cpu.py(https://github.com/sonic-net/sonic-mgmt/blob/master/tests/snmp/test_snmp_cpu.py) tests the scenario.

And the test case passes in my local run.
<!--
If PR needs to be backported, then the PR must be tested against the base branch and the earliest backport release branch and provide tested image version on these two branches. For example, if the PR is requested for master, 202211 and 202012, then the requester needs to provide test results on master and 202012.
-->

#### Which release branch to backport (provide reason below if selected)

<!--
- Note we only backport fixes to a release branch, *not* features!
- Please also provide a reason for the backporting below.
- e.g.
- [x] 202006
-->

- [ ] 201811
- [ ] 201911
- [ ] 202006
- [ ] 202012
- [ ] 202106
- [ ] 202111
- [ ] 202205
- [ ] 202211
- [ ] 202305
- [x] 202405

#### Tested branch (Please provide the tested image version)

<!--
- Please provide tested image version
- e.g.
- [x] 20201231.100
-->

- [ ] <!-- image version 1 -->
- [ ] <!-- image version 2 -->

#### Description for the changelog
<!--
Write a short (one line) summary that describes the changes in this
pull request for inclusion in the changelog:
-->
For issue:
sonic-net#21314

Change of SNMP-AgentX sonic-net/sonic-snmpagent#345 will fix the issue.
But we want to limit the scope of the change, since we only see this issue on packet-chassis and old devices with low performance CPUs.

<!--
 Ensure to add label/tag for the feature raised. example - PR#2174 under sonic-utilities repo. where, Generic Config and Update feature has been labelled as GCU.
-->

#### Link to config_db schema for YANG module changes
<!--
Provide a link to config_db schema for the table for which YANG model
is defined
Link should point to correct section on https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md
-->

#### A picture of a cute animal (not mandatory but encouraged)
@mssonicbld mssonicbld mentioned this pull request Jan 14, 2025
Merged
12 tasks
mssonicbld added a commit to mssonicbld/sonic-buildimage that referenced this pull request Jan 14, 2025
@mssonicbld
[chassis-packet][SNMP] Enable SNMP dynamic frequency on packet chassis
3490cb5 
<!--
     Please make sure you've read and understood our contributing guidelines:
     https://github.com/Azure/SONiC/blob/gh-pages/CONTRIBUTING.md

     ** Make sure all your commits include a signature generated with `git commit -s` **

     If this is a bug fix, make sure your description includes "fixes #xxxx", or
     "closes #xxxx" or "resolves #xxxx"

     Please provide the following information:
-->

#### Why I did it
It's one part of the fixes of sonic-net#21314
SNMP walker request will always timeout when 100% CPU utilization.
##### Work item tracking
- Microsoft ADO **30112399**:

#### How I did it
Enable SNMP dynamic frequency on packet chassis.
#### How to verify it
snmp/test_snmp_cpu.py(https://github.com/sonic-net/sonic-mgmt/blob/master/tests/snmp/test_snmp_cpu.py) tests the scenario.

And the test case passes in my local run.
<!--
If PR needs to be backported, then the PR must be tested against the base branch and the earliest backport release branch and provide tested image version on these two branches. For example, if the PR is requested for master, 202211 and 202012, then the requester needs to provide test results on master and 202012.
-->

#### Which release branch to backport (provide reason below if selected)

<!--
- Note we only backport fixes to a release branch, *not* features!
- Please also provide a reason for the backporting below.
- e.g.
- [x] 202006
-->

- [ ] 201811
- [ ] 201911
- [ ] 202006
- [ ] 202012
- [ ] 202106
- [ ] 202111
- [ ] 202205
- [ ] 202211
- [ ] 202305
- [x] 202405

#### Tested branch (Please provide the tested image version)

<!--
- Please provide tested image version
- e.g.
- [x] 20201231.100
-->

- [ ] <!-- image version 1 -->
- [ ] <!-- image version 2 -->

#### Description for the changelog
<!--
Write a short (one line) summary that describes the changes in this
pull request for inclusion in the changelog:
-->
For issue:
sonic-net#21314

Change of SNMP-AgentX sonic-net/sonic-snmpagent#345 will fix the issue.
But we want to limit the scope of the change, since we only see this issue on packet-chassis and old devices with low performance CPUs.

<!--
 Ensure to add label/tag for the feature raised. example - PR#2174 under sonic-utilities repo. where, Generic Config and Update feature has been labelled as GCU.
-->

#### Link to config_db schema for YANG module changes
<!--
Provide a link to config_db schema for the table for which YANG model
is defined
Link should point to correct section on https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md
-->

#### A picture of a cute animal (not mandatory but encouraged)
@mssonicbld mssonicbld mentioned this pull request Jan 14, 2025
Merged
12 tasks
mssonicbld added a commit to sonic-net/sonic-buildimage that referenced this pull request Jan 14, 2025
@mssonicbld
[chassis-packet][SNMP] Enable SNMP dynamic frequency on packet chassis (
ffdaef9 
#21430)

<!--
 Please make sure you've read and understood our contributing guidelines:
 https://github.com/Azure/SONiC/blob/gh-pages/CONTRIBUTING.md

 failure_prs.log Make sure all your commits include a signature generated with `git commit -s` **

 If this is a bug fix, make sure your description includes "fixes #xxxx", or
 "closes #xxxx" or "resolves #xxxx"

 Please provide the following information:
-->

#### Why I did it
It's one part of the fixes of #21314
SNMP walker request will always timeout when 100% CPU utilization.
##### Work item tracking
- Microsoft ADO **30112399**:

#### How I did it
Enable SNMP dynamic frequency on packet chassis.
#### How to verify it
snmp/test_snmp_cpu.py(https://github.com/sonic-net/sonic-mgmt/blob/master/tests/snmp/test_snmp_cpu.py) tests the scenario.

And the test case passes in my local run.
<!--
If PR needs to be backported, then the PR must be tested against the base branch and the earliest backport release branch and provide tested image version on these two branches. For example, if the PR is requested for master, 202211 and 202012, then the requester needs to provide test results on master and 202012.
-->

#### Which release branch to backport (provide reason below if selected)

<!--
- Note we only backport fixes to a release branch, *not* features!
- Please also provide a reason for the backporting below.
- e.g.
- [x] 202006
-->

- [ ] 201811
- [ ] 201911
- [ ] 202006
- [ ] 202012
- [ ] 202106
- [ ] 202111
- [ ] 202205
- [ ] 202211
- [ ] 202305
- [x] 202405

#### Tested branch (Please provide the tested image version)

<!--
- Please provide tested image version
- e.g.
- [x] 20201231.100
-->

- [ ] <!-- image version 1 -->
- [ ] <!-- image version 2 -->

#### Description for the changelog
<!--
Write a short (one line) summary that describes the changes in this
pull request for inclusion in the changelog:
-->
For issue:
#21314

Change of SNMP-AgentX sonic-net/sonic-snmpagent#345 will fix the issue.
But we want to limit the scope of the change, since we only see this issue on packet-chassis and old devices with low performance CPUs.

<!--
 Ensure to add label/tag for the feature raised. example - PR#2174 under sonic-utilities repo. where, Generic Config and Update feature has been labelled as GCU.
-->

#### Link to config_db schema for YANG module changes
<!--
Provide a link to config_db schema for the table for which YANG model
is defined
Link should point to correct section on https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md
-->

#### A picture of a cute animal (not mandatory but encouraged)
mssonicbld added a commit to sonic-net/sonic-buildimage that referenced this pull request Jan 15, 2025
@mssonicbld
[chassis-packet][SNMP] Enable SNMP dynamic frequency on packet chassis (
d8a340c 
#21429)

<!--
 Please make sure you've read and understood our contributing guidelines:
 https://github.com/Azure/SONiC/blob/gh-pages/CONTRIBUTING.md

 failure_prs.log Make sure all your commits include a signature generated with `git commit -s` **

 If this is a bug fix, make sure your description includes "fixes #xxxx", or
 "closes #xxxx" or "resolves #xxxx"

 Please provide the following information:
-->

#### Why I did it
It's one part of the fixes of #21314
SNMP walker request will always timeout when 100% CPU utilization.
##### Work item tracking
- Microsoft ADO **30112399**:

#### How I did it
Enable SNMP dynamic frequency on packet chassis.
#### How to verify it
snmp/test_snmp_cpu.py(https://github.com/sonic-net/sonic-mgmt/blob/master/tests/snmp/test_snmp_cpu.py) tests the scenario.

And the test case passes in my local run.
<!--
If PR needs to be backported, then the PR must be tested against the base branch and the earliest backport release branch and provide tested image version on these two branches. For example, if the PR is requested for master, 202211 and 202012, then the requester needs to provide test results on master and 202012.
-->

#### Which release branch to backport (provide reason below if selected)

<!--
- Note we only backport fixes to a release branch, *not* features!
- Please also provide a reason for the backporting below.
- e.g.
- [x] 202006
-->

- [ ] 201811
- [ ] 201911
- [ ] 202006
- [ ] 202012
- [ ] 202106
- [ ] 202111
- [ ] 202205
- [ ] 202211
- [ ] 202305
- [x] 202405

#### Tested branch (Please provide the tested image version)

<!--
- Please provide tested image version
- e.g.
- [x] 20201231.100
-->

- [ ] <!-- image version 1 -->
- [ ] <!-- image version 2 -->

#### Description for the changelog
<!--
Write a short (one line) summary that describes the changes in this
pull request for inclusion in the changelog:
-->
For issue:
#21314

Change of SNMP-AgentX sonic-net/sonic-snmpagent#345 will fix the issue.
But we want to limit the scope of the change, since we only see this issue on packet-chassis and old devices with low performance CPUs.

<!--
 Ensure to add label/tag for the feature raised. example - PR#2174 under sonic-utilities repo. where, Generic Config and Update feature has been labelled as GCU.
-->

#### Link to config_db schema for YANG module changes
<!--
Provide a link to config_db schema for the table for which YANG model
is defined
Link should point to correct section on https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md
-->

#### A picture of a cute animal (not mandatory but encouraged)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qiluo-msft qiluo-msft qiluo-msft approved these changes

@SuvarnaMeenakshi SuvarnaMeenakshi Awaiting requested review from SuvarnaMeenakshi

Assignees

@yejianquan yejianquan

Labels
Approved for 202405 Branch Approved for 202411 Branch Included in 202405 Branch Included in 202411 Branch P0 Request for msft-202405 Request for 202405 Branch Request for 202411 Branch
Projects
SONiC Chassis
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

SNMP request timeout when CPU utilization is high
4 participants
@yejianquan @mssonicbld @qiluo-msft @rlhui