-
Notifications
You must be signed in to change notification settings - Fork 153
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
[ISSUE-392] Fix the bug in the shuffle data cleanup checker that causes false reports of disk corruption #393
Conversation
…huffle-server unhealthy
Codecov Report
@@ Coverage Diff @@
## master #393 +/- ##
============================================
- Coverage 58.80% 58.77% -0.04%
Complexity 1602 1602
============================================
Files 193 193
Lines 10939 10939
Branches 955 955
============================================
- Hits 6433 6429 -4
- Misses 4128 4132 +4
Partials 378 378
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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, except one minor comment
@@ -164,7 +164,8 @@ boolean checkStorageReadAndWrite() { | |||
if (storage.isCorrupted()) { | |||
return false; | |||
} | |||
File checkDir = new File(storageDir, "check"); | |||
// Use the hidden file to avoid being cleanup | |||
File checkDir = new File(storageDir, ".check"); |
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.
Nit, make .check
a final static variable, and we can reference this name in test
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.
Done
Thanks @advancedxy |
What changes were proposed in this pull request?
[ISSUE-392] Fix the bug in the shuffle data cleanup checker that causes false reports of disk corruption #393
Why are the changes needed?
Fix the bug in the shuffle data checker that causes false reports of disk corruption during cleanup.
Does this PR introduce any user-facing change?
No
How was this patch tested?