You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/05/12 16:21:08 UTC
[GitHub] [spark] mridulm commented on a diff in pull request #36512: [SPARK-39152][CORE] Deregistering disk persisted local RDD blocks in case of IO related errors
mridulm commented on code in PR #36512:
URL: https://github.com/apache/spark/pull/36512#discussion_r871575182
##########
core/src/main/scala/org/apache/spark/storage/BlockManager.scala:
##########
@@ -347,7 +347,7 @@ private[spark] class BlockManager(
case ex: KryoException if ex.getCause.isInstanceOf[IOException] =>
// We need to have detailed log message to catch environmental problems easily.
// Further details: https://issues.apache.org/jira/browse/SPARK-37710
Review Comment:
nit: We can remove this comment from here, since it is in `processIORelatedException`.
##########
core/src/main/scala/org/apache/spark/storage/BlockManager.scala:
##########
@@ -933,10 +933,29 @@ private[spark] class BlockManager(
})
Some(new BlockResult(ci, DataReadMethod.Memory, info.size))
} else if (level.useDisk && diskStore.contains(blockId)) {
- try {
- val diskData = diskStore.getBytes(blockId)
- val iterToReturn: Iterator[Any] = {
- if (level.deserialized) {
+ var retryCount = 0
+ val retryLimit = 3
Review Comment:
What kind of transient failures do you expect would recover on retry ? To get a sense of why/how many retries help.
##########
core/src/main/scala/org/apache/spark/storage/BlockManager.scala:
##########
@@ -947,27 +966,42 @@ private[spark] class BlockManager(
.getOrElse { diskData.toInputStream() }
serializerManager.dataDeserializeStream(blockId, stream)(info.classTag)
}
+ } catch {
+ case e: KryoException if e.getCause.isInstanceOf[IOException] =>
+ handleRetriableException(e)
+ case e: IOException =>
+ handleRetriableException(e)
+ case t: Throwable =>
+ if (diskData != null) {
+ diskData.dispose()
+ }
+ // not a retriable exception
Review Comment:
We were not doing it earlier, but release lock 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org