LUCENE-8320: Fix WindowsFS#rename with hardlinks

This commit is contained in:
Simon Willnauer 2018-05-18 08:38:39 +02:00
parent 4a9a8397e4
commit 42a79970d5
3 changed files with 61 additions and 15 deletions

View File

@ -226,6 +226,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)

View File

@ -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);
}
}

View File

@ -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);
}
}
}