attilapiros commented on a change in pull request #32114:
URL: https://github.com/apache/spark/pull/32114#discussion_r640503459
##########
File path:
core/src/main/scala/org/apache/spark/storage/BlockManagerMasterEndpoint.scala
##########
@@ -273,12 +282,12 @@ class BlockManagerMasterEndpoint(
}
}
bmIdsExecutor.foreach { bmId =>
- blockManagerInfo.get(bmId).foreach { bmInfo =>
+ blockManagerInfo.get(bmId).filter(_.isAlive).foreach { bmInfo =>
Review comment:
@mridulm
No, you are right! Thanks!
Although this is not a critical leak as there is only one
`blockStatusByShuffleService` entry for each Yarn node and probably in most
cases you will get a new executor allocated from an already used node. In
addition the node number is much lower than the number of executors.
But for sure we should fix the small leaks too (we have a proverb for this
in Hungary: "many a little becomes much").
I think it is fine to fix it in a separate issue (I would not block this PR)
so I will create a jira issue for this.
Thanks again!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
