-
Notifications
You must be signed in to change notification settings - Fork 134
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
[#886] fix(mr): MR Client may lost data or throw exception when rss.storage.type without MEMORY. #887
Conversation
…age.type without MEMORY.
You can use mockClient to test this issue like |
Codecov Report
@@ Coverage Diff @@
## master #887 +/- ##
============================================
+ Coverage 56.20% 57.06% +0.86%
- Complexity 2150 2197 +47
============================================
Files 327 313 -14
Lines 15905 14084 -1821
Branches 1247 1306 +59
============================================
- Hits 8939 8037 -902
+ Misses 6466 5606 -860
+ Partials 500 441 -59
... and 43 files with indirect coverage changes 📣 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, thanks @zhengchenyu , wait for CI. Merged to master and branch-0.7.
…torage.type without MEMORY. (#887) ### What changes were proposed in this pull request? Make sure finishShuffle after send all shuffle data. ### Why are the changes needed? If type without MEMORY, some data will never flush. ### How was this patch tested? I test in two mode: * Tez local debug mode * MR on yarn mode Add new UT Co-authored-by: zhengchenyu001 <zhengchenyu001@ke.com>
Branch 0.7 has conflicts when I cherry-pick this pr. Considering this fix is used for storage without MEMORY. These storage types isn't recommended for production environment. This pr won't be merged to branch 0.7. |
…ion when rss.storage.type without MEMORY. (apache#887)" This reverts commit 4423b43.
What changes were proposed in this pull request?
Make sure finishShuffle after send all shuffle data.
Why are the changes needed?
If type without MEMORY, some data will never flush.
How was this patch tested?
I test in two mode:
Add new UT