-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Change number of shards and fix typo #4980
Conversation
Gradle Check (Jenkins) Run Completed with:
|
Please fix DCO and see CI failures above. Thanks. |
@iofit Can you explain why this change is the right thing to do? Is there a bug? Or is this just a test misconfiguration? |
Hi @andrross , I believe there's a bug not only on this matter but others, and it's mainly related to java 17 and how math works on it, I've seen that in java17 math doesn't work the same as in java11 for example, I'm not sure what the bug is but I read somewhere that it might be related to the little endian and big endian calculations or perhaps the floating point operations. |
PRed #5256 with signed DCO and confirmed this fixes the bug. |
This still fails:
|
@@ -952,7 +952,7 @@ public void testWeightedOperationRoutingWeightUndefinedForOneZone() throws Excep | |||
selectedNodes = new HashSet<>(); | |||
setting = Settings.builder().put("cluster.routing.allocation.awareness.attributes", "zone").build(); |
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.
We seem to define setting
above already.
@iofit Are you still working on this? This is easily reproduced. Can you help us understand where the math matters? Where's the calculation that produces different results and what's expected? Write a unit test for that calculation? |
@anshu1106 looks like this was introduced in #4241, can you please take a look? |
Resolved by #5289 |
@anshu1106 seems this issue is resolved by #5289. Can we close this PR? Correct me if i am mistaken. |
Yes, we can |
Description
Fixes issue with test catched by specific build env:
./gradlew ':server:test' --tests "org.opensearch.cluster.routing.OperationRoutingTests.testWeightedOperationRoutingWeightUndefinedForOneZone" -Dtests.seed=B1745FEE14F29C02 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=ar-EG -Dtests.timezone=Asia/Magadan -Druntime.java=17
Issues Resolved
#4881
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.