[orchdaemon]: Fixed sairedis record file rotation #2299

bacrossland · 2022-05-27T19:51:24Z

What I did
Fix Azure/sonic-buildimage#8162

Moved sairedis record file rotation logic out of flush() to fix issue.

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

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

Signed-off-by: Bryan Crossland bryan.crossland@target.com

ghost · 2022-05-27T19:51:35Z

All CLA requirements met.

lguohan · 2022-05-28T19:24:46Z

@kcudnik , can you review this?

bacrossland · 2022-06-03T18:37:45Z

Pipeline fixes are still broken:

@kcudnik @prsunny The errors that are happing on this PR are not related to the code change it makes. The failure are related to problems in the master branch already or problems in the build pipeline.

Path does not exist: /home/vsts/work/1/a/gcov_output

How do you want me to proceed so this PR can be merged?

kcudnik · 2022-06-06T16:57:02Z

Please take a look at this PR too: sonic-net/sonic-sairedis#1058

Blueve · 2022-07-12T13:13:42Z

/azp run

azure-pipelines · 2022-07-12T13:13:52Z

Azure Pipelines successfully started running 1 pipeline(s).

Blueve · 2022-07-13T09:14:10Z

@bacrossland Build has passed. Could you add UT for your change?
https://dev.azure.com/mssonic/build/_build/results?buildId=121405&view=codecoverage-tab

bacrossland · 2022-07-13T12:31:37Z

Thank you @Blueve. I will get that unit test added.

bacrossland · 2022-08-08T15:37:18Z

/easycla

linux-foundation-easycla · 2022-08-08T15:37:25Z

The committers listed above are authorized under a signed CLA.

✅ login: bacrossland / name: Bryan Crossland (01cb5df, 80181c4)

Blueve · 2022-08-17T06:03:33Z

/azp run coverage.Azure.sonic-swss.amd64

azure-pipelines · 2022-08-17T06:03:38Z

No pipelines are associated with this pull request.

Blueve · 2022-08-17T06:08:59Z

/azp run

azure-pipelines · 2022-08-17T06:09:08Z

Azure Pipelines successfully started running 1 pipeline(s).

Blueve · 2022-08-17T06:16:02Z

@kcudnik The UT coverage for this repo might not accurate now and most PR are not having UT. Do you think we can merge this PR wo/ UT?

kcudnik · 2022-08-22T14:36:46Z

no, please add corresponding tests to fix code coverage, we dont want to skip this, threshold must be met

bacrossland · 2022-09-16T19:26:07Z

@kcudnik I'm setting aside time next week to fix this PR and get the testing in.

kcudnik · 2022-09-18T16:46:55Z

sure, lets fix this this week

bacrossland · 2022-09-29T20:05:05Z

Merged in master.

bacrossland · 2022-09-30T13:36:53Z

/azp run

azure-pipelines · 2022-09-30T13:36:59Z

Commenter does not have sufficient privileges for PR 2299 in repo sonic-net/sonic-swss

bacrossland · 2022-09-30T13:48:45Z

@kcudnik @Blueve I've added the ut for code coverage. The PR is now failing on files missing from swss-common. I don't have permission to rerun the test. Can you rerun the pipeline to help clear up these issues? Thanks.

Blueve · 2022-10-01T08:08:00Z

/azp run

azure-pipelines · 2022-10-01T08:08:10Z

Azure Pipelines successfully started running 1 pipeline(s).

bacrossland · 2022-10-01T13:02:46Z

The LGTM analysis is failing on this. It's coming from swss-common:

[2022-10-01 09:09:02] [build-stderr] In file included from defaultvalueprovider.cpp:10:
[2022-10-01 09:09:02] [build-stderr] defaultvalueprovider.h:8:10: fatal error: libyang/libyang.h: No such file or directory
[2022-10-01 09:09:02] [build-stderr]     8 | #include <libyang/libyang.h>
[2022-10-01 09:09:02] [build-stderr]       |          ^~~~~~~~~~~~~~~~~~~
[2022-10-01 09:09:02] [build-stderr] compilation terminated.
[2022-10-01 09:09:02] [build-stderr] make[3]: *** [Makefile:787: libswsscommon_la-defaultvalueprovider.lo] Error 1
[2022-10-01 09:09:02] [build-stderr] make[3]: *** Waiting for unfinished jobs....
[2022-10-01 09:09:06] [build-stdout] make[3]: Leaving directory '/opt/src/sonic-swss-common/common'
[2022-10-01 09:09:06] [build-stderr] make[2]: *** [Makefile:440: all-recursive] Error 1
[2022-10-01 09:09:06] [build-stdout] make[2]: Leaving directory '/opt/src/sonic-swss-common'
[2022-10-01 09:09:06] [build-stderr] make[1]: *** [Makefile:372: all] Error 2
[2022-10-01 09:09:06] [build-stdout] make[1]: Leaving directory '/opt/src/sonic-swss-common'
[2022-10-01 09:09:06] [build-stderr] dh_auto_build: error: make -j4 returned exit code 2
[2022-10-01 09:09:06] [build-stderr] make: *** [debian/rules:33: build] Error 25
[2022-10-01 09:09:06] [build-stderr] dpkg-buildpackage: error: debian/rules build subprocess returned exit status 2
[2022-10-01 09:09:06] [ERROR] Spawned process exited abnormally (code 2; tried to run: [/opt/work/lgtm-workspace/lgtm/extract.sh])
A fatal error occurred: Exit status 2 from command: [/opt/work/lgtm-workspace/lgtm/extract.sh]

bacrossland · 2022-10-01T13:07:36Z

Test vstest is failing on this. Timeout of p2mp_tunnel:

test_p2mp_tunnel failed (1 runs remaining out of 2).
	<class 'AssertionError'>
	Operation timed out after 30 seconds with result None
	[<TracebackEntry /agent/_work/1/s/tests/conftest.py:1803>, <TracebackEntry /agent/_work/1/s/tests/conftest.py:1770>, <TracebackEntry /agent/_work/1/s/tests/conftest.py:405>, <TracebackEntry /agent/_work/1/s/tests/conftest.py:462>, <TracebackEntry /agent/_work/1/s/tests/conftest.py:500>, <TracebackEntry /agent/_work/1/s/tests/conftest.py:105>, <TracebackEntry /agent/_work/1/s/tests/conftest.py:126>, <TracebackEntry /agent/_work/1/s/tests/dvslib/dvs_common.py:60>]
test_p2mp_tunnel failed; it passed 0 out of the required 1 times.
	<class 'AssertionError'>
	Operation timed out after 30 seconds with result None
	[<TracebackEntry /usr/local/lib/python3.8/dist-packages/six.py:718>, <TracebackEntry /agent/_work/1/s/tests/conftest.py:1803>, <TracebackEntry /agent/_work/1/s/tests/conftest.py:1770>, <TracebackEntry /agent/_work/1/s/tests/conftest.py:405>, <TracebackEntry /agent/_work/1/s/tests/conftest.py:462>, <TracebackEntry /agent/_work/1/s/tests/conftest.py:500>, <TracebackEntry /agent/_work/1/s/tests/conftest.py:105>, <TracebackEntry /agent/_work/1/s/tests/conftest.py:126>, <TracebackEntry /agent/_work/1/s/tests/dvslib/dvs_common.py:60>]
test_vlan_extension failed (1 runs remaining out of 2).
	<class 'AssertionError'>
	Operation timed out after 30 seconds with result None
	[<TracebackEntry /usr/local/lib/python3.8/dist-packages/six.py:718>, <TracebackEntry /agent/_work/1/s/tests/conftest.py:1803>, <TracebackEntry /agent/_work/1/s/tests/conftest.py:1770>, <TracebackEntry /agent/_work/1/s/tests/conftest.py:405>, <TracebackEntry /agent/_work/1/s/tests/conftest.py:462>, <TracebackEntry /agent/_work/1/s/tests/conftest.py:500>, <TracebackEntry /agent/_work/1/s/tests/conftest.py:105>, <TracebackEntry /agent/_work/1/s/tests/conftest.py:126>, <TracebackEntry /agent/_work/1/s/tests/dvslib/dvs_common.py:60>]
test_vlan_extension failed; it passed 0 out of the required 1 times.
	<class 'AssertionError'>
	Wrong number of created entries.
	[<TracebackEntry /agent/_work/1/s/tests/test_evpn_tunnel_p2mp.py:65>, <TracebackEntry /agent/_work/1/s/tests/evpn_tunnel.py:524>, <TracebackEntry /agent/_work/1/s/tests/evpn_tunnel.py:50>]

Also here. Assertion failed on removing ipv6 link:

test_NeighborAddRemoveIpv6LinkLocal failed (1 runs remaining out of 2).
	<class 'AssertionError'>
	assert 2 == 4
  -2
  +4
	[<TracebackEntry /agent/_work/1/s/tests/test_ipv6_link_local.py:63>]

bacrossland · 2022-10-01T13:24:31Z

Pulled in latest commits that have fixes for missing libyang

bacrossland · 2022-10-02T01:29:08Z

@kcudnik @Blueve @prsunny Everything is passing. Ready for a review and a merge.

yxieca · 2022-10-03T19:23:58Z

@bacrossland can you raise a separate PR for 202205 branch? This change cannot be cherry-picked to 202205 branch cleanly.

qiluo-msft · 2022-10-03T20:27:48Z

This commit could not be cleanly cherry-picked to 202012. Please submit another PR.

bacrossland · 2022-10-03T23:30:36Z

Sure. I'll put those in later tonight or tomorrow morning.

bacrossland · 2022-10-04T03:56:45Z

PR 202205 Release: #2480

bacrossland · 2022-10-04T03:59:44Z

@qiluo-msft 202012 is going to take more work as that release is missing things I relied on for the unit tests I put in master. I have to rework my unit tests to make it work for that old release. I could also just put in a PR without the unit tests but that would be less desirable.

bacrossland · 2022-10-04T19:29:13Z

PR 202012 Release: #2481

* [orchdaemon]: Fixed sairedis record file rotation * What I did Fix sonic-net/sonic-buildimage#8162 Moved sairedis record file rotation logic out of flush() to fix issue. Why I did it Sairedis record file was not releasing the file handle on rotation. This is because the file handle release was inside the flush() which was only being called if a select timeout was triggered. Moved the logic to its own function which is called in the start() loop. How I verified it Ran a script to fill log and verified that rotation was happening correctly. Signed-off-by: Bryan Crossland bryan.crossland@target.com

liuh-80 · 2023-02-02T02:02:55Z

202012 branch PR merged: #2481

[orchdaemon]: Fixed sairedis record file rotation

01cb5df

Signed-off-by: Bryan Crossland bryan.crossland@target.com

bacrossland requested a review from prsunny as a code owner May 27, 2022 19:51

bacrossland mentioned this pull request May 27, 2022

SAIREDIS Record when facing continuous SAI events is not compressing and rotating the record causing /var/log partition to completely run out of space. sonic-net/sonic-buildimage#8162

Closed

lguohan requested a review from kcudnik May 28, 2022 19:25

kcudnik previously approved these changes May 31, 2022

View reviewed changes

Merge branch 'master' of github.com:target/sonic-swss into logrotate

80181c4

Blueve added the Request for 202012 Branch label Jun 7, 2022

zhangyanzhao added the Request for 202205 Branch label Jun 7, 2022

Merge current master

c29e376

bacrossland dismissed kcudnik’s stale review via c29e376 September 20, 2022 15:57

kcudnik previously approved these changes Sep 20, 2022

View reviewed changes

Adding test. Still working on mocking issues.

4e0e66b

Merge branch 'master' of github.com:target/sonic-swss into logrotate

78cf4dd

Merge branch 'master' of github.com:target/sonic-swss into logrotate

707c0be

kcudnik approved these changes Oct 2, 2022

View reviewed changes

Blueve merged commit 24d29f1 into sonic-net:master Oct 3, 2022

yxieca added the Cherry Pick Conflict_202205 label Oct 3, 2022

qiluo-msft added the Cherry Pick Conflict_202012 label Oct 3, 2022

bacrossland mentioned this pull request Oct 4, 2022

[orchdaemon]: Fixed sairedis record file rotation #2480

Merged

bacrossland mentioned this pull request Oct 4, 2022

[orchdaemon]: Fixed sairedis record file rotation #2481

Merged

yxieca removed Cherry Pick Conflict_202205 Request for 202205 Branch labels Oct 25, 2022

liuh-80 added the Included in 202012 Branch label Feb 2, 2023

prsunny mentioned this pull request Mar 29, 2023

Fix orchagent missing request when logrotate happens. #2712

Merged

[orchdaemon]: Fixed sairedis record file rotation #2299

[orchdaemon]: Fixed sairedis record file rotation #2299

Conversation

bacrossland commented May 27, 2022

ghost commented May 27, 2022 • edited by ghost Loading

lguohan commented May 28, 2022

bacrossland commented Jun 3, 2022

kcudnik commented Jun 6, 2022

Blueve commented Jul 12, 2022

azure-pipelines bot commented Jul 12, 2022

Blueve commented Jul 13, 2022

bacrossland commented Jul 13, 2022

bacrossland commented Aug 8, 2022

linux-foundation-easycla bot commented Aug 8, 2022 • edited Loading

Blueve commented Aug 17, 2022

azure-pipelines bot commented Aug 17, 2022

Blueve commented Aug 17, 2022

azure-pipelines bot commented Aug 17, 2022

Blueve commented Aug 17, 2022

kcudnik commented Aug 22, 2022

bacrossland commented Sep 16, 2022

kcudnik commented Sep 18, 2022

bacrossland commented Sep 29, 2022

bacrossland commented Sep 30, 2022

azure-pipelines bot commented Sep 30, 2022

bacrossland commented Sep 30, 2022

Blueve commented Oct 1, 2022

azure-pipelines bot commented Oct 1, 2022

bacrossland commented Oct 1, 2022 • edited Loading

bacrossland commented Oct 1, 2022

bacrossland commented Oct 1, 2022

bacrossland commented Oct 2, 2022

yxieca commented Oct 3, 2022

qiluo-msft commented Oct 3, 2022

bacrossland commented Oct 3, 2022

bacrossland commented Oct 4, 2022

bacrossland commented Oct 4, 2022

bacrossland commented Oct 4, 2022

liuh-80 commented Feb 2, 2023

ghost commented May 27, 2022 •

edited by ghost

Loading

linux-foundation-easycla bot commented Aug 8, 2022 •

edited

Loading

bacrossland commented Oct 1, 2022 •

edited

Loading