attilapiros commented on a change in pull request #23614:
[SPARK-26689][CORE]Support blacklisting bad disk directory and retry in
DiskBlockManager
URL: https://github.com/apache/spark/pull/23614#discussion_r253440206
##########
File path: core/src/main/scala/org/apache/spark/storage/DiskBlockManager.scala
##########
@@ -48,32 +56,69 @@ private[spark] class DiskBlockManager(conf: SparkConf,
deleteFilesOnStop: Boolea
// of subDirs(i) is protected by the lock of subDirs(i)
private val subDirs = Array.fill(localDirs.length)(new
Array[File](subDirsPerLocalDir))
+ private[spark] val badDirs = ArrayBuffer[File]()
+ private[spark] val dirToBlacklistExpiryTime = new HashMap[File, Long]
+ // Filename hash to dirId, it should be small enough to put into memory
+ private[spark] val migratedDirIdIndex = new ConcurrentHashMap[Int,
Int].asScala
+
private val shutdownHook = addShutdownHook()
/** Looks up a file by hashing it into one of our local subdirectories. */
// This method should be kept in sync with
// org.apache.spark.network.shuffle.ExternalShuffleBlockResolver#getFile().
def getFile(filename: String): File = {
+ var mostRecentFailure: Exception = null
// Figure out which local directory it hashes to, and which subdirectory
in that
val hash = Utils.nonNegativeHash(filename)
- val dirId = hash % localDirs.length
+ val dirId = migratedDirIdIndex.getOrElse(hash, hash % localDirs.length)
Review comment:
I think we cannot use hash code as a key for the `migratedDirIdIndex` map
(as if a filename is already mapped then always the same FILE instance should
be given back for it).
Example: Let's assume `alpha` and `beta` are two valid filenames with the
the same hash code.
Let's say first we call `getFile(alpha)` and there is no corruption (a FILE
is given back and some content is written for storing a block) then we call
`getFile(beta)` where corruption is detected so `badDirs` and
`migratedDirIdIndex` are updated. Now comes a reading from `getFile(alpha)`
which because of the `migratedDirIdIndex` gives back a new FILE instance with
empty content.
You can extend your test with some reading and writing of contents.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
