Fix retry() race conditions #2997
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
To induce failure I run this in a loop: ./gradlew -i -Dtest.single=OperatorRetry cleanTest testOn Ubuntu 14.04 64-bit i7 i7 CPU 920 @ 2.67GHz × 8, home desktop, java 1.8u45: ? tested with wrong branch version, testing again ! On Ubuntu 14.04 64-bit i5 CPU U 470 @ 1.33GHz × 4, laptop, java 1.8u45: 238th run: |
This was referenced Jun 2, 2015
|
I still don't understand the cause of the failure. The best thing we can do is to increase that await timeout in the test to 5-7 seconds because this seems to be some transient hiccup rather than a hang. Otherwise, support this PR but defer the judgement to @benjchristensen . |
|
removed last comment, ran on wrong branch |
|
I bumped up await timeout to 5 minutes and the 54th run failed in 5 mins 9.9 seconds (on home desktop): rx.internal.operators.OperatorRetryTest > testRetryWithBackpressureParallel FAILED
java.lang.AssertionError: Data content mismatch: 950={beginningEveryTime x 136}
at org.junit.Assert.fail(Assert.java:93)
at rx.internal.operators.OperatorRetryTest.testRetryWithBackpressureParallel(OperatorRetryTes
t.java:768)5 minutes is a long time for some OS hiccup and makes me think that the freeze is occurring because of our code. I'll run another test and dump all threads after the freeze. |
|
Then we need a full rewrite, but I don't have time for it for at least 3 months. |
|
Thread dump after freeze occuring on 84th run on home desktop: |
Closed
|
At the moment I don't suspect problems with the
I've been looking elsewhere and now my suspicions lie with the gradle infrastructure. I believe the tests for This is an unusual case inasmuch as we don't normally rely on tests looping on gradle tests but rather run loops in a single JVM. A fresh JVM can sometimes expose race conditions that aren't as easily exposed by a warmed-up JVM but I think this is being pushed a bit far and might be exposing us to rarely encountered gradle bugs too. I think |
|
I think it is as good as it can get for now. I think we can return to the case when we port things over to RxJava 2.0. Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the continuation of #2930 with a rebased PR.
There are sporadic testing failures with this PR so not ready for merge. I'll note some failures with this one soon.