Skip to content

[SPARK-3709] Executors don't always report broadcast block removal properly back to the driver #2588

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.

Sign up for GitHub

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

Closed
rxin wants to merge 3 commits into from
Closed

[SPARK-3709] Executors don't always report broadcast block removal properly back to the driver #2588

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2a13f70
iii
rxin Sep 30, 2014
f430686
Always report broadcast removal back to master.
rxin Sep 30, 2014
6dab2e3
Don't log random messages.
rxin Sep 30, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/src/main/scala/org/apache/spark/network/nio/NioBlockTransferService.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,6 @@ final class NioBlockTransferService(conf: SparkConf, securityManager: SecurityMa
val buffer = blockDataManager.getBlockData(blockId).orNull
logDebug("GetBlock " + blockId + " used " + Utils.getUsedTimeMs(startTimeMs)
+ " and got buffer " + buffer)
buffer.nioByteBuffer()
if (buffer == null) null else buffer.nioByteBuffer()
}
}
4 changes: 2 additions & 2 deletions core/src/main/scala/org/apache/spark/storage/BlockManagerSlaveActor.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ class BlockManagerSlaveActor(
SparkEnv.get.shuffleManager.unregisterShuffle(shuffleId)
}

case RemoveBroadcast(broadcastId, tellMaster) =>
case RemoveBroadcast(broadcastId, _) =>
Copy link
Contributor Author

@rxin rxin Sep 30, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem was that the 2nd argument in RemoveBroadcast is not tellMaster! It is "removeFromDriver". Basically when removeFromDriver is not true, we don't report broadcast block removal back to the driver, and then other executors mistakenly think that the executor would still have the block, and try to fetch from it.

doAsync[Int]("removing broadcast " + broadcastId, sender) {
blockManager.removeBroadcast(broadcastId, tellMaster)
blockManager.removeBroadcast(broadcastId, tellMaster = true)
}

case GetBlockStatus(blockId, _) =>
Expand Down