WIP test: make mockscheduler test more reliable by amitiuttarwar · Pull Request #18174 · bitcoin/bitcoin

amitiuttarwar · 2020-02-18T17:12:20Z

The scheduler_tests/mockforward test introduced in #18037 sometimes fails on the x86_64 Linux machine with no wallet.

This is an attempt to fix by changing the order of events in the tests & starting the scheduler thread before queueing events.

Unfortunately I'm unable to reproduce locally, so I'm opening this PR & would like to get a few green runs to see if this is actually the fix.

theStack · 2020-02-18T17:26:14Z

Just as a general hint for the approach of "get a few green runs", it's also possible to register at travis-ci.org with your github account and add your forked bitcoin repository there: then on every push the CI will be triggered as well. This proved to be very helpful for me personally, as check before I opened PRs, as I wrongly assumed for some reason that the CI runs can only be triggered here on the main bitcoin repository.

amitiuttarwar · 2020-02-18T17:30:05Z

thanks for the tip @theStack !

so you were able to get travis passing on your forked repo? when I tried previously I wouldn't get meaningful results, eg. they would frequently time out or error for unrelated reasons. But I didn't investigate too closely, so I can take another look

theStack · 2020-02-18T17:52:19Z

so you were able to get travis passing on your forked repo? when I tried previously I wouldn't get meaningful results, eg. they would frequently time out or error for unrelated reasons. But I didn't investigate too closely, so I can take another look

It could be that on a first build you have to restart the job manually from the Travis dashboard, if a message like this appears at the very bottom of the job log:

Error! Initial build successful, but not enough time remains to run later build stages and tests.
See https://docs.travis-ci.com/user/customizing-the-build#build-timeouts . Please manually re-run
this job by using the travis restart button. The next run should not time out because the build cache
has been saved.

Other than that, after this initial hurdle I think a Travis run here should behave the same as a run from your bitcoin fork -- at least that's my experience :)

maflcko · 2020-02-18T19:16:04Z

src/test/scheduler_tests.cpp

-    std::thread scheduler_thread([&]() { scheduler.serviceQueue(); });
-
    // bump the scheduler forward 5 minutes
    scheduler.MockForward(boost::chrono::seconds(5*60));


It looks like this line (or any line after it, but before the next BOOST_CHECK) still fails. E.g.

Running 3 test cases... Test cases order is shuffled using seed: 902263820 Entering test module "Bitcoin Core Test Suite" test/scheduler_tests.cpp(11): Entering test suite "scheduler_tests" test/scheduler_tests.cpp(156): Entering test case "mockforward" terminate called after throwing an instance of 'boost::wrapexcept<boost::condition_error>' what(): boost::condition_variable::do_wait_until failed in pthread_cond_timedwait: Invalid argument unknown location(0): fatal error: in "scheduler_tests/mockforward": signal: SIGABRT (application abort requested) test/scheduler_tests.cpp(173): last checkpoint test/scheduler_tests.cpp(156): Leaving test case "mockforward"; testing time: 430us test/scheduler_tests.cpp(112): Entering test case "singlethreadedscheduler_ordered" test/scheduler_tests.cpp(112): Leaving test case "singlethreadedscheduler_ordered"; testing time: 7233us test/scheduler_tests.cpp(38): Entering test case "manythreads" test/scheduler_tests.cpp(38): Leaving test case "manythreads"; testing time: 8082us test/scheduler_tests.cpp(11): Leaving test suite "scheduler_tests"; testing time: 15837us Leaving test module "Bitcoin Core Test Suite"; testing time: 15956us

maflcko · 2020-02-18T23:16:56Z

I tried on three machines and could not reproduce the issue locally

maflcko · 2020-02-19T00:56:24Z

From @jonasschnelli on IRC: Fails also on https://bitcoinbuilds.org/?job=cda0e018-8437-4dea-80b8-283fd445ec8a

maflcko · 2020-02-19T01:21:22Z

@jonasschnelli Can the build be reproduced locally? Is it using depends or system boost?

jonasschnelli · 2020-02-19T18:09:14Z

I looked a bit into it.
This PR seems still to fail on bitcoinbuilds.org:
https://bitcoinbuilds.org/index.php?job=2f3c8572-cbaf-4cab-a2d2-20809d713084

What I observed was that only the depends-build configuration fail (system-libs configuration works). If I clear the ccache-cache, the build runs successful.

Maybe that helps identifying the issue.

maflcko · 2020-02-19T18:14:00Z

If I clear the ccache-cache, the build runs successful.

I think the failure is intermittent, so it might be hard to draw conclusions early.

jonasschnelli · 2020-02-19T18:19:13Z

I think the failure is intermittent, so it might be hard to draw conclusions early.

It seems to fail on bitcoinbuilds.org 100% on "Linux x86_64 depends" "Linux 32 depends" when using ccache/dependency cache.

maflcko · 2020-02-20T00:57:31Z

I am finally able to reproduce on a single CPU instance on gce n1-standard-1 (1 vCPU, 3.75 GB memory)

I am using depends with NO_QT=1 NO_WALLET=1 and ccache. The bug is only reproducible once after each reboot with: cd bitcoin/ && while sudo bash -c 'rm -f ./src/test/test_bitcoin && (echo 1 > /proc/sys/vm/drop_caches) && make -j 2 check' ; do true ; done

I could not reproduce it when attaching gdb, running in valgrind or when enabling core dumps (in combination with #18183 )

practicalswift · 2020-02-20T13:25:19Z

@jonasschnelli @MarcoFalke I'm unable to reproduce :(

What toolchain are you compiling using? Is there any theory as to why the use of ccache would matter?

practicalswift · 2020-02-20T13:34:59Z

Also, are you able to trigger it when doing src/test/test_bitcoin -t scheduler_tests?

…r now fab2527 test: Disable mockforward scheduler unit test for now (MarcoFalke) Pull request description: This should be a workaround to fix bitcoin#18174 in the short run and buy us more time to investigate the issue while ci runs are green again 🙏 ACKs for top commit: fanquake: ACK fab2527 - be good to get Travis back. laanwj: ACK fab2527 Tree-SHA512: 027e86b3dfec203a464e5bf528e9933c208c36633c2d4bfcdbc10da1799637a5d6ea0a63af33a4174fb1ad7115df631a4cb838f56e31f4cbd15498e1e9fdf9cc

amitiuttarwar · 2020-02-28T20:40:20Z

thank you all for your help with this bug !

I opened issue #18227 to track findings & continue investigation

…r now fab2527 test: Disable mockforward scheduler unit test for now (MarcoFalke) Pull request description: This should be a workaround to fix bitcoin#18174 in the short run and buy us more time to investigate the issue while ci runs are green again 🙏 ACKs for top commit: fanquake: ACK fab2527 - be good to get Travis back. laanwj: ACK fab2527 Tree-SHA512: 027e86b3dfec203a464e5bf528e9933c208c36633c2d4bfcdbc10da1799637a5d6ea0a63af33a4174fb1ad7115df631a4cb838f56e31f4cbd15498e1e9fdf9cc

start scheduler thread before queuing jobs

df29a71

DrahtBot added the Tests label Feb 18, 2020

maflcko reviewed Feb 18, 2020

View reviewed changes

jonasschnelli mentioned this pull request Feb 19, 2020

Util: Allow scheduler to be mocked #18037

Merged

This was referenced Feb 20, 2020

test: Bump process time in scheduler_tests to avoid race #18188

Closed

test: Disable mockforward scheduler unit test for now #18211

Merged

fanquake closed this in 4502ed7 Feb 27, 2020

fanquake closed this in #18211 Feb 27, 2020

amitiuttarwar deleted the 2020-02-fix-scheduler-test branch February 28, 2020 20:21

amitiuttarwar mentioned this pull request Feb 28, 2020

scheduler test causing pthread_cond_timedwait: Invalid argument error #18227

Closed

bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022

Conversation

amitiuttarwar commented Feb 18, 2020

Uh oh!

theStack commented Feb 18, 2020

Uh oh!

amitiuttarwar commented Feb 18, 2020

Uh oh!

theStack commented Feb 18, 2020

Uh oh!

maflcko Feb 18, 2020

Choose a reason for hiding this comment

Uh oh!

maflcko commented Feb 18, 2020

Uh oh!

maflcko commented Feb 19, 2020

Uh oh!

maflcko commented Feb 19, 2020

Uh oh!

jonasschnelli commented Feb 19, 2020

Uh oh!

maflcko commented Feb 19, 2020

Uh oh!

jonasschnelli commented Feb 19, 2020

Uh oh!

maflcko commented Feb 20, 2020

Uh oh!

practicalswift commented Feb 20, 2020

Uh oh!

practicalswift commented Feb 20, 2020

Uh oh!

amitiuttarwar commented Feb 28, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants