Skip to content
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

[BUG] Incorrect cast to int in RemoteSegmentTransferTrackerTests.testStatsObjectCreationViaStream() test #14694

Closed
lukas-vlcek opened this issue Jul 9, 2024 · 1 comment · Fixed by #14696
Labels
bug Something isn't working Storage:Remote untriaged

Comments

@lukas-vlcek
Copy link
Contributor

lukas-vlcek commented Jul 9, 2024

Describe the bug

I am running into the following issue:

./gradlew ':server:test' --tests "org.opensearch.index.remote.RemoteSegmentTransferTrackerTests.testStatsObjectCreationViaStream" -Dtests.security.manager=true -Druntime.java=21
RemoteSegmentTransferTrackerTests > testStatsObjectCreationViaStream FAILED
    java.lang.AssertionError: expected:<2.147483647E9> but was:<5.037149882E9>
        at __randomizedtesting.SeedInfo.seed([17B4E7DE40BB6316:F74F7F9D73752E5]:0)
        at org.junit.Assert.fail(Assert.java:89)
        at org.junit.Assert.failNotEquals(Assert.java:835)
        at org.junit.Assert.assertEquals(Assert.java:555)
        at org.junit.Assert.assertEquals(Assert.java:685)
        at org.opensearch.index.remote.RemoteSegmentTransferTrackerTests.testStatsObjectCreationViaStream(RemoteSegmentTransferTrackerTests.java:580)

The following line is causing the issue:

assertEquals((int) deserializedStats.uploadTimeMovingAverage, transferTrackerStats.uploadTimeMovingAverage, 0);

When I add the following assert before the line 580

assert deserializedStats.uploadTimeMovingAverage <= Integer.MAX_VALUE : "uploadTimeMovingAverage value too large for casting to int";

Then the assert fails

RemoteSegmentTransferTrackerTests > testStatsObjectCreationViaStream FAILED
    java.lang.AssertionError: uploadTimeMovingAverage value too large for casting to int
        at __randomizedtesting.SeedInfo.seed([C0053EEBF80305D6:D8C52ECC6F8F3425]:0)
        at org.opensearch.index.remote.RemoteSegmentTransferTrackerTests.testStatsObjectCreationViaStream(RemoteSegmentTransferTrackerTests.java:580)

What is the point of casting double to int when the original value can be larger than Integer.MAX_VALUE (for example 5037810928 in my case)?

In fact all the following three lines are IMO incorrectly using cast to int:

assertEquals((int) deserializedStats.uploadBytesMovingAverage, transferTrackerStats.uploadBytesMovingAverage, 0);
assertEquals(
(int) deserializedStats.uploadBytesPerSecMovingAverage,
transferTrackerStats.uploadBytesPerSecMovingAverage,
0
);
assertEquals((int) deserializedStats.uploadTimeMovingAverage, transferTrackerStats.uploadTimeMovingAverage, 0);

BTW, compare it to similar three lines from different test from the same class

assertEquals(transferTracker.getUploadBytesMovingAverage(), transferTrackerStats.uploadBytesMovingAverage, 0);
assertEquals(transferTracker.getUploadBytesPerSecMovingAverage(), transferTrackerStats.uploadBytesPerSecMovingAverage, 0);
assertEquals(transferTracker.getUploadTimeMovingAverage(), transferTrackerStats.uploadTimeMovingAverage, 0);

I believe this issues is perhaps quick cut&paste issue or something along the lines.

This issue has been already partially reported in #10014

Feel free to assign this ticket to me, I will send PR. PR sent.

Related component

Storage:Remote

To Reproduce

Please see the description above.

Expected behavior

The test should pass.

Additional Details

Host/Environment (please complete the following information):

  • OS: % uname -a Darwin Lukass-MBP.xxxx.xxxx 22.5.0 Darwin Kernel Version 22.5.0: Mon Apr 24 20:52:24 PDT 2023; root:xnu-8796.121.2~5/RELEASE_ARM64_T6000 arm64
  • Version: main branch
@lukas-vlcek lukas-vlcek added bug Something isn't working untriaged labels Jul 9, 2024
@lukas-vlcek
Copy link
Contributor Author

Relevant autocut ticket: #14325

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Storage:Remote untriaged
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

1 participant