Replace LoadingCache usage with a simple ConcurrentHashMap

This commit replaces the usage of LoadedCache with a simple CHM and calls
to computeIfAbsent and adds LoadingCache and CacheLoader to forbidden APIs

Relates to #13224
This commit is contained in:
Simon Willnauer 2015-09-14 11:35:22 +02:00
parent a96350d785
commit 30bd46ab25
2 changed files with 22 additions and 43 deletions

View File

@ -18,16 +18,13 @@
*/ */
package org.elasticsearch.indices.analysis; package org.elasticsearch.indices.analysis;
import com.google.common.util.concurrent.UncheckedExecutionException;
import org.apache.lucene.analysis.hunspell.Dictionary; import org.apache.lucene.analysis.hunspell.Dictionary;
import org.apache.lucene.util.IOUtils;
import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.collect.CopyOnWriteHashMap;
import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.io.FileSystemUtils; import org.elasticsearch.common.io.FileSystemUtils;
import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.KeyedLock;
import org.elasticsearch.env.Environment; import org.elasticsearch.env.Environment;
import java.io.IOException; import java.io.IOException;
@ -36,6 +33,8 @@ import java.nio.file.DirectoryStream;
import java.nio.file.Files; import java.nio.file.Files;
import java.nio.file.Path; import java.nio.file.Path;
import java.util.*; import java.util.*;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Function;
/** /**
* Serves as a node level registry for hunspell dictionaries. This services expects all dictionaries to be located under * Serves as a node level registry for hunspell dictionaries. This services expects all dictionaries to be located under
@ -71,24 +70,29 @@ public class HunspellService extends AbstractComponent {
public final static String HUNSPELL_LAZY_LOAD = "indices.analysis.hunspell.dictionary.lazy"; public final static String HUNSPELL_LAZY_LOAD = "indices.analysis.hunspell.dictionary.lazy";
public final static String HUNSPELL_IGNORE_CASE = "indices.analysis.hunspell.dictionary.ignore_case"; public final static String HUNSPELL_IGNORE_CASE = "indices.analysis.hunspell.dictionary.ignore_case";
private final static String OLD_HUNSPELL_LOCATION = "indices.analysis.hunspell.dictionary.location"; private final static String OLD_HUNSPELL_LOCATION = "indices.analysis.hunspell.dictionary.location";
private final Environment env; private final ConcurrentHashMap<String, Dictionary> dictionaries = new ConcurrentHashMap<>();
private volatile CopyOnWriteHashMap<String, Dictionary> dictionaries = new CopyOnWriteHashMap<>();
private final Map<String, Dictionary> knownDictionaries; private final Map<String, Dictionary> knownDictionaries;
private KeyedLock<String> keyedLock = new KeyedLock<>();
private final boolean defaultIgnoreCase; private final boolean defaultIgnoreCase;
private final Path hunspellDir; private final Path hunspellDir;
private final Function<String, Dictionary> loadingFunction;
@Inject @Inject
public HunspellService(final Settings settings, final Environment env, final Map<String, Dictionary> knownDictionaries) throws IOException { public HunspellService(final Settings settings, final Environment env, final Map<String, Dictionary> knownDictionaries) throws IOException {
super(settings); super(settings);
this.knownDictionaries = knownDictionaries; this.knownDictionaries = Collections.unmodifiableMap(knownDictionaries);
this.hunspellDir = resolveHunspellDirectory(settings, env); this.hunspellDir = resolveHunspellDirectory(settings, env);
this.defaultIgnoreCase = settings.getAsBoolean(HUNSPELL_IGNORE_CASE, false); this.defaultIgnoreCase = settings.getAsBoolean(HUNSPELL_IGNORE_CASE, false);
this.env = env; this.loadingFunction = (locale) -> {
try {
return loadDictionary(locale, settings, env);
} catch (Throwable e) {
throw new IllegalStateException("failed to load hunspell dictionary for locale: " + locale, e);
}
};
if (!settings.getAsBoolean(HUNSPELL_LAZY_LOAD, false)) { if (!settings.getAsBoolean(HUNSPELL_LAZY_LOAD, false)) {
scanAndLoadDictionaries(); scanAndLoadDictionaries();
} }
} }
/** /**
@ -97,22 +101,9 @@ public class HunspellService extends AbstractComponent {
* @param locale The name of the locale * @param locale The name of the locale
*/ */
public Dictionary getDictionary(String locale) { public Dictionary getDictionary(String locale) {
Dictionary dictionary = dictionaries.get(locale); Dictionary dictionary = knownDictionaries.get(locale);
if (dictionary == null) { if (dictionary == null) {
dictionary = knownDictionaries.get(locale); dictionary = dictionaries.computeIfAbsent(locale, loadingFunction);
if (dictionary == null) {
keyedLock.acquire(locale);
dictionary = dictionaries.get(locale);
if (dictionary == null) {
try {
dictionary = loadDictionary(locale, settings, env);
} catch (Exception e) {
throw new IllegalStateException("failed to load hunspell dictionary for local: " + locale, e);
}
dictionaries = dictionaries.copyAndPut(locale, dictionary);
}
keyedLock.release(locale);
}
} }
return dictionary; return dictionary;
} }
@ -137,10 +128,10 @@ public class HunspellService extends AbstractComponent {
if (inner.iterator().hasNext()) { // just making sure it's indeed a dictionary dir if (inner.iterator().hasNext()) { // just making sure it's indeed a dictionary dir
try { try {
getDictionary(file.getFileName().toString()); getDictionary(file.getFileName().toString());
} catch (UncheckedExecutionException e) { } catch (Throwable e) {
// The cache loader throws unchecked exception (see #loadDictionary()), // The cache loader throws unchecked exception (see #loadDictionary()),
// here we simply report the exception and continue loading the dictionaries // here we simply report the exception and continue loading the dictionaries
logger.error("exception while loading dictionary {}", file.getFileName(), e); logger.error("exception while loading dictionary {}", e, file.getFileName());
} }
} }
} }
@ -198,22 +189,8 @@ public class HunspellService extends AbstractComponent {
logger.error("Could not load hunspell dictionary [{}]", e, locale); logger.error("Could not load hunspell dictionary [{}]", e, locale);
throw e; throw e;
} finally { } finally {
if (affixStream != null) { IOUtils.close(affixStream);
try { IOUtils.close(dicStreams);
affixStream.close();
} catch (IOException e) {
// nothing much we can do here
}
}
for (InputStream in : dicStreams) {
if (in != null) {
try {
in.close();
} catch (IOException e) {
// nothing much we can do here
}
}
}
} }
} }

View File

@ -110,6 +110,8 @@ com.google.common.collect.ImmutableSortedMap
com.google.common.base.Charsets com.google.common.base.Charsets
com.google.common.base.Function com.google.common.base.Function
com.google.common.collect.Collections2 com.google.common.collect.Collections2
com.google.common.cache.LoadingCache
com.google.common.cache.CacheLoader
@defaultMessage Do not violate java's access system @defaultMessage Do not violate java's access system
java.lang.reflect.AccessibleObject#setAccessible(boolean) java.lang.reflect.AccessibleObject#setAccessible(boolean)