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

[eventd] eventd unit test is not stable #21140

Closed
Junchao-Mellanox opened this issue Dec 12, 2024 · 6 comments · Fixed by #21200
Closed

[eventd] eventd unit test is not stable #21140

Junchao-Mellanox opened this issue Dec 12, 2024 · 6 comments · Fixed by #21200
Assignees
@zbud-msft
Labels
MSFT Triaged this issue has been triaged

Comments

@Junchao-Mellanox
Copy link
Collaborator

Description

There is statistical failure in eventd unit test:

16:10:15  [ RUN      ] eventd.proxy
16:10:15  Proxy TEST started
16:10:15  tests/eventd_ut.cpp:170: Failure
16:10:15  Expected equality of these values:
16:10:15    1
16:10:15    rc
16:10:15      Which is: 36
16:10:15  tests/eventd_ut.cpp:279: Failure
16:10:15  Expected equality of these values:
16:10:15    rd_cevts_sz
16:10:15      Which is: 6
16:10:15    wr_evts.size()
16:10:15      Which is: 5
16:10:15  eventd_proxy is tested GOOD
16:10:15  [  FAILED  ] eventd.proxy (1603 ms)

Steps to reproduce the issue:

  1. build eventd

Describe the results you received:

eventd.proxy failed

Describe the results you expected:

no faillure

Output of show version:

commit hash: 7bdf8d4bb26709001cf95397c6a8e617986dd7fd

Output of show techsupport:

(paste your output here or download and attach the file here )

Additional information you deem important (e.g. issue happens only occasionally):
@anamehra
Copy link
Contributor

We have also observed this error intermittently recently causing multiple build failures. It works with retyr most of the time but sometime requires multiple retires.

@abdosi , @yejianquan , for you viz

@zbud-msft zbud-msft self-assigned this Dec 16, 2024
@zbud-msft zbud-msft mentioned this issue Dec 17, 2024
Merged
11 tasks
@zbud-msft
Copy link
Contributor

zbud-msft commented Dec 17, 2024 •
edited
Loading

PR fix: #21200, ETA for merge: 12/20

@tjchadaga tjchadaga added Triaged this issue has been triaged MSFT labels Dec 18, 2024
@zbud-msft
Copy link
Contributor

Hi @Junchao-Mellanox, is it possible for you take changes in #21200 and see if there is any flakiness in eventd builds on your side? I have ran ~25 image builds and have not seen any flakiness with these changes.

@Junchao-Mellanox
Copy link
Collaborator Author

sure, will try

@zbud-msft
Copy link
Contributor

@Junchao-Mellanox Any update on testing?

qiluo-msft pushed a commit that referenced this issue Dec 24, 2024
@zbud-msft
Ignore control character event if it comes (#21200)
943a4aa 
Why I did it
Fixes #21140

In #20024 and sonic-net/sonic-swss-common#906, we made the change that when a control character is read, zmq_message_read will return with rc 0, which will create an empty internal event.

As part of eventd design, empty structured events are dropped, which leads to control characters being a no-op which is the expected behavior.

In UT, we are still always expecting a control character to be the first message to be read by zmq which is not the case as described in #20024. We are also not ignoring empty events that are read which is being done by eventd.

With this change, control characters are properly ignored if it does after the first test event.

How I did it
Ignore empty structured events and not expect first zmq_message_read to be control character.

How to verify it
Manual test/pipeline
@Junchao-Mellanox
Copy link
Collaborator Author

i ran a few times, looks good

mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this issue Dec 24, 2024
@zbud-msft @mssonicbld
Ignore control character event if it comes (sonic-net#21200)
a40397e 
Why I did it
Fixes sonic-net#21140

In sonic-net#20024 and sonic-net/sonic-swss-common#906, we made the change that when a control character is read, zmq_message_read will return with rc 0, which will create an empty internal event.

As part of eventd design, empty structured events are dropped, which leads to control characters being a no-op which is the expected behavior.

In UT, we are still always expecting a control character to be the first message to be read by zmq which is not the case as described in sonic-net#20024. We are also not ignoring empty events that are read which is being done by eventd.

With this change, control characters are properly ignored if it does after the first test event.

How I did it
Ignore empty structured events and not expect first zmq_message_read to be control character.

How to verify it
Manual test/pipeline
@mssonicbld mssonicbld mentioned this issue Dec 24, 2024
Merged
mssonicbld pushed a commit that referenced this issue Dec 25, 2024
@zbud-msft @mssonicbld
Ignore control character event if it comes (#21200)
d8afbcd 
Why I did it
Fixes #21140

In #20024 and sonic-net/sonic-swss-common#906, we made the change that when a control character is read, zmq_message_read will return with rc 0, which will create an empty internal event.

As part of eventd design, empty structured events are dropped, which leads to control characters being a no-op which is the expected behavior.

In UT, we are still always expecting a control character to be the first message to be read by zmq which is not the case as described in #20024. We are also not ignoring empty events that are read which is being done by eventd.

With this change, control characters are properly ignored if it does after the first test event.

How I did it
Ignore empty structured events and not expect first zmq_message_read to be control character.

How to verify it
Manual test/pipeline
github-actions bot pushed a commit to bradh352/sonic-buildimage that referenced this issue Jan 2, 2025
@zbud-msft @bradh352
Ignore control character event if it comes (sonic-net#21200)
c9dd12e 
Why I did it
Fixes sonic-net#21140

In sonic-net#20024 and sonic-net/sonic-swss-common#906, we made the change that when a control character is read, zmq_message_read will return with rc 0, which will create an empty internal event.

As part of eventd design, empty structured events are dropped, which leads to control characters being a no-op which is the expected behavior.

In UT, we are still always expecting a control character to be the first message to be read by zmq which is not the case as described in sonic-net#20024. We are also not ignoring empty events that are read which is being done by eventd.

With this change, control characters are properly ignored if it does after the first test event.

How I did it
Ignore empty structured events and not expect first zmq_message_read to be control character.

How to verify it
Manual test/pipeline
github-actions bot pushed a commit to bradh352/sonic-buildimage that referenced this issue Jan 2, 2025
@zbud-msft @bradh352
Ignore control character event if it comes (sonic-net#21200)
4406045 
Why I did it
Fixes sonic-net#21140

In sonic-net#20024 and sonic-net/sonic-swss-common#906, we made the change that when a control character is read, zmq_message_read will return with rc 0, which will create an empty internal event.

As part of eventd design, empty structured events are dropped, which leads to control characters being a no-op which is the expected behavior.

In UT, we are still always expecting a control character to be the first message to be read by zmq which is not the case as described in sonic-net#20024. We are also not ignoring empty events that are read which is being done by eventd.

With this change, control characters are properly ignored if it does after the first test event.

How I did it
Ignore empty structured events and not expect first zmq_message_read to be control character.

How to verify it
Manual test/pipeline
github-actions bot pushed a commit to bradh352/sonic-buildimage that referenced this issue Jan 2, 2025
@zbud-msft @bradh352
Ignore control character event if it comes (sonic-net#21200)
6096f5b 
Why I did it
Fixes sonic-net#21140

In sonic-net#20024 and sonic-net/sonic-swss-common#906, we made the change that when a control character is read, zmq_message_read will return with rc 0, which will create an empty internal event.

As part of eventd design, empty structured events are dropped, which leads to control characters being a no-op which is the expected behavior.

In UT, we are still always expecting a control character to be the first message to be read by zmq which is not the case as described in sonic-net#20024. We are also not ignoring empty events that are read which is being done by eventd.

With this change, control characters are properly ignored if it does after the first test event.

How I did it
Ignore empty structured events and not expect first zmq_message_read to be control character.

How to verify it
Manual test/pipeline
VladimirKuk pushed a commit to Marvell-switching/sonic-buildimage that referenced this issue Jan 21, 2025
@zbud-msft @VladimirKuk
Ignore control character event if it comes (sonic-net#21200)
abc9f0e 
Why I did it
Fixes sonic-net#21140

In sonic-net#20024 and sonic-net/sonic-swss-common#906, we made the change that when a control character is read, zmq_message_read will return with rc 0, which will create an empty internal event.

As part of eventd design, empty structured events are dropped, which leads to control characters being a no-op which is the expected behavior.

In UT, we are still always expecting a control character to be the first message to be read by zmq which is not the case as described in sonic-net#20024. We are also not ignoring empty events that are read which is being done by eventd.

With this change, control characters are properly ignored if it does after the first test event.

How I did it
Ignore empty structured events and not expect first zmq_message_read to be control character.

How to verify it
Manual test/pipeline
mssonicbld added a commit to mssonicbld/sonic-buildimage that referenced this issue Jan 27, 2025
@mssonicbld
[eventd] Ignore control character event if it comes in UT
e6a58d9 
<!--
     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

Fixes sonic-net#21140

In sonic-net#20024 and sonic-net/sonic-swss-common#906, we made the change that when a control character is read, zmq_message_read will return with rc 0, which will create an empty internal event.

As part of eventd design, empty structured events are dropped, which leads to control characters being a no-op which is the expected behavior.

In UT, we are still always expecting a control character to be the first message to be read by zmq which is not the case as described in sonic-net#20024. We are also not ignoring empty events that are read which is being done by eventd.

With this change, control characters are properly ignored if it does after the first test event.

##### Work item tracking
- Microsoft ADO **(number only)**:28728116

#### How I did it

Ignore empty structured events and not expect first zmq_message_read to be control character.

#### How to verify it

Manual test/pipeline

<!--
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

#### 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:
-->

<!--
 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 issue Jan 27, 2025
Open
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zbud-msft zbud-msft

Labels
MSFT Triaged this issue has been triaged
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[eventd] Ignore control character event if it comes in UT zbud-msft/sonic-buildimage
4 participants
@anamehra @Junchao-Mellanox @tjchadaga @zbud-msft