-
Notifications
You must be signed in to change notification settings - Fork 888
Prevent long overflow in SNI address resolution #2035
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
Conversation
@@ -64,7 +64,7 @@ public InetSocketAddress resolve() { | |||
// The order of the returned address is unspecified. Sort by IP to make sure we get a true | |||
// round-robin | |||
Arrays.sort(aRecords, IP_COMPARATOR); | |||
int index = (aRecords.length == 1) ? 0 : (int) OFFSET.getAndIncrement() % aRecords.length; | |||
int index = (aRecords.length == 1) ? 0 : Math.abs((int) OFFSET.getAndIncrement()) % aRecords.length; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is OFFSET
an AtomicLong
, shouldn't it be an AtomicInteger
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, changing.
8a57448
to
d68ec67
Compare
core/src/main/java/com/datastax/oss/driver/internal/core/metadata/SniEndPoint.java
Outdated
Show resolved
Hide resolved
Jenkins build failed due to formatting issues. Just run "mvn com.coveo:fmt-maven-plugin:format" and commit the results... that should be good enough. |
9594562
to
4cdfa39
Compare
Done. |
Kicked off Jenkins tests again now that formatting has been resolved |
I keep wanting to try to make this work with IntStream but the concurrency win from using AtomicInteger is just too big a benefit to overcome. I'll wait until the Jenkins run finishes but assuming there's nothing unexpected there I'm good with this. |
Re-run of Jenkins build after format fixes was as green as expected. There were a few test failures but they were all of the CASSJAVA-95 variety. @lukasz-antoniak can you update the commit message for this guy to include the "patch by" line? I think once we have that this is ready to go in. |
patch by Lukasz Antoniak and Alexandre Dutra; reviewed by Bret McGuire
4cdfa39
to
965f0dd
Compare
Prevent overflowing to negative value.