Ngone51 commented on a change in pull request #32114:
URL: https://github.com/apache/spark/pull/32114#discussion_r638514447
##########
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:
> Does this actually have an impact ?
Yes, this looks like a no-op as `bmIdsExecutor` should be empty for an
inactive BlockManager.
> Looking more, @Ngone51 do we have a leak w.r.t blockStatusByShuffleService
in this class ?
Is bmInfo.removeBlock helping to reduce impact of the leak ?
I'm not sure about it because I'm now not sure when `bmIdsExtShuffle` is not
empty (or say when the block location can be external shuffle service).
@attilapiros could you give more context here?
--
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]
