[snmp_memory] add SNMP memory test by okanchou9 · Pull Request #373 · sonic-net/sonic-mgmt

okanchou9 · 2017-12-01T01:44:32Z

Signed-off-by: okanchou9 [email protected]

What I did

Add test to check the difference percentage of Total Memory between SNMP and shell is 0
Add test to check the difference percentage of Total Free Memory between SNMP and shell small than 3% when:
- Check free memory before stress test
- Check free memory in stress test
- Check free memory after stress test
Add test to check memory leak issue, the difference percentage between Total Free Memory before stress test and after should small than 3%

How I did it
Please refer the diff for the details.
How to verify it
Run test with on my box and passed without any issue. Log

Signed-off-by: okanchou9 <[email protected]>

ansible/roles/test/tasks/snmp/memory.yml

lguohan · 2017-12-04T19:37:21Z

ansible/roles/test/tasks/snmp/memory.yml

+
+    - name: Validating SNMP total free memory matches shell result before stress test
+      assert:
+        that: "{{ '%.6f'|format((ansible_sysTotalFreeMemery|int - shell_total_free_memory.stdout|int)|abs / ansible_sysTotalFreeMemery|int) }} <= 0.03"


is 3% tolerance too small? can we allow like 10% tolerance to make the test more robust.

Since I have 8G RAM in my box, the 3% tolerance of 8G is about 246M already.
I'm not sure the quality of this test if we allow 10% tolerance in this test.
Any good idea?

make this as an test parameter, let's the default to 5%?

lguohan · 2017-12-04T19:42:04Z

ansible/roles/test/tasks/snmp/memory.yml

+    - name: Stop memory stress generation
+      shell: killall python /tmp/memory.py
+      become: yes
+      ignore_errors: true


if previous assert fails, your code will not kill the process and the box will have high memory consumption.

Don't worry about it, I wrote the "always" section which will kill again this python process at line 99 if previous assert failed.

actually, my question is that if the above assert, will ansible stop there? If ansible stops, then you will not be able to execute subsequent commands.

check here. http://docs.ansible.com/ansible/latest/playbooks_error_handling.html

"Generally playbooks will stop executing any more steps on a host that has a task fail."

I see what you mean. I think it is ok.

lguohan · 2017-12-04T19:45:03Z

ansible/roles/test/tasks/snmp/memory.yml

+
+    - name: Validating memory leak issue on DUT
+      assert:
+        that: "{{ '%.6f'|format((free_memery_before_test|int - ansible_sysTotalFreeMemery|int)|abs / free_memery_before_test|int) }} <= 0.03"


where does this memory leak comes from? the python program?

Yes, those memories which hold by this python process should be released after terminated it.
If the difference between the size of free memory before and after this stress test is more than 3%, I can only consider there is a memory leak happened when terminated the python process.

it is hard to say, the other process might increase the usage of memory.

checking memory leak is a hard problem, I am mainly worry about the false positive.

hmmm, just removed this test case from this PR and I'll find better way to test the memory leak. Thanks for your info.

lguohan · 2017-12-04T19:47:38Z

ansible/roles/test/tasks/snmp/memory.yml

+
+    - set_fact: free_memery_before_test="{{ ansible_sysTotalFreeMemery }}"
+
+    - set_fact: test_momory="{{ ((ansible_sysTotalFreeMemery - 512000) / 1024)|int }}"


why minus 512M memory? what is the purpose of memory stress test? is it just to verify the snmp can pick the memory consumption changes or there is some other goal? if only the first goal, then we just need to fill a few hundreds MB and check. We do not need to fill all the available memory.

This test will also check the SNMP process is still functional or not in suck low free memory condition.
It's enough for my box that only left 512M for system free memory.
Please let me know if any.

my request here is that can we skip this, if the sysTotalFreeMemory is already less then 512000? As above, can we make this as a tunable parameter, you can make 512M as the default value.

lguohan

as comments.

Signed-off-by: okanchou9 <[email protected]>

lguohan · 2017-12-05T11:16:30Z

can you add instruction to here. https://github.com/Azure/sonic-mgmt/blob/master/ansible/README.test.md

Signed-off-by: okanchou9 <[email protected]>

Signed-off-by: Guohan Lu <[email protected]>

…D automatically (sonic-net#15749) src/sonic-platform-daemons * 112656c - (HEAD -> 202205, origin/202205) [ycabled][active-active] no initialize Async Client, when no active-active cable type; fix names for all ycabled threads (sonic-net#373) (4 minutes ago) [vdahiya12] * e325d5a - Revert "Revert "[ycabled] correct the wrong function call for 'config hwmode state' (sonic-net#372)"" (4 minutes ago) [Ying Xie] * ddabca1 - Revert "Revert "[dualtor] Fix command `show mux status` (sonic-net#371)"" (4 minutes ago) [Ying Xie] * 28918da - Revert "Revert "[ycabled] fix bug for `show mux status` delayed response (sonic-net#364)"" (4 minutes ago) [Ying Xie] * a849de9 - Revert "Revert "add async notification support in active-active topo; refactor code for ycable tasks for change events (sonic-net#327)"" (4 minutes ago) [Ying Xie] * cf1e73a - Revert "Revert "[ycabled] refactor code for onboarding async client changes;refactor (sonic-net#355)"" (4 minutes ago) [Ying Xie]

[snmp_memory] add SNMP memory test

356fd66

Signed-off-by: okanchou9 <[email protected]>

lguohan reviewed Dec 4, 2017

View reviewed changes

ansible/roles/test/tasks/snmp/memory.yml Outdated Show resolved Hide resolved

lguohan reviewed Dec 4, 2017

View reviewed changes

lguohan requested changes Dec 4, 2017

View reviewed changes

[snmp_memory] remove unnecessary SNMP query

629eac2

Signed-off-by: okanchou9 <[email protected]>

lguohan requested a review from maggiemsft December 5, 2017 11:10

[snmp_memory] update script with request

52e5081

Signed-off-by: okanchou9 <[email protected]>

Merge branch 'master' into snmp_memory

8177e2e

lguohan approved these changes Mar 20, 2020

View reviewed changes

lguohan added 2 commits March 20, 2020 09:28

Merge branch 'okanchou9-snmp_memory'

ad96cfe

remove text

035beb5

Signed-off-by: Guohan Lu <[email protected]>

lguohan merged commit 63af908 into sonic-net:master Mar 20, 2020


		- set_fact: free_memery_before_test="{{ ansible_sysTotalFreeMemery }}"

		- set_fact: test_momory="{{ ((ansible_sysTotalFreeMemery - 512000) / 1024)\|int }}"

Conversation

okanchou9 commented Dec 1, 2017

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

okanchou9 Dec 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

okanchou9 Dec 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

okanchou9 Dec 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

okanchou9 Dec 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lguohan left a comment

Choose a reason for hiding this comment

Uh oh!

lguohan commented Dec 5, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

okanchou9 Dec 5, 2017 •

edited

Loading

okanchou9 Dec 5, 2017 •

edited

Loading

okanchou9 Dec 5, 2017 •

edited

Loading

okanchou9 Dec 12, 2017 •

edited

Loading