You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by si...@apache.org on 2018/05/17 09:02:44 UTC
lucene-solr:master: LUCENE-8318: Ensure pending delete is not brought
back on a try delete attempt
Repository: lucene-solr
Updated Branches:
refs/heads/master 0159e4b97 -> 3fe612bed
LUCENE-8318: Ensure pending delete is not brought back on a try delete attempt
When renaming a file, `FSDirectory#rename` tries to delete the dest file
if it's in the pending deletes list. If that delete fails, it adds the
dest to the pending deletes list again. This causes the dest file to be
deleted later by `deletePendingFiles`.
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/3fe612be
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/3fe612be
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/3fe612be
Branch: refs/heads/master
Commit: 3fe612bed2080af0b3dd47ece7067ae56794fc82
Parents: 0159e4b
Author: Simon Willnauer <si...@apache.org>
Authored: Thu May 17 09:15:31 2018 +0200
Committer: Simon Willnauer <si...@apache.org>
Committed: Thu May 17 11:02:35 2018 +0200
----------------------------------------------------------------------
.../org/apache/lucene/store/FSDirectory.java | 2 +
.../lucene/store/TestSimpleFSDirectory.java | 48 ++++++++++++++++++++
.../org/apache/lucene/mockfile/WindowsFS.java | 30 +++++++++---
3 files changed, 73 insertions(+), 7 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/3fe612be/lucene/core/src/java/org/apache/lucene/store/FSDirectory.java
----------------------------------------------------------------------
diff --git a/lucene/core/src/java/org/apache/lucene/store/FSDirectory.java b/lucene/core/src/java/org/apache/lucene/store/FSDirectory.java
index 111352e..fb03f85 100644
--- a/lucene/core/src/java/org/apache/lucene/store/FSDirectory.java
+++ b/lucene/core/src/java/org/apache/lucene/store/FSDirectory.java
@@ -250,6 +250,7 @@ public abstract class FSDirectory extends BaseDirectory {
// If this file was pending delete, we are now bringing it back to life:
if (pendingDeletes.remove(name)) {
privateDeleteFile(name, true); // try again to delete it - this is best effort
+ pendingDeletes.remove(name); // watch out - if the delete fails it put
}
return new FSIndexOutput(name);
}
@@ -297,6 +298,7 @@ public abstract class FSDirectory extends BaseDirectory {
maybeDeletePendingFiles();
if (pendingDeletes.remove(dest)) {
privateDeleteFile(dest, true); // try again to delete it - this is best effort
+ pendingDeletes.remove(dest); // watch out if the delete fails it's back in here.
}
Files.move(directory.resolve(source), directory.resolve(dest), StandardCopyOption.ATOMIC_MOVE);
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/3fe612be/lucene/core/src/test/org/apache/lucene/store/TestSimpleFSDirectory.java
----------------------------------------------------------------------
diff --git a/lucene/core/src/test/org/apache/lucene/store/TestSimpleFSDirectory.java b/lucene/core/src/test/org/apache/lucene/store/TestSimpleFSDirectory.java
index eb82571..e550bcc 100644
--- a/lucene/core/src/test/org/apache/lucene/store/TestSimpleFSDirectory.java
+++ b/lucene/core/src/test/org/apache/lucene/store/TestSimpleFSDirectory.java
@@ -18,8 +18,14 @@ package org.apache.lucene.store;
import java.io.IOException;
+import java.net.URI;
+import java.nio.file.FileSystem;
import java.nio.file.Path;
+import org.apache.lucene.mockfile.FilterPath;
+import org.apache.lucene.mockfile.WindowsFS;
+import org.apache.lucene.util.IOUtils;
+
/**
* Tests SimpleFSDirectory
*/
@@ -29,4 +35,46 @@ public class TestSimpleFSDirectory extends BaseDirectoryTestCase {
protected Directory getDirectory(Path path) throws IOException {
return new SimpleFSDirectory(path);
}
+
+ public void testRenameWithPendingDeletes() throws IOException {
+ Path path = createTempDir();
+ // Use WindowsFS to prevent open files from being deleted:
+ FileSystem fs = new WindowsFS(path.getFileSystem()).getFileSystem(URI.create("file:///"));
+ Path root = new FilterPath(path, fs);
+ Directory directory = getDirectory(root);
+ IndexOutput output = directory.createOutput("target.txt", IOContext.DEFAULT);
+ output.writeInt(1);
+ output.close();
+ IndexOutput output1 = directory.createOutput("source.txt", IOContext.DEFAULT);
+ output1.writeInt(2);
+ output1.close();
+
+ IndexInput input = directory.openInput("target.txt", IOContext.DEFAULT);
+ directory.deleteFile("target.txt");
+ directory.rename("source.txt", "target.txt");
+ IndexInput input1 = directory.openInput("target.txt", IOContext.DEFAULT);
+ assertTrue(directory.getPendingDeletions().isEmpty());
+ assertEquals(1, input.readInt());
+ assertEquals(2, input1.readInt());
+ IOUtils.close(input1, input, directory);
+ }
+
+ public void testCreateOutputWithPendingDeletes() throws IOException {
+ Path path = createTempDir();
+ // Use WindowsFS to prevent open files from being deleted:
+ FileSystem fs = new WindowsFS(path.getFileSystem()).getFileSystem(URI.create("file:///"));
+ Path root = new FilterPath(path, fs);
+ Directory directory = getDirectory(root);
+ IndexOutput output = directory.createOutput("file.txt", IOContext.DEFAULT);
+ output.writeInt(1);
+ output.close();
+ IndexInput input = directory.openInput("file.txt", IOContext.DEFAULT);
+ directory.deleteFile("file.txt");
+ expectThrows(IOException.class, () -> {
+ directory.createOutput("file.txt", IOContext.DEFAULT);
+ });
+ assertTrue(directory.getPendingDeletions().isEmpty());
+ assertEquals(1, input.readInt());
+ IOUtils.close(input, directory);
+ }
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/3fe612be/lucene/test-framework/src/java/org/apache/lucene/mockfile/WindowsFS.java
----------------------------------------------------------------------
diff --git a/lucene/test-framework/src/java/org/apache/lucene/mockfile/WindowsFS.java b/lucene/test-framework/src/java/org/apache/lucene/mockfile/WindowsFS.java
index b7b5457..5efc725 100644
--- a/lucene/test-framework/src/java/org/apache/lucene/mockfile/WindowsFS.java
+++ b/lucene/test-framework/src/java/org/apache/lucene/mockfile/WindowsFS.java
@@ -88,19 +88,22 @@ public class WindowsFS extends HandleTrackingFS {
}
}
}
+
+ private Object getKeyOrNull(Path path) {
+ try {
+ return getKey(path);
+ } catch (Exception ignore) {
+ // we don't care if the file doesn't exist
+ }
+ return null;
+ }
/**
* Checks that it's ok to delete {@code Path}. If the file
* is still open, it throws IOException("access denied").
*/
private void checkDeleteAccess(Path path) throws IOException {
- Object key = null;
- try {
- key = getKey(path);
- } catch (Throwable ignore) {
- // we don't care if the file doesn't exist
- }
-
+ Object key = getKeyOrNull(path);
if (key != null) {
synchronized(openFiles) {
if (openFiles.containsKey(key)) {
@@ -122,7 +125,20 @@ public class WindowsFS extends HandleTrackingFS {
public void move(Path source, Path target, CopyOption... options) throws IOException {
synchronized (openFiles) {
checkDeleteAccess(source);
+ Object key = getKeyOrNull(target);
super.move(source, target, options);
+ if (key != null) {
+ Object newKey = getKey(target);
+ if (newKey.equals(key) == false) {
+ // we need to transfer ownership here if we have open files on this file since the getKey() method will
+ // return a different i-node next time we call it with the target path and our onClose method will
+ // trip an assert
+ Integer remove = openFiles.remove(key);
+ if (remove != null) {
+ openFiles.put(newKey, remove);
+ }
+ }
+ }
}
}