-
Notifications
You must be signed in to change notification settings - Fork 148
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
[#408] test: fix memory check failure in ShuffleBufferManagerTest#bufferSizeTest #657
Conversation
…st#bufferSizeTest
Codecov Report
@@ Coverage Diff @@
## master #657 +/- ##
============================================
+ Coverage 60.60% 62.84% +2.24%
- Complexity 1799 1800 +1
============================================
Files 216 202 -14
Lines 12458 10494 -1964
Branches 1052 1052
============================================
- Hits 7550 6595 -955
+ Misses 4500 3551 -949
+ Partials 408 348 -60 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
// Need to wait for `event.doCleanup` to be executed | ||
// to ensure the correctness of subsequent checks of | ||
// `shuffleBufferManager.getUsedMemory()` and `shuffleBufferManager.getInFlushSize()`. | ||
Thread.sleep(100); |
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.
is there some way to ensure event.doCleanup is executed?
This still could be flaky. But I'm ok to merge if there's no easy way.
server/src/test/java/org/apache/uniffle/server/buffer/ShuffleBufferManagerTest.java
Show resolved
Hide resolved
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.
LGTM
@zuston Could you cherry-pick and commit to branch 0.7? |
…ferSizeTest (#657) ### What changes were proposed in this pull request? Fix memory check failure due to the execution sequence in ShuffleBufferManagerTest#bufferSizeTest ### Why are the changes needed? Flaky test fix: #408 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Existing UTs
…st#bufferSizeTest (apache#657) ### What changes were proposed in this pull request? Fix memory check failure due to the execution sequence in ShuffleBufferManagerTest#bufferSizeTest ### Why are the changes needed? Flaky test fix: apache#408 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Existing UTs
What changes were proposed in this pull request?
Fix memory check failure due to the execution sequence in ShuffleBufferManagerTest#bufferSizeTest
Why are the changes needed?
Flaky test fix: #408
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Existing UTs