Skip to content

Commit

Permalink
Core: DistributorDirectory shouldn't search for directory when readin…
Browse files Browse the repository at this point in the history
…g existing file

This was causing too much work e.g. when pulling node stats or when
opening a new reader, because the least_used distributor would
unnecessarily check free disk space on all path.data entires every
time we try to open a file for reading or check its length.

Closes #7306

Closes #7323
  • Loading branch information
mikemccand committed Aug 19, 2014
1 parent bf2099e commit 91d2f8e
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 13 deletions.
Expand Up @@ -159,11 +159,14 @@ private Directory getDirectory(String name, boolean failIfNotAssociated, boolean
if (usePrimary(name)) {
return distributor.primary();
}
if (!nameDirMapping.containsKey(name)) {
if (iterate) { // in order to get stuff like "write.lock" that might not be written though this directory
Directory directory = nameDirMapping.get(name);
if (directory == null) {
// name is not yet bound to a directory:

if (iterate) { // in order to get stuff like "write.lock" that might not be written through this directory
for (Directory dir : distributor.all()) {
if (dir.fileExists(name)) {
final Directory directory = nameDirMapping.putIfAbsent(name, dir);
directory = nameDirMapping.putIfAbsent(name, dir);
return directory == null ? dir : directory;
}
}
Expand All @@ -172,10 +175,17 @@ private Directory getDirectory(String name, boolean failIfNotAssociated, boolean
if (failIfNotAssociated) {
throw new FileNotFoundException("No such file [" + name + "]");
}

// Pick a directory and associate this new file with it:
final Directory dir = distributor.any();
directory = nameDirMapping.putIfAbsent(name, dir);
if (directory == null) {
// putIfAbsent did in fact put dir:
directory = dir;
}
}
final Directory dir = distributor.any();
final Directory directory = nameDirMapping.putIfAbsent(name, dir);
return directory == null ? dir : directory;

return directory;
}

@Override
Expand Down
Expand Up @@ -18,19 +18,21 @@
*/
package org.elasticsearch.index.store;

import com.carrotsearch.randomizedtesting.annotations.Listeners;
import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters;
import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
import com.carrotsearch.randomizedtesting.annotations.TimeoutSuite;
import java.io.File;
import java.io.IOException;

import org.apache.lucene.store.BaseDirectoryTestCase;
import org.apache.lucene.store.Directory;
import org.apache.lucene.store.IOContext;
import org.apache.lucene.util.LuceneTestCase;
import org.apache.lucene.util.TimeUnits;
import org.elasticsearch.index.store.distributor.Distributor;
import org.elasticsearch.test.ElasticsearchThreadFilter;
import org.elasticsearch.test.junit.listeners.LoggingListener;

import java.io.File;
import java.io.IOException;
import com.carrotsearch.randomizedtesting.annotations.Listeners;
import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters;
import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
import com.carrotsearch.randomizedtesting.annotations.TimeoutSuite;

@ThreadLeakFilters(defaultFilters = true, filters = {ElasticsearchThreadFilter.class})
@ThreadLeakScope(ThreadLeakScope.Scope.NONE)
Expand All @@ -48,4 +50,40 @@ protected Directory getDirectory(File path) throws IOException {
return new DistributorDirectory(directories);
}

// #7306: don't invoke the distributor when we are opening an already existing file
public void testDoNotCallDistributorOnRead() throws Exception {
Directory dir = newDirectory();
dir.createOutput("one.txt", IOContext.DEFAULT).close();

final Directory[] dirs = new Directory[] {dir};

Distributor distrib = new Distributor() {

@Override
public Directory primary() {
return dirs[0];
}

@Override
public Directory[] all() {
return dirs;
}

@Override
public synchronized Directory any() {
throw new IllegalStateException("any should not be called");
}
};

Directory dd = new DistributorDirectory(distrib);
assertEquals(0, dd.fileLength("one.txt"));
dd.openInput("one.txt", IOContext.DEFAULT).close();
try {
dd.createOutput("three.txt", IOContext.DEFAULT).close();
fail("didn't hit expected exception");
} catch (IllegalStateException ise) {
// expected
}
dd.close();
}
}

0 comments on commit 91d2f8e

Please sign in to comment.