Skip to content

Commit

Permalink
Clear the GroovyClassLoader cache before compiling
Browse files Browse the repository at this point in the history
Since we don't use the cache, it's okay to clear it entirely if needed,
Elasticsearch maintains its own cache for compiled scripts.

Adds loader.clearCache() into a listener, the listener is called when a
script is removed from the Guava cache.

This also lowers the amount of cached scripts to 100, since 500 is
around the limit some users have run into before hitting an out of
memory error in permgem.

Fixes elastic#7658
  • Loading branch information
dakrone committed Oct 14, 2014
1 parent 82b16ae commit 2c6d31d
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 3 deletions.
Expand Up @@ -93,4 +93,9 @@ public Object unwrap(Object value) {
@Override
public void close() {
}

@Override
public void scriptRemoved(CompiledScript script) {
// Nothing to do here
}
}
Expand Up @@ -46,4 +46,11 @@ public interface ScriptEngineService {
Object unwrap(Object value);

void close();

/**
* Handler method called when a script is removed from the Guava cache.
*
* The passed script may be null if it has already been garbage collected.
* */
void scriptRemoved(@Nullable CompiledScript script);
}
33 changes: 32 additions & 1 deletion src/main/java/org/elasticsearch/script/ScriptService.java
Expand Up @@ -22,9 +22,12 @@
import com.google.common.base.Charsets;
import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.RemovalListener;
import com.google.common.cache.RemovalNotification;
import com.google.common.collect.ImmutableMap;
import org.elasticsearch.ElasticsearchIllegalArgumentException;
import org.elasticsearch.ElasticsearchIllegalStateException;
import org.elasticsearch.ExceptionsHelper;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.delete.DeleteRequest;
import org.elasticsearch.action.delete.DeleteResponse;
Expand Down Expand Up @@ -64,12 +67,15 @@
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStreamReader;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.TimeUnit;

import static com.google.common.collect.Lists.newArrayList;

/**
*
*/
Expand Down Expand Up @@ -206,7 +212,7 @@ public ScriptService(Settings settings, Environment env, Set<ScriptEngineService
ResourceWatcherService resourceWatcherService) {
super(settings);

int cacheMaxSize = settings.getAsInt(SCRIPT_CACHE_SIZE_SETTING, 500);
int cacheMaxSize = settings.getAsInt(SCRIPT_CACHE_SIZE_SETTING, 100);
TimeValue cacheExpire = settings.getAsTime(SCRIPT_CACHE_EXPIRE_SETTING, null);
logger.debug("using script cache with max_size [{}], expire [{}]", cacheMaxSize, cacheExpire);

Expand All @@ -220,6 +226,7 @@ public ScriptService(Settings settings, Environment env, Set<ScriptEngineService
if (cacheExpire != null) {
cacheBuilder.expireAfterAccess(cacheExpire.nanos(), TimeUnit.NANOSECONDS);
}
cacheBuilder.removalListener(new ScriptCacheRemovalListener());
this.cache = cacheBuilder.build();

ImmutableMap.Builder<String, ScriptEngineService> builder = ImmutableMap.builder();
Expand Down Expand Up @@ -483,6 +490,30 @@ private boolean dynamicScriptEnabled(String lang) {
}
}

/**
* A small listener for the script cache that calls each
* {@code ScriptEngineService}'s {@code scriptRemoved} method when the
* script has been removed from the cache
*/
private class ScriptCacheRemovalListener implements RemovalListener<CacheKey, CompiledScript> {

@Override
public void onRemoval(RemovalNotification<CacheKey, CompiledScript> notification) {
if (logger.isDebugEnabled()) {
logger.debug("notifying script services of script removal due to: [{}]", notification.getCause());
}
List<Exception> errors = newArrayList();
for (ScriptEngineService service : scriptEngines.values()) {
try {
service.scriptRemoved(notification.getValue());
} catch (Exception e) {
errors.add(e);
}
}
ExceptionsHelper.maybeThrowRuntimeAndSuppress(errors);
}
}

private class ScriptChangesListener extends FileChangesListener {

private Tuple<String, String> scriptNameExt(File file) {
Expand Down
Expand Up @@ -33,6 +33,7 @@
import org.elasticsearch.index.mapper.FieldMapper;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.core.NumberFieldMapper;
import org.elasticsearch.script.CompiledScript;
import org.elasticsearch.script.ExecutableScript;
import org.elasticsearch.script.ScriptEngineService;
import org.elasticsearch.script.SearchScript;
Expand Down Expand Up @@ -154,4 +155,9 @@ public Object unwrap(Object value) {

@Override
public void close() {}

@Override
public void scriptRemoved(CompiledScript script) {
// Nothing to do
}
}
Expand Up @@ -43,7 +43,6 @@
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.script.*;
import org.elasticsearch.search.lookup.SearchLookup;
import org.elasticsearch.search.suggest.term.TermSuggestion;

import java.io.IOException;
import java.math.BigDecimal;
Expand Down Expand Up @@ -89,6 +88,16 @@ public void close() {
}
}

@Override
public void scriptRemoved(@Nullable CompiledScript script) {
// script could be null, meaning the script has already been garbage collected
if (script == null || "groovy".equals(script.lang())) {
// Clear the cache, this removes old script versions from the
// cache to prevent running out of PermGen space
loader.clearCache();
}
}

@Override
public String[] types() {
return new String[]{"groovy"};
Expand Down Expand Up @@ -313,4 +322,4 @@ public Expression transform(Expression expr) {
}
}

}
}
Expand Up @@ -26,6 +26,7 @@
import org.elasticsearch.common.io.UTF8StreamWriter;
import org.elasticsearch.common.io.stream.BytesStreamOutput;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.script.CompiledScript;
import org.elasticsearch.script.ExecutableScript;
import org.elasticsearch.script.ScriptEngineService;
import org.elasticsearch.script.SearchScript;
Expand Down Expand Up @@ -148,6 +149,11 @@ public void close() {
// Nothing to do here
}

@Override
public void scriptRemoved(CompiledScript script) {
// Nothing to do here
}

/**
* Used at query execution time by script service in order to execute a query template.
* */
Expand Down
Expand Up @@ -132,6 +132,11 @@ public Object unwrap(Object value) {
public void close() {

}

@Override
public void scriptRemoved(CompiledScript script) {
// Nothing to do here
}
}

}

0 comments on commit 2c6d31d

Please sign in to comment.