events: catch and ignore DirectoryIteratorException
A DirectoryIteratorException wraps an IOException for a case where the
method is not allowed to throw an IOException. For many cases of our
error handling, treat it the same as an IOException by throwing the
wrapped IOException. Also clean up some usages of IOException handling
where it should be propagated further up.
Change-Id: I34b386dbd7bc1ab6cb890ab4c9616b37585a4f52
diff --git a/src/main/java/com/googlesource/gerrit/plugins/events/fsstore/Fs.java b/src/main/java/com/googlesource/gerrit/plugins/events/fsstore/Fs.java
index 85334bd..f3f7444 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/events/fsstore/Fs.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/events/fsstore/Fs.java
@@ -17,6 +17,7 @@
import java.io.File;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
+import java.nio.file.DirectoryIteratorException;
import java.nio.file.DirectoryStream;
import java.nio.file.FileAlreadyExistsException;
import java.nio.file.FileVisitResult;
@@ -78,7 +79,7 @@
/**
* Try to recursively delete entries, up to max count, in a dir older than expiry. Do NOT throw
- * IOExceptions.
+ * IOExceptions or DirectoryIteratorExceptions.
*
* @return whether all entries were deleted
*/
@@ -93,12 +94,18 @@
}
}
} catch (IOException e) { // Intent of 'try' function is to ignore these.
+ } catch (DirectoryIteratorException e) {
+ // dir was deleted by another actor, thus so were all its entries
}
return true;
}
- /** Are all entries in a directory tree older than expiry? Do NOT throw IOExceptions. */
- public static boolean isAllEntriesOlderThan(Path dir, FileTime expiry) {
+ /**
+ * Are all entries in a directory tree older than expiry?
+ *
+ * @throws IOException
+ */
+ public static boolean isAllEntriesOlderThan(Path dir, FileTime expiry) throws IOException {
if (!isOlderThan(dir, expiry)) {
return false;
}
@@ -109,8 +116,8 @@
}
}
} catch (NotDirectoryException e) { // can't recurse if not a directory
- } catch (IOException e) {
- return false; // Modified after start, so not older
+ } catch (DirectoryIteratorException e) {
+ throw e.getCause(); // Throw the causal checked exception
}
return true;
}
@@ -182,6 +189,8 @@
try (DirectoryStream<Path> dirEntries = Files.newDirectoryStream(dir)) {
Iterator<Path> it = dirEntries.iterator();
return it.hasNext() ? it.next() : null;
+ } catch (DirectoryIteratorException e) {
+ throw e.getCause(); // Throw the causal checked exception
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/events/fsstore/FsTransaction.java b/src/main/java/com/googlesource/gerrit/plugins/events/fsstore/FsTransaction.java
index cb7192b..dc7bc0e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/events/fsstore/FsTransaction.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/events/fsstore/FsTransaction.java
@@ -15,6 +15,7 @@
package com.googlesource.gerrit.plugins.events.fsstore;
import java.io.IOException;
+import java.nio.file.DirectoryIteratorException;
import java.nio.file.DirectoryStream;
import java.nio.file.Files;
import java.nio.file.Path;
@@ -124,9 +125,10 @@
/**
* Used to atomically delete entries in a directory tree older than expiry, up to max count. Do
- * NOT throw IOExceptions.
+ * NOT throw DirectoryIteratorExceptions.
*
* @return whether all entries were deleted
+ * @throws IOException
*/
public static boolean renameAndDeleteEntriesOlderThan(
Path dir, Path del, FileTime expiry, int max) throws IOException {
@@ -139,7 +141,10 @@
renameAndDelete(path, del);
}
}
- return true;
+ } catch (DirectoryIteratorException e) {
+ // dir was deleted by another actor, thus so were all its entries
+ Nfs.throwIfNotStaleFileHandle(e.getCause());
}
+ return true;
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/events/fsstore/Nfs.java b/src/main/java/com/googlesource/gerrit/plugins/events/fsstore/Nfs.java
index aacb3a9..1454fbf 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/events/fsstore/Nfs.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/events/fsstore/Nfs.java
@@ -15,7 +15,6 @@
package com.googlesource.gerrit.plugins.events.fsstore;
import java.io.IOException;
-import java.nio.file.DirectoryIteratorException;
import java.nio.file.Path;
import java.nio.file.attribute.FileTime;
import java.util.Locale;
@@ -58,23 +57,23 @@
}
/**
- * Is any entry in a directory tree older than expiry. Do NOT throw IOExceptions, or
- * DirectoryIteratorExceptions.
+ * Is any entry in a directory tree older than expiry
+ *
+ * @throws IOException
*/
- public static boolean isAllEntriesOlderThan(Path dir, FileTime expiry) {
+ public static boolean isAllEntriesOlderThan(Path dir, FileTime expiry) throws IOException {
try {
return Fs.isAllEntriesOlderThan(dir, expiry);
- } catch (DirectoryIteratorException e) {
- return false; // Modified after start, so not older
+ } catch (IOException e) {
+ throwIfNotStaleFileHandle(e);
}
+ return false; // Modified after start, so not older
}
/** Get the first entry in a directory. */
public static Path getFirstDirEntry(Path dir) throws IOException {
try {
return Fs.getFirstDirEntry(dir);
- } catch (DirectoryIteratorException e) {
- throwIfNotStaleFileHandle(e);
} catch (IOException e) {
throwIfNotStaleFileHandle(e);
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/events/fsstore/UpdatableFileValue.java b/src/main/java/com/googlesource/gerrit/plugins/events/fsstore/UpdatableFileValue.java
index bc8e9a5..b3779be 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/events/fsstore/UpdatableFileValue.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/events/fsstore/UpdatableFileValue.java
@@ -280,7 +280,7 @@
return null;
}
- protected boolean shouldCompleteOngoing() {
+ protected boolean shouldCompleteOngoing() throws IOException {
// Collisions are expected, and we don't actually want to
// complete them too often since it affects fairness
// by potentially preventing slower actors from ever
@@ -290,7 +290,7 @@
// Maximum delay incurred due to a server crash.
FileTime expiry = Fs.getFileTimeAgo(10, TimeUnit.SECONDS);
- return Fs.isAllEntriesOlderThan(paths.update, expiry);
+ return Nfs.isAllEntriesOlderThan(paths.update, expiry);
}
protected abstract UniqueUpdate<T> createUniqueUpdate(String uuid, boolean ours, long maxTries)
