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());