-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-39988][CORE] Use try-with-resource to ensure DBIterator is close after use
#37420
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
Conversation
try-with-resource to ensure DBIterator is close after use in RemoteBlockPushResolver, YarnShuffleService and ExternalShuffleBlockResolvertry-with-resource to ensure DBIterator is close after use in RemoteBlockPushResolver, YarnShuffleService and ExternalShuffleBlockResolver
try-with-resource to ensure DBIterator is close after use in RemoteBlockPushResolver, YarnShuffleService and ExternalShuffleBlockResolvertry-with-resource to ensure DBIterator is close after use in RemoteBlockPushResolver, YarnShuffleService and ExternalShuffleBlockResolver
| throws IOException { | ||
| ConcurrentMap<AppExecId, ExecutorShuffleInfo> registeredExecutors = Maps.newConcurrentMap(); | ||
| if (db != null) { | ||
| DBIterator itr = db.iterator(); |
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.
The methods reloadActiveAppAttemptsPathInfo and reloadFinalizedAppAttemptsShuffleMergeInfo should be Spark 3.4 only. Please let me know if this PR needs to be backport to other branch
|
cc @dongjoon-hyun @mridulm Changes of this pr is for bug fix, which is not within the refactor scope of SPARK-38909 |
try-with-resource to ensure DBIterator is close after use in RemoteBlockPushResolver, YarnShuffleService and ExternalShuffleBlockResolvertry-with-resource to ensure DBIterator is close after use
dongjoon-hyun
left a comment
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.
Looks reasonable to me, but we had better ping more people in this area, @LuciferYang .
mridulm
left a comment
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.
Looks good to me
|
Merged to master. |
Sorry for not reply at the weekend, this issue exists in the historical version, do we need a backport? |
|
No, I don't think so for now, @LuciferYang . Let's keep this in master branch for Apache Spark 3.4.0 first. |
|
OK |
What changes were proposed in this pull request?
This pr use
try-with-resourceto ensureDBIteratoris close after use inRemoteBlockPushResolver,YarnShuffleServiceandExternalShuffleBlockResolverto avoid resource leakage.Why are the changes needed?
Avoid
DBIteratorresource leakage.Does this PR introduce any user-facing change?
No
How was this patch tested?
Pass GitHub Actions