Ngone51 commented on a change in pull request #29014:
URL: https://github.com/apache/spark/pull/29014#discussion_r458860168
##########
File path: core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala
##########
@@ -1767,8 +1767,13 @@ private[spark] class DAGScheduler(
// TODO: mark the executor as failed only if there were lots of
fetch failures on it
if (bmAddress != null) {
- val hostToUnregisterOutputs = if
(env.blockManager.externalShuffleServiceEnabled &&
- unRegisterOutputOnHostOnFetchFailure) {
+ val externalShuffleServiceEnabled =
env.blockManager.externalShuffleServiceEnabled
+ val isHostDecommissioned = taskScheduler
+ .getExecutorDecommissionInfo(bmAddress.executorId)
+ .exists(_.isHostDecommissioned)
Review comment:
> if (host is decommissioned) {
// clear shuffle state for the entire host
} else if (!isLocalFetchEnabled || numRemainingLiveExecutorsOnThisHost ==
0) {
// clear shuffle state for just this executor that suffered the fetch
failure
// because we can be sure that no one else will serve this executor's
data
}
> should implement the optimization of whether or not a shuffle state should
be cleared when you get a fetchf failure from an executor
These're actually good idea! I just have a few concerns that whether they
are doable with current implementation... I mean, when fetch failure happens on
an executor, the possible reasons may be executor lost or shuffle data corrupt.
For these two reasons, host-local shuffle reading could still result in fetch
failure at the end. Because, even if we use host-local shuffle data reading, we
need to ask the lost executor for the directory first. And if the shuffle data
is corrupted, we'll fail to read the shuffle data anyway. Though, in the case
of executor lost, it's maybe doable as we have local disk directories cache
locally so that the executor on the same host won't ask the lost executor for
the directory. However, the problem is that the driver is not aware of the
status of the cache. Thus, the driver also has no idea whether we should clean
up the shuffle state for the lost executor or not.
----------------------------------------------------------------
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]
