Make LeastActiveBalancingPolicy.ShufflingCompare comparisons stable #646
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
…o avoid a problem where
mutable state in ConcurrentHClientPool causes
IllegalArgumentException: Comparison method violates its general contract!
failures when running in Java 7.
zznate
added a commit
that referenced
this pull request
Jan 13, 2014
Make LeastActiveBalancingPolicy.ShufflingCompare comparisons stable
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 avoids a problem where mutable state in ConcurrentHClientPool causes
failures when running in Java 7.
The default sort algorithm changed in Java 7 (see this blog post for details and links). The new TimSort algorithm is pickier about the Comparable and Comparator honoring their contract. It appears that because ConcurrentHClientPool. getNumActive can return different values while the sort is going on, that can trigger the error.
We've ran into this issue immediately after deploying one of our apps in production on Java 7. We have worked around the issue by setting the system property
java.util.Arrays.useLegacyMergeSort=true. We did not see the issue in our pre-production environments. Looking at the Java 7 source the new algorithm is only used when the list to be sorted is 32 or more elements. Our production cluster has 36 hosts in the list, while our pre-production clusters are much smaller.I've included a test which shows the error when run agains the old ShufflingCompare implementation in Java 7. It passes with this new implementation or with the old one under Java 6.
My solution was to cache the compared values for the duration of the sort. If someone has a better idea, I'm all ears.
I'd like to see this backported to the 1.0 branch if possible. The commit should apply cleanly.