Author: amitj
Date: Fri Dec  8 06:20:45 2017
New Revision: 1817457

URL: http://svn.apache.org/viewvc?rev=1817457&view=rev
Log:
OAK-7038: Make deletion of temp files resilient in some common utility methods

 Moved the deletion to the finally block

Modified:
    
jackrabbit/oak/trunk/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/FileIOUtils.java
    
jackrabbit/oak/trunk/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/sort/ExternalSort.java
    
jackrabbit/oak/trunk/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/FileIOUtilsTest.java

Modified: 
jackrabbit/oak/trunk/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/FileIOUtils.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/FileIOUtils.java?rev=1817457&r1=1817456&r2=1817457&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/FileIOUtils.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/FileIOUtils.java
 Fri Dec  8 06:20:45 2017
@@ -172,13 +172,13 @@ public final class FileIOUtils {
                     closeQuietly(iStream);
                 }
             }
+            threw = false;
+        } finally {
             if (delete) {
                 for (File f : files) {
                     f.delete();
                 }
             }
-            threw = false;
-        } finally {
             close(appendStream, threw);
         }
     }

Modified: 
jackrabbit/oak/trunk/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/sort/ExternalSort.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/sort/ExternalSort.java?rev=1817457&r1=1817456&r2=1817457&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/sort/ExternalSort.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/sort/ExternalSort.java
 Fri Dec  8 06:20:45 2017
@@ -459,7 +459,7 @@ public class ExternalSort {
     }
 
     /**
-     * This merges a bunch of temporary flat files
+     * This merges a bunch of temporary flat files and deletes them on success 
or error.
      * 
      * @param files
      *            The {@link List} of sorted {@link File}s to be merged.
@@ -488,28 +488,28 @@ public class ExternalSort {
                                            boolean append, boolean usegzip, 
Function<T, String> typeToString,
                                            Function<String, T> stringToType) 
throws IOException {
         ArrayList<BinaryFileBuffer<T>> bfbs = new ArrayList<>();
-        for (File f : files) {
-            final int bufferSize = 2048;
-            InputStream in = new FileInputStream(f);
-            BufferedReader br;
-            if (usegzip) {
-                br = new BufferedReader(new InputStreamReader(
-                        new GZIPInputStream(in, bufferSize), cs));
-            } else {
-                br = new BufferedReader(new InputStreamReader(in,
-                        cs));
-            }
+        try {
+            for (File f : files) {
+                final int bufferSize = 2048;
+                InputStream in = new FileInputStream(f);
+                BufferedReader br;
+                if (usegzip) {
+                    br = new BufferedReader(new InputStreamReader(new 
GZIPInputStream(in, bufferSize), cs));
+                } else {
+                    br = new BufferedReader(new InputStreamReader(in, cs));
+                }
 
-            BinaryFileBuffer<T> bfb = new BinaryFileBuffer<>(br, stringToType);
-            bfbs.add(bfb);
-        }
-        BufferedWriter fbw = new BufferedWriter(new OutputStreamWriter(
-                new FileOutputStream(outputfile, append), cs));
-        int rowcounter = merge(fbw, cmp, distinct, bfbs, typeToString);
-        for (File f : files) {
-            f.delete();
+                BinaryFileBuffer<T> bfb = new BinaryFileBuffer<>(br, 
stringToType);
+                bfbs.add(bfb);
+            }
+            BufferedWriter fbw = new BufferedWriter(new OutputStreamWriter(new 
FileOutputStream(outputfile, append), cs));
+            int rowcounter = merge(fbw, cmp, distinct, bfbs, typeToString);
+            return rowcounter;
+        } finally {
+            for (File f : files) {
+                f.delete();
+            }
         }
-        return rowcounter;
     }
 
     /**

Modified: 
jackrabbit/oak/trunk/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/FileIOUtilsTest.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/FileIOUtilsTest.java?rev=1817457&r1=1817456&r2=1817457&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/FileIOUtilsTest.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/FileIOUtilsTest.java
 Fri Dec  8 06:20:45 2017
@@ -49,6 +49,7 @@ import org.apache.jackrabbit.oak.commons
 import org.junit.Assert;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.junit.rules.TemporaryFolder;
 
 import static com.google.common.base.Charsets.UTF_8;
@@ -59,6 +60,7 @@ import static org.apache.jackrabbit.oak.
 import static org.apache.jackrabbit.oak.commons.FileIOUtils.copy;
 import static org.apache.jackrabbit.oak.commons.FileIOUtils.lexComparator;
 import static 
org.apache.jackrabbit.oak.commons.FileIOUtils.lineBreakAwareComparator;
+import static org.apache.jackrabbit.oak.commons.FileIOUtils.merge;
 import static org.apache.jackrabbit.oak.commons.FileIOUtils.readStringsAsSet;
 import static org.apache.jackrabbit.oak.commons.FileIOUtils.sort;
 import static org.apache.jackrabbit.oak.commons.FileIOUtils.writeStrings;
@@ -271,6 +273,22 @@ public class FileIOUtilsTest {
     }
 
     @Test
+    public void appendTestFileDeleteOnError() throws IOException {
+        Set<String> added2 = newHashSet("2", "3", "5", "6");
+        File f2 = assertWrite(added2.iterator(), false, added2.size());
+
+        Set<String> added3 = newHashSet("t", "y", "8", "9");
+        File f3 = assertWrite(added3.iterator(), false, added3.size());
+
+        try {
+            append(newArrayList(f2, f3), null, true);
+        } catch (Exception e) {
+        }
+        assertTrue(!f2.exists());
+        assertTrue(!f3.exists());
+    }
+
+    @Test
     public void appendRandomizedTest() throws Exception {
         Set<String> added1 = newHashSet();
         for (int i = 0; i < 100; i++) {
@@ -301,6 +319,22 @@ public class FileIOUtilsTest {
     }
 
     @Test
+    public void mergeWithErrorsTest() throws IOException {
+        Set<String> added2 = newHashSet("2", "3", "5", "6");
+        File f2 = assertWrite(added2.iterator(), false, added2.size());
+
+        Set<String> added3 = newHashSet("t", "y", "8", "9");
+        File f3 = assertWrite(added3.iterator(), false, added3.size());
+
+        try {
+            merge(newArrayList(f2, f3), null);
+        } catch(Exception e) {}
+
+        assertTrue(!f2.exists());
+        assertTrue(!f3.exists());
+    }
+
+    @Test
     public void fileIteratorTest() throws Exception {
         Set<String> added = newHashSet("a", "z", "e", "b");
         File f = assertWrite(added.iterator(), false, added.size());


Reply via email to