honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Remove usage of FileSwitchDirectory (#42937) 

We are still using `FileSwitchDirectory` in the case a user configures file based pre-load of mmaps. This is trappy for multiple reasons if the both directories used by `FileSwitchDirectory` point to the same filesystem directory. One issue is LUCENE-8835 that cause issues like #37111 - unless LUCENE-8835 isn't fixed we should not use it in elasticsearch. Instead we use a similar trick as we use for HybridFS and subclass mmap directory directly.
This commit is contained in:
Simon Willnauer 2019-06-12 17:42:08 +02:00
parent 7f2f0b7620
commit 9d2adfb41e
3 changed files with 147 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
plugins/store-smb/src/main/java/org/elasticsearch/index/store/smbmmapfs/SmbMmapFsDirectoryFactory.java
View File

@ -22,17 +22,20 @@ package org.elasticsearch.index.store.smbmmapfs;
import org.apache.lucene.store.Directory;
import org.apache.lucene.store.LockFactory;
import org.apache.lucene.store.MMapDirectory;
import org.elasticsearch.index.IndexModule;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.store.FsDirectoryFactory;
import org.elasticsearch.index.store.SmbDirectoryWrapper;
import java.io.IOException;
import java.nio.file.Path;
import java.util.HashSet;
public final class SmbMmapFsDirectoryFactory extends FsDirectoryFactory {
@Override
protected Directory newFSDirectory(Path location, LockFactory lockFactory, IndexSettings indexSettings) throws IOException {
return new SmbDirectoryWrapper(new MMapDirectory(location, lockFactory));
return new SmbDirectoryWrapper(setPreload(new MMapDirectory(location, lockFactory), lockFactory, new HashSet<>(
indexSettings.getValue(IndexModule.INDEX_STORE_PRE_LOAD_SETTING))));
}
}

125
server/src/main/java/org/elasticsearch/index/store/FsDirectoryFactory.java
View File

@ -64,11 +64,7 @@ public class FsDirectoryFactory implements IndexStorePlugin.DirectoryFactory {
final Path location = path.resolveIndex();
final LockFactory lockFactory = indexSettings.getValue(INDEX_LOCK_FACTOR_SETTING);
Files.createDirectories(location);
Directory wrapped = newFSDirectory(location, lockFactory, indexSettings);
Set<String> preLoadExtensions = new HashSet<>(
indexSettings.getValue(IndexModule.INDEX_STORE_PRE_LOAD_SETTING));
wrapped = setPreload(wrapped, location, lockFactory, preLoadExtensions);
return wrapped;
return newFSDirectory(location, lockFactory, indexSettings);
}
protected Directory newFSDirectory(Path location, LockFactory lockFactory, IndexSettings indexSettings) throws IOException {
@ -80,17 +76,20 @@ public class FsDirectoryFactory implements IndexStorePlugin.DirectoryFactory {
} else {
type = IndexModule.Type.fromSettingsKey(storeType);
}
Set<String> preLoadExtensions = new HashSet<>(
indexSettings.getValue(IndexModule.INDEX_STORE_PRE_LOAD_SETTING));
switch (type) {
case HYBRIDFS:
// Use Lucene defaults
final FSDirectory primaryDirectory = FSDirectory.open(location, lockFactory);
if (primaryDirectory instanceof MMapDirectory) {
return new HybridDirectory(location, lockFactory, primaryDirectory);
MMapDirectory mMapDirectory = (MMapDirectory) primaryDirectory;
return new HybridDirectory(lockFactory, setPreload(mMapDirectory, lockFactory, preLoadExtensions));
} else {
return primaryDirectory;
}
case MMAPFS:
return new MMapDirectory(location, lockFactory);
return setPreload(new MMapDirectory(location, lockFactory), lockFactory, preLoadExtensions);
case SIMPLEFS:
return new SimpleFSDirectory(location, lockFactory);
case NIOFS:
@ -100,26 +99,17 @@ public class FsDirectoryFactory implements IndexStorePlugin.DirectoryFactory {
}
}
private static Directory setPreload(Directory directory, Path location, LockFactory lockFactory,
public static MMapDirectory setPreload(MMapDirectory mMapDirectory, LockFactory lockFactory,
Set<String> preLoadExtensions) throws IOException {
if (preLoadExtensions.isEmpty() == false
&& directory instanceof MMapDirectory
&& ((MMapDirectory) directory).getPreload() == false) {
assert mMapDirectory.getPreload() == false;
if (preLoadExtensions.isEmpty() == false) {
if (preLoadExtensions.contains("*")) {
((MMapDirectory) directory).setPreload(true);
return directory;
mMapDirectory.setPreload(true);
} else {
return new PreLoadMMapDirectory(mMapDirectory, lockFactory, preLoadExtensions);
}
MMapDirectory primary = new MMapDirectory(location, lockFactory);
primary.setPreload(true);
return new FileSwitchDirectory(preLoadExtensions, primary, directory, true) {
@Override
public String[] listAll() throws IOException {
// avoid listing twice
return primary.listAll();
}
};
}
return directory;
return mMapDirectory;
}
/**
@ -131,15 +121,35 @@ public class FsDirectoryFactory implements IndexStorePlugin.DirectoryFactory {
}
static final class HybridDirectory extends NIOFSDirectory {
private final FSDirectory randomAccessDirectory;
private final MMapDirectory delegate;
HybridDirectory(Path location, LockFactory lockFactory, FSDirectory randomAccessDirectory) throws IOException {
super(location, lockFactory);
this.randomAccessDirectory = randomAccessDirectory;
HybridDirectory(LockFactory lockFactory, MMapDirectory delegate) throws IOException {
super(delegate.getDirectory(), lockFactory);
this.delegate = delegate;
}
@Override
public IndexInput openInput(String name, IOContext context) throws IOException {
if (useDelegate(name)) {
// we need to do these checks on the outer directory since the inner doesn't know about pending deletes
ensureOpen();
ensureCanRead(name);
// we only use the mmap to open inputs. Everything else is managed by the NIOFSDirectory otherwise
// we might run into trouble with files that are pendingDelete in one directory but still
// listed in listAll() from the other. We on the other hand don't want to list files from both dirs
// and intersect for perf reasons.
return delegate.openInput(name, context);
} else {
return super.openInput(name, context);
}
}
@Override
public void close() throws IOException {
IOUtils.close(super::close, delegate);
}
boolean useDelegate(String name) {
String extension = FileSwitchDirectory.getExtension(name);
switch(extension) {
// We are mmapping norms, docvalues as well as term dictionaries, all other files are served through NIOFS
@ -148,26 +158,59 @@ public class FsDirectoryFactory implements IndexStorePlugin.DirectoryFactory {
case "dvd":
case "tim":
case "cfs":
// we need to do these checks on the outer directory since the inner doesn't know about pending deletes
ensureOpen();
ensureCanRead(name);
// we only use the mmap to open inputs. Everything else is managed by the NIOFSDirectory otherwise
// we might run into trouble with files that are pendingDelete in one directory but still
// listed in listAll() from the other. We on the other hand don't want to list files from both dirs
// and intersect for perf reasons.
return randomAccessDirectory.openInput(name, context);
return true;
default:
return super.openInput(name, context);
return false;
}
}
@Override
public void close() throws IOException {
IOUtils.close(super::close, randomAccessDirectory);
MMapDirectory getDelegate() {
return delegate;
}
}
// TODO it would be nice to share code between PreLoadMMapDirectory and HybridDirectory but due to the nesting aspect of
// directories here makes it tricky. It would be nice to allow MMAPDirectory to pre-load on a per IndexInput basis.
static final class PreLoadMMapDirectory extends MMapDirectory {
private final MMapDirectory delegate;
private final Set<String> preloadExtensions;
PreLoadMMapDirectory(MMapDirectory delegate, LockFactory lockFactory, Set<String> preload) throws IOException {
super(delegate.getDirectory(), lockFactory);
super.setPreload(false);
this.delegate = delegate;
this.delegate.setPreload(true);
this.preloadExtensions = preload;
assert getPreload() == false;
}
Directory getRandomAccessDirectory() {
return randomAccessDirectory;
@Override
public void setPreload(boolean preload) {
throw new IllegalArgumentException("can't set preload on a preload-wrapper");
}
@Override
public IndexInput openInput(String name, IOContext context) throws IOException {
if (useDelegate(name)) {
// we need to do these checks on the outer directory since the inner doesn't know about pending deletes
ensureOpen();
ensureCanRead(name);
return delegate.openInput(name, context);
}
return super.openInput(name, context);
}
@Override
public synchronized void close() throws IOException {
IOUtils.close(super::close, delegate);
}
boolean useDelegate(String name) {
final String extension = FileSwitchDirectory.getExtension(name);
return preloadExtensions.contains(extension);
}
MMapDirectory getDelegate() {
return delegate;
}
}
}

91
server/src/test/java/org/elasticsearch/index/store/FsDirectoryFactoryTests.java
View File

@ -18,8 +18,9 @@
*/
package org.elasticsearch.index.store;
import org.apache.lucene.store.AlreadyClosedException;
import org.apache.lucene.store.Directory;
import org.apache.lucene.store.FileSwitchDirectory;
import org.apache.lucene.store.IOContext;
import org.apache.lucene.store.MMapDirectory;
import org.apache.lucene.store.NIOFSDirectory;
import org.apache.lucene.store.NoLockFactory;
@ -36,6 +37,7 @@ import org.elasticsearch.index.shard.ShardId;
import org.elasticsearch.index.shard.ShardPath;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.IndexSettingsModule;
import org.hamcrest.Matchers;
import java.io.IOException;
import java.nio.file.Files;
@ -49,34 +51,65 @@ public class FsDirectoryFactoryTests extends ESTestCase {
doTestPreload();
doTestPreload("nvd", "dvd", "tim");
doTestPreload("*");
Settings build = Settings.builder()
.put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.HYBRIDFS.name().toLowerCase(Locale.ROOT))
.putList(IndexModule.INDEX_STORE_PRE_LOAD_SETTING.getKey(), "dvd", "bar")
.build();
try (Directory directory = newDirectory(build)) {
assertTrue(FsDirectoryFactory.isHybridFs(directory));
FsDirectoryFactory.HybridDirectory hybridDirectory = (FsDirectoryFactory.HybridDirectory) directory;
assertTrue(hybridDirectory.useDelegate("foo.dvd"));
assertTrue(hybridDirectory.useDelegate("foo.nvd"));
assertTrue(hybridDirectory.useDelegate("foo.tim"));
assertTrue(hybridDirectory.useDelegate("foo.cfs"));
assertFalse(hybridDirectory.useDelegate("foo.bar"));
MMapDirectory delegate = hybridDirectory.getDelegate();
assertThat(delegate, Matchers.instanceOf(FsDirectoryFactory.PreLoadMMapDirectory.class));
FsDirectoryFactory.PreLoadMMapDirectory preLoadMMapDirectory = (FsDirectoryFactory.PreLoadMMapDirectory) delegate;
assertTrue(preLoadMMapDirectory.useDelegate("foo.dvd"));
assertTrue(preLoadMMapDirectory.useDelegate("foo.bar"));
}
}
private Directory newDirectory(Settings settings) throws IOException {
IndexSettings idxSettings = IndexSettingsModule.newIndexSettings("foo", settings);
Path tempDir = createTempDir().resolve(idxSettings.getUUID()).resolve("0");
Files.createDirectories(tempDir);
ShardPath path = new ShardPath(false, tempDir, tempDir, new ShardId(idxSettings.getIndex(), 0));
return new FsDirectoryFactory().newDirectory(idxSettings, path);
}
private void doTestPreload(String...preload) throws IOException {
Settings build = Settings.builder()
.put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), "mmapfs")
.putList(IndexModule.INDEX_STORE_PRE_LOAD_SETTING.getKey(), preload)
.build();
IndexSettings settings = IndexSettingsModule.newIndexSettings("foo", build);
Path tempDir = createTempDir().resolve(settings.getUUID()).resolve("0");
Files.createDirectories(tempDir);
ShardPath path = new ShardPath(false, tempDir, tempDir, new ShardId(settings.getIndex(), 0));
FsDirectoryFactory fsDirectoryFactory = new FsDirectoryFactory();
Directory directory = fsDirectoryFactory.newDirectory(settings, path);
assertFalse(directory instanceof SleepingLockWrapper);
if (preload.length == 0) {
assertTrue(directory.toString(), directory instanceof MMapDirectory);
assertFalse(((MMapDirectory) directory).getPreload());
} else if (Arrays.asList(preload).contains("*")) {
assertTrue(directory.toString(), directory instanceof MMapDirectory);
assertTrue(((MMapDirectory) directory).getPreload());
} else {
assertTrue(directory.toString(), directory instanceof FileSwitchDirectory);
FileSwitchDirectory fsd = (FileSwitchDirectory) directory;
assertTrue(fsd.getPrimaryDir() instanceof MMapDirectory);
assertTrue(((MMapDirectory) fsd.getPrimaryDir()).getPreload());
assertTrue(fsd.getSecondaryDir() instanceof MMapDirectory);
assertFalse(((MMapDirectory) fsd.getSecondaryDir()).getPreload());
.put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), "mmapfs")
.putList(IndexModule.INDEX_STORE_PRE_LOAD_SETTING.getKey(), preload)
.build();
Directory directory = newDirectory(build);
try (Directory dir = directory){
assertSame(dir, directory); // prevent warnings
assertFalse(directory instanceof SleepingLockWrapper);
if (preload.length == 0) {
assertTrue(directory.toString(), directory instanceof MMapDirectory);
assertFalse(((MMapDirectory) directory).getPreload());
} else if (Arrays.asList(preload).contains("*")) {
assertTrue(directory.toString(), directory instanceof MMapDirectory);
assertTrue(((MMapDirectory) directory).getPreload());
} else {
assertTrue(directory.toString(), directory instanceof FsDirectoryFactory.PreLoadMMapDirectory);
FsDirectoryFactory.PreLoadMMapDirectory preLoadMMapDirectory = (FsDirectoryFactory.PreLoadMMapDirectory) directory;
for (String ext : preload) {
assertTrue("ext: " + ext, preLoadMMapDirectory.useDelegate("foo." + ext));
assertTrue("ext: " + ext, preLoadMMapDirectory.getDelegate().getPreload());
}
assertFalse(preLoadMMapDirectory.useDelegate("XXX"));
assertFalse(preLoadMMapDirectory.getPreload());
preLoadMMapDirectory.close();
expectThrows(AlreadyClosedException.class, () -> preLoadMMapDirectory.getDelegate().openInput("foo.bar",
IOContext.DEFAULT));
}
}
expectThrows(AlreadyClosedException.class, () -> directory.openInput(randomBoolean() && preload.length != 0 ?
"foo." + preload[0] : "foo.bar", IOContext.DEFAULT));
}
public void testStoreDirectory() throws IOException {
@ -102,7 +135,7 @@ public class FsDirectoryFactoryTests extends ESTestCase {
try (Directory directory = service.newFSDirectory(tempDir, NoLockFactory.INSTANCE, indexSettings)) {
switch (type) {
case HYBRIDFS:
assertHybridDirectory(directory);
assertTrue(FsDirectoryFactory.isHybridFs(directory));
break;
case NIOFS:
assertTrue(type + " " + directory.toString(), directory instanceof NIOFSDirectory);
@ -115,7 +148,7 @@ public class FsDirectoryFactoryTests extends ESTestCase {
break;
case FS:
if (Constants.JRE_IS_64BIT && MMapDirectory.UNMAP_SUPPORTED) {
assertHybridDirectory(directory);
assertTrue(FsDirectoryFactory.isHybridFs(directory));
} else if (Constants.WINDOWS) {
assertTrue(directory.toString(), directory instanceof SimpleFSDirectory);
} else {
@ -127,10 +160,4 @@ public class FsDirectoryFactoryTests extends ESTestCase {
}
}
}
private void assertHybridDirectory(Directory directory) {
assertTrue(directory.toString(), directory instanceof FsDirectoryFactory.HybridDirectory);
Directory randomAccessDirectory = ((FsDirectoryFactory.HybridDirectory) directory).getRandomAccessDirectory();
assertTrue("randomAccessDirectory: " + randomAccessDirectory.toString(), randomAccessDirectory instanceof MMapDirectory);
}
}