[GitHub] squito commented on a change in pull request #23614: [SPARK-26689][CORE]Support blacklisting bad disk directory and retry in DiskBlockManager

[GitBox <gi...@apache.org>]

squito 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_r251961563
 
 

 ##########
 File path: core/src/main/scala/org/apache/spark/storage/DiskBlockManager.scala
 ##########
 @@ -48,33 +50,57 @@ 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 val badDirs = ArrayBuffer[File]()
+  private val maxRetries = conf.get(config.DISK_STORE_MAX_RETIRES)
+  private val blacklistTimeout = conf.get(config.DISK_STORE_BLACKLIST_TIMEOUT)
+  private val dirToBlacklistExpiryTime = new HashMap[File, Long]
+
   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 = {
-    // Figure out which local directory it hashes to, and which subdirectory in that
-    val hash = Utils.nonNegativeHash(filename)
-    val dirId = hash % localDirs.length
-    val subDirId = (hash / localDirs.length) % subDirsPerLocalDir
-
-    // Create the subdirectory if it doesn't already exist
-    val subDir = subDirs(dirId).synchronized {
-      val old = subDirs(dirId)(subDirId)
-      if (old != null) {
-        old
-      } else {
-        val newDir = new File(localDirs(dirId), "%02x".format(subDirId))
-        if (!newDir.exists() && !newDir.mkdir()) {
-          throw new IOException(s"Failed to create local dir in $newDir.")
+    var mostRecentFailure: Exception = null
+    // Update blacklist
+    val now = System.currentTimeMillis()
+    badDirs.dropWhile(now > dirToBlacklistExpiryTime(_))
 
 Review comment:
   this method needs to be threadsafe, so `badDirs` and `dirToBlacklistExpiryTime` have to be protected or handle that somehow.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org