[MINOR] fix(spark): Fix NPE for ShuffleWriteClientImpl.unregisterShuffle #1367
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jerqi
approved these changes
Dec 13, 2023
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #1367 +/- ##
============================================
+ Coverage 53.22% 54.12% +0.90%
Complexity 2715 2715
============================================
Files 418 398 -20
Lines 23932 21573 -2359
Branches 2043 2043
============================================
- Hits 12738 11677 -1061
+ Misses 10408 9181 -1227
+ Partials 786 715 -71 ☔ View full report in Codecov by Sentry. |
xianjingfeng
pushed a commit
that referenced
this pull request
Dec 13, 2023
…fle (#1367) ### What changes were proposed in this pull request? Fix NPE for ShuffleWriteClientImpl.unregisterShuffle. ### Why are the changes needed? Since the `id` of `RssShuffleManager` in Executor is assigned in `getWriter`, this means that the `id` may be null before `getWriter`, so this may cause NPE to occur in `ShuffleWriteClientImpl.unregisterShuffle`. error log: ``` 23/12/12 14:25:47 ERROR RssShuffleManager: Errors on closing shuffle write client java.lang.NullPointerException at java.util.concurrent.ConcurrentHashMap.get(ConcurrentHashMap.java:936) at org.apache.uniffle.client.impl.ShuffleWriteClientImpl.unregisterShuffle(ShuffleWriteClientImpl.java:751) at org.apache.spark.shuffle.RssShuffleManager.stop(RssShuffleManager.java:591) at org.apache.spark.SparkEnv.stop(SparkEnv.scala:90) at org.apache.spark.executor.Executor.stop(Executor.scala:335) at org.apache.spark.executor.CoarseGrainedExecutorBackend$$anonfun$receive$1$$anon$1.run(CoarseGrainedExecutorBackend.scala:203) ``` ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? (cherry picked from commit f17e060)
lifeSo
added a commit
to lifeSo/incubator-uniffle
that referenced
this pull request
Dec 22, 2023
* master: (23 commits) format use synchronized format modify to singleton format format format format format change Coordinator to singleton like ShuffleServerClientFactory.java [apache#1189] feat(server): Add netty used direct memory size metric (apache#1363) format format format rename file rename [apache#960] fix(dashboard): simplify dependency and correct the startup script (apache#1347) [apache#1348] improvement(metrics): Unify tags generation for shuffle-server metrics reporter (apache#1349) [MINOR] chore: fix kubernetes ci pipeline (apache#1368) [MINOR] fix(spark): Fix NPE for ShuffleWriteClientImpl.unregisterShuffle (apache#1367) ...
lifeSo
added a commit
to lifeSo/incubator-uniffle
that referenced
this pull request
Dec 26, 2023
…re_1119_new_2023_12_23 * feature_1119_BUFFER_LIMIT_OF_HUGE_PARTITION: add file for commit [apache#1189] feat(server): Add netty used direct memory size metric (apache#1363) format format format rename file rename [apache#960] fix(dashboard): simplify dependency and correct the startup script (apache#1347) [apache#1348] improvement(metrics): Unify tags generation for shuffle-server metrics reporter (apache#1349) [MINOR] chore: fix kubernetes ci pipeline (apache#1368) [MINOR] fix(spark): Fix NPE for ShuffleWriteClientImpl.unregisterShuffle (apache#1367) format code format code 添加堆外内存监控的代码
zuston
pushed a commit
to zuston/incubator-uniffle
that referenced
this pull request
Jan 18, 2024
…fle (apache#1367) ### What changes were proposed in this pull request? Fix NPE for ShuffleWriteClientImpl.unregisterShuffle. ### Why are the changes needed? Since the `id` of `RssShuffleManager` in Executor is assigned in `getWriter`, this means that the `id` may be null before `getWriter`, so this may cause NPE to occur in `ShuffleWriteClientImpl.unregisterShuffle`. error log: ``` 23/12/12 14:25:47 ERROR RssShuffleManager: Errors on closing shuffle write client java.lang.NullPointerException at java.util.concurrent.ConcurrentHashMap.get(ConcurrentHashMap.java:936) at org.apache.uniffle.client.impl.ShuffleWriteClientImpl.unregisterShuffle(ShuffleWriteClientImpl.java:751) at org.apache.spark.shuffle.RssShuffleManager.stop(RssShuffleManager.java:591) at org.apache.spark.SparkEnv.stop(SparkEnv.scala:90) at org.apache.spark.executor.Executor.stop(Executor.scala:335) at org.apache.spark.executor.CoarseGrainedExecutorBackend$$anonfun$receive$1$$anon$1.run(CoarseGrainedExecutorBackend.scala:203) ``` ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Fix NPE for ShuffleWriteClientImpl.unregisterShuffle.
Why are the changes needed?
Since the
idofRssShuffleManagerin Executor is assigned ingetWriter, this means that theidmay be null beforegetWriter, so this may cause NPE to occur inShuffleWriteClientImpl.unregisterShuffle.error log:
Does this PR introduce any user-facing change?
No.
How was this patch tested?
(Please test your changes, and provide instructions on how to test it: