Github user jerryshao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19476#discussion_r144765076
  
    --- Diff: core/src/main/scala/org/apache/spark/storage/BlockManager.scala 
---
    @@ -1552,4 +1582,65 @@ private[spark] object BlockManager {
         override val metricRegistry = new MetricRegistry
         metricRegistry.registerAll(metricSet)
       }
    +
    +  class RemoteBlockTempFileManager(blockManager: BlockManager)
    +      extends TempFileManager with Logging {
    +
    +    private class ReferenceWithCleanup(file: File, referenceQueue: 
JReferenceQueue[File])
    +        extends WeakReference[File](file, referenceQueue) {
    +      private val filePath = file.getAbsolutePath
    +
    +      def cleanUp(): Unit = {
    +        logDebug(s"Clean up file $filePath")
    +
    +        if (!new File(filePath).delete()) {
    +          logDebug(s"Fail to delete file $filePath")
    +        }
    +      }
    +    }
    --- End diff --
    
    I think the overhead is not big, but I'm not sure why there's a file leak 
issue here in `ShuffleBlockFetcherIterator` (the implementation of 
`TempFileManager`). From the code, all the temp files are tracked in 
`shuffleFilesSet`, and will be deleted during `cleanup`, can you please 
elaborate more?


---

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

Reply via email to