-
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
[#1358] fix(spark): pre-check bytebuffer whether is direct before uncompress #1360
Conversation
PTAL @leixm |
Can you explain why the original code fails to make the ByteBuffer consistent? |
I haven't found the root cause, but I think the bug is related with the Anyway, this PR is to use workaround to fix this. But we still need to dig this more. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #1360 +/- ##
============================================
+ Coverage 53.27% 54.11% +0.84%
- Complexity 2712 2715 +3
============================================
Files 418 398 -20
Lines 23913 21568 -2345
Branches 2043 2045 +2
============================================
- Hits 12739 11672 -1067
+ Misses 10388 9180 -1208
+ Partials 786 716 -70 ☔ View full report in Codecov by Sentry. |
Maybe index BytBbuffer and data ByteBuffer have different types. |
I think no. The possible point has been attached in the issue. And I hope this PR could be merged into the master to prevent the ZSTD error ASSP. And also this should be merged into the 0.8.1 version. |
cc @LuciferYang Could you give us some advice? |
client-spark/common/src/main/java/org/apache/spark/shuffle/reader/RssShuffleDataIterator.java
Show resolved
Hide resolved
client-spark/common/src/main/java/org/apache/spark/shuffle/reader/RssShuffleDataIterator.java
Show resolved
Hide resolved
client-spark/common/src/main/java/org/apache/spark/shuffle/reader/RssShuffleDataIterator.java
Show resolved
Hide resolved
PTAL again @jerqi @LuciferYang |
+1 from myside |
Thanks for your review @jerqi @LuciferYang , Merged. |
…re uncompress (apache#1360) ### What changes were proposed in this pull request? precheck bytebuffer whether is direct before uncompress, and rebuild the temp bytebuffer. ### Why are the changes needed? For apache#1358 . But this PR is not the fundamental solution ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Existing tests.
What changes were proposed in this pull request?
precheck bytebuffer whether is direct before uncompress, and rebuild the temp bytebuffer.
Why are the changes needed?
For #1358 . But this PR is not the fundamental solution
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Existing tests.