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.

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

Fix flaky subscription test #98

Merged
merged 1 commit into from
Dec 18, 2015
Merged

Fix flaky subscription test #98

merged 1 commit into from
Dec 18, 2015

Conversation

gerkey
Copy link
Member

@gerkey gerkey commented Dec 18, 2015

Fix flaky subscription test by adding:

  • a 1ms sleep between setup and the start of publishing; and
  • a maximum-2s loop of 10ms sleeps to wait for message delivery.

Both features appear to be required to ensure reliable test results with Connext when the system is under load.

These tests fail under CI most often with Connext on OSX (e.g., http://ci.ros2.org/job/ci_osx/679/testReport/%28root%29/projectroot/gtest_subscription__rmw_connext_cpp/). To reproduce locally, I load my 8-core Linux machine with stress -c 8. I consider the problem "fixed" when the test passes under those conditions.

* a 1ms sleep between setup and the start of publishing; and
* a maximum-2s loop of 10ms sleeps to wait for message delivery.
Both features appear to be required to ensure reliable test results when the
system is under load (e.g., `stress -c 8` on an 8-core machine).
@gerkey gerkey added the in progress Actively being worked on (Kanban column) label Dec 18, 2015
@gerkey
Copy link
Member Author

gerkey commented Dec 18, 2015

FYI, the sleep-loop structure is the same as was used in #94.

@gerkey gerkey added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Dec 18, 2015
@wjwwood
Copy link
Member

wjwwood commented Dec 18, 2015

The changes look good, here is an OS X job with just Connext enabled (mostly to ensure that this didn't break the test, not to check that the flakiness is gone): http://ci.ros2.org/job/ci_osx/685/

In parallel I'll test it locally.

@gerkey
Copy link
Member Author

gerkey commented Dec 18, 2015

Sneak peak from that CI job:

22:09:59  5/31 Test  #5: gtest_subscription__rmw_connext_cpp ............................   Passed   20.70 sec
22:12:02 18/31 Test #18: gtest_subscription__rmw_connext_dynamic_cpp ....................   Passed   20.68 sec

@wjwwood
Copy link
Member

wjwwood commented Dec 18, 2015

It's green. +1

gerkey added a commit that referenced this pull request Dec 18, 2015
@gerkey gerkey merged commit f8434e6 into master Dec 18, 2015
@gerkey gerkey removed the in review Waiting for review (Kanban column) label Dec 18, 2015
@gerkey gerkey deleted the fix_flaky_subscription_test branch December 18, 2015 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants