Make use of nio.Files to delete (#448)

use nio.Files for delete Co-authored-by: Suryansh Agnihotri <suryansha@qubole.com>
qubole · Sep 4, 2020 · 266499c · 266499c
1 parent 4771b33
commit 266499c
Showing 1 changed file with 27 additions and 25 deletions.
diff --git a/rubix-bookkeeper/src/main/java/com/qubole/rubix/bookkeeper/FileMetadata.java b/rubix-bookkeeper/src/main/java/com/qubole/rubix/bookkeeper/FileMetadata.java
@@ -28,6 +28,8 @@
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.RandomAccessFile;
+import java.nio.file.Files;
+import java.nio.file.Paths;
 import java.util.OptionalInt;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.locks.Lock;
@@ -134,7 +136,6 @@ private static int findGenerationNumber(String remotePath,
     }
 
     int genNumber;
-    Closer oldFilesRemover = Closer.create();
 
     if (!fileAccessedBloomFilter.mightContain(remotePath)) {
       // first access to the file since BKS started
@@ -148,7 +149,7 @@ private static int findGenerationNumber(String remotePath,
       highestGenNumberOnDisk--;
       if (CacheConfig.isCleanupFilesDuringStartEnabled(conf)) {
         // Pick the generationNumber as one more than the highestGenNumberOnDisk
-        addFilesForDeletion(oldFilesRemover, highestGenNumberOnDisk, remotePath, conf);
+        addFilesForDeletion(highestGenNumberOnDisk, remotePath, conf);
         genNumber = highestGenNumberOnDisk + 1;
       }
       else {
@@ -159,11 +160,11 @@ private static int findGenerationNumber(String remotePath,
         // If both datafile and mdfile exist for highestGenNumberOnDisk, use that as genNumber
         else if (new File(CacheUtil.getLocalPath(remotePath, conf, highestGenNumberOnDisk)).exists() &&
                 new File(CacheUtil.getMetadataFilePath(remotePath, conf, highestGenNumberOnDisk)).exists()) {
-          addFilesForDeletion(oldFilesRemover, highestGenNumberOnDisk - 1, remotePath, conf);
+          addFilesForDeletion(highestGenNumberOnDisk - 1, remotePath, conf);
           genNumber = highestGenNumberOnDisk;
         }
         else {
-          addFilesForDeletion(oldFilesRemover, highestGenNumberOnDisk, remotePath, conf);
+          addFilesForDeletion(highestGenNumberOnDisk, remotePath, conf);
           genNumber = highestGenNumberOnDisk + 1;
         }
       }
@@ -175,23 +176,26 @@ else if (new File(CacheUtil.getLocalPath(remotePath, conf, highestGenNumberOnDis
               new File(CacheUtil.getMetadataFilePath(remotePath, conf, genNumber)).exists()) {
         genNumber++;
       }
-      addFilesForDeletion(oldFilesRemover, genNumber - 1, remotePath, conf);
+      addFilesForDeletion(genNumber - 1, remotePath, conf);
     }
     generationNumberCache.put(remotePath, genNumber);
-    try {
-      oldFilesRemover.close();
-    }
-    catch (IOException e) {
-      log.warn("Exception while deleting old files", e);
-    }
     return genNumber;
   }
 
-  private static void addFilesForDeletion(Closer fileRemover, int generationNumber, String remotePath, Configuration conf)
-  {
+  private static void addFilesForDeletion(int generationNumber, String remotePath, Configuration conf) {
     for (int i = 1; i <= generationNumber; i++) {
-      fileRemover.register(new File(CacheUtil.getLocalPath(remotePath, conf, i))::delete);
-      fileRemover.register(new File(CacheUtil.getMetadataFilePath(remotePath, conf, i))::delete);
+      String localPath = CacheUtil.getLocalPath(remotePath, conf, i);
+      String mdPath = CacheUtil.getMetadataFilePath(remotePath, conf, i);
+      try {
+        Files.delete(Paths.get(localPath));
+      } catch (IOException e) {
+        log.warn(String.format("Exception while deleting old local file %s", localPath), e);
+      }
+      try {
+        Files.delete(Paths.get(mdPath));
+      } catch (IOException e) {
+        log.warn(String.format("Exception while deleting old md file %s ", mdPath), e);
+      }
     }
   }
 
@@ -349,16 +353,14 @@ void deleteFiles(Cache<String, FileMetadata> cache)
     Lock lock = stripes.get(getRemotePath());
     try {
       lock.lock();
-
-      File mdFile = new File(mdFilePath);
-      if (!mdFile.delete()) {
-        log.error("Failed to delete metadata file " + mdFilePath);
-      }
-
-      File localFile = new File(localPath);
-      if (!localFile.delete()) {
-        log.error("Failed to delete local file " + localPath);
-      }
+      Files.delete(Paths.get(mdFilePath));
+    } catch (IOException ex) {
+      log.error(String.format("Could not delete cached files %s", mdFilePath), ex);
+    }
+    try {
+      Files.delete(Paths.get(localPath));
+    } catch (IOException ex) {
+      log.error(String.format("Could not delete cached files %s", localPath), ex);
     }
     finally {
       lock.unlock();