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/18 07:41:51 UTC
lucene-solr:branch_7x: LUCENE-8320: Fix WindowsFS#rename with
hardlinks
Repository: lucene-solr
Updated Branches:
refs/heads/branch_7x 96f6c65e4 -> fcae664e8
LUCENE-8320: Fix WindowsFS#rename with hardlinks
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/fcae664e
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/fcae664e
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/fcae664e
Branch: refs/heads/branch_7x
Commit: fcae664e88110821647c1b28b0ad2c1d99ad3e13
Parents: 96f6c65
Author: Simon Willnauer <si...@apache.org>
Authored: Fri May 18 08:38:39 2018 +0200
Committer: Simon Willnauer <si...@apache.org>
Committed: Fri May 18 09:34:25 2018 +0200
----------------------------------------------------------------------
lucene/CHANGES.txt | 3 ++
.../store/TestHardLinkCopyDirectoryWrapper.java | 33 ++++++++++++++++
.../org/apache/lucene/mockfile/WindowsFS.java | 40 ++++++++++++--------
3 files changed, 61 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/fcae664e/lucene/CHANGES.txt
----------------------------------------------------------------------
diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index 278ba9b..a2d7740 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -128,6 +128,9 @@ Bug Fixes
Future deletes could potentially be exposed to flushes/commits/refreshes if the
amount of RAM used by deletes is greater than half of the IW RAM buffer. (Simon Willnauer)
+* LUCENE-8320: Fix WindowsFS to correctly account for rename and hardlinks.
+ (Simon Willnauer, Nhat Nguyen)
+
Other
* LUCENE-8301: Update randomizedtesting to 2.6.0. (Dawid Weiss)
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/fcae664e/lucene/misc/src/test/org/apache/lucene/store/TestHardLinkCopyDirectoryWrapper.java
----------------------------------------------------------------------
diff --git a/lucene/misc/src/test/org/apache/lucene/store/TestHardLinkCopyDirectoryWrapper.java b/lucene/misc/src/test/org/apache/lucene/store/TestHardLinkCopyDirectoryWrapper.java
index 2422065..0cce132 100644
--- a/lucene/misc/src/test/org/apache/lucene/store/TestHardLinkCopyDirectoryWrapper.java
+++ b/lucene/misc/src/test/org/apache/lucene/store/TestHardLinkCopyDirectoryWrapper.java
@@ -18,12 +18,16 @@
package org.apache.lucene.store;
import java.io.IOException;
+import java.net.URI;
+import java.nio.file.FileSystem;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.Collections;
import org.apache.lucene.codecs.CodecUtil;
+import org.apache.lucene.mockfile.FilterPath;
+import org.apache.lucene.mockfile.WindowsFS;
import org.apache.lucene.util.IOUtils;
// See: https://issues.apache.org/jira/browse/SOLR-12028 Tests cannot remove files on Windows machines occasionally
@@ -87,4 +91,33 @@ public class TestHardLinkCopyDirectoryWrapper extends BaseDirectoryTestCase {
}
}
+
+ public void testRenameWithHardLink() throws Exception {
+ Path path = createTempDir();
+ FileSystem fs = new WindowsFS(path.getFileSystem()).getFileSystem(URI.create("file:///"));
+ Directory dir1 = new SimpleFSDirectory(new FilterPath(path, fs));
+ Directory dir2 = new SimpleFSDirectory(new FilterPath(path.resolve("link"), fs));
+
+ IndexOutput target = dir1.createOutput("target.txt", IOContext.DEFAULT);
+ target.writeInt(1);
+ target.close();
+
+ HardlinkCopyDirectoryWrapper wrapper = new HardlinkCopyDirectoryWrapper(dir2);
+ wrapper.copyFrom(dir1, "target.txt", "link.txt", IOContext.DEFAULT);
+
+ IndexOutput source = dir1.createOutput("source.txt", IOContext.DEFAULT);
+ source.writeInt(2);
+ source.close();
+
+ IndexInput link = dir2.openInput("link.txt", IOContext.DEFAULT);
+ // Rename while opening a hard-link file
+ dir1.rename("source.txt", "target.txt");
+ link.close();
+
+ IndexInput in = dir1.openInput("target.txt", IOContext.DEFAULT);
+ assertEquals(2, in.readInt());
+ in.close();
+
+ IOUtils.close(dir1, dir2);
+ }
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/fcae664e/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 5efc725..ecf73b6 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
@@ -32,7 +32,11 @@ import java.util.Map;
* Currently this filesystem only prevents deletion of open files.
*/
public class WindowsFS extends HandleTrackingFS {
- final Map<Object,Integer> openFiles = new HashMap<>();
+ // This map also supports fileKey -> Path -> counts
+ // which is important to effectively support renames etc.
+ // in the rename case we have to transfer ownership but need to make sure we only transfer ownership for
+ // the path we rename ie. hardlinks will still resolve to the same key
+ final Map<Object,Map<Path, Integer>> openFiles = new HashMap<>();
// TODO: try to make this as realistic as possible... it depends e.g. how you
// open files, if you map them, etc, if you can delete them (Uwe knows the rules)
@@ -62,13 +66,8 @@ public class WindowsFS extends HandleTrackingFS {
final Object key = getKey(path);
// we have to read the key under the lock otherwise me might leak the openFile handle
// if we concurrently delete or move this file.
- Integer v = openFiles.get(key);
- if (v != null) {
- v = Integer.valueOf(v.intValue()+1);
- openFiles.put(key, v);
- } else {
- openFiles.put(key, Integer.valueOf(1));
- }
+ Map<Path, Integer> pathMap = openFiles.computeIfAbsent(key, k -> new HashMap<>());
+ pathMap.put(path, pathMap.computeIfAbsent(path, p -> 0).intValue() +1);
}
}
@@ -76,16 +75,21 @@ public class WindowsFS extends HandleTrackingFS {
protected void onClose(Path path, Object stream) throws IOException {
Object key = getKey(path); // here we can read this outside of the lock
synchronized (openFiles) {
- Integer v = openFiles.get(key);
- assert v != null;
+ Map<Path, Integer> pathMap = openFiles.get(key);
+ assert pathMap != null;
+ assert pathMap.containsKey(path);
+ Integer v = pathMap.get(path);
if (v != null) {
if (v.intValue() == 1) {
- openFiles.remove(key);
+ pathMap.remove(path);
} else {
v = Integer.valueOf(v.intValue()-1);
- openFiles.put(key, v);
+ pathMap.put(path, v);
}
}
+ if (pathMap.isEmpty()) {
+ openFiles.remove(key);
+ }
}
}
@@ -133,9 +137,15 @@ public class WindowsFS extends HandleTrackingFS {
// 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);
+ Map<Path, Integer> map = openFiles.get(key);
+ Integer v = map.remove(target);
+ if (v != null) {
+ Map<Path, Integer> pathIntegerMap = openFiles.computeIfAbsent(newKey, k -> new HashMap<>());
+ Integer existingValue = pathIntegerMap.getOrDefault(target, 0);
+ pathIntegerMap.put(target, existingValue + v);
+ }
+ if (map.isEmpty()) {
+ openFiles.remove(key);
}
}
}