Skip to content

Commit

Permalink
Scripting: Groundwork for caching script results (elastic#49895)
Browse files Browse the repository at this point in the history
In order to cache script results in the query shard cache, we need to
check if scripts are deterministic.  This change adds a default method
to the script factories, `isResultDeterministic() -> false` which is
used by the `QueryShardContext`.

Script results were never cached and that does not change here.  Future
changes will implement this method based on whether the results of the
scripts are deterministic or not and therefore cacheable.

Refs: elastic#49466
  • Loading branch information
stu-elastic authored and SivagurunathanV committed Jan 21, 2020
1 parent 261b57c commit f2b233c
Show file tree
Hide file tree
Showing 59 changed files with 236 additions and 114 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.apache.lucene.analysis.tokenattributes.TypeAttribute;
import org.apache.lucene.util.AttributeSource;
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.script.ScriptFactory;

/**
* A predicate based on the current token in a TokenStream
Expand Down Expand Up @@ -107,7 +108,7 @@ public boolean isKeyword() {
*/
public abstract boolean execute(Token token);

public interface Factory {
public interface Factory extends ScriptFactory {
AnalysisPredicateScript newInstance();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.elasticsearch.indices.analysis.AnalysisModule;
import org.elasticsearch.script.Script;
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.script.ScriptFactory;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.test.ESTokenStreamTestCase;
import org.elasticsearch.test.IndexSettingsModule;
Expand Down Expand Up @@ -63,7 +64,7 @@ public boolean execute(Token token) {
@SuppressWarnings("unchecked")
ScriptService scriptService = new ScriptService(indexSettings, Collections.emptyMap(), Collections.emptyMap()){
@Override
public <FactoryType> FactoryType compile(Script script, ScriptContext<FactoryType> context) {
public <FactoryType extends ScriptFactory> FactoryType compile(Script script, ScriptContext<FactoryType> context) {
assertEquals(context, AnalysisPredicateScript.CONTEXT);
assertEquals(new Script("my_script"), script);
return (FactoryType) factory;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.elasticsearch.indices.analysis.AnalysisModule;
import org.elasticsearch.script.Script;
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.script.ScriptFactory;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.test.ESTokenStreamTestCase;
import org.elasticsearch.test.IndexSettingsModule;
Expand Down Expand Up @@ -63,7 +64,7 @@ public boolean execute(Token token) {
@SuppressWarnings("unchecked")
ScriptService scriptService = new ScriptService(indexSettings, Collections.emptyMap(), Collections.emptyMap()){
@Override
public <FactoryType> FactoryType compile(Script script, ScriptContext<FactoryType> context) {
public <FactoryType extends ScriptFactory> FactoryType compile(Script script, ScriptContext<FactoryType> context) {
assertEquals(context, AnalysisPredicateScript.CONTEXT);
assertEquals(new Script("token.getPosition() > 1"), script);
return (FactoryType) factory;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.script.ScriptEngine;
import org.elasticsearch.script.ScriptException;
import org.elasticsearch.script.ScriptFactory;
import org.elasticsearch.script.TermsSetQueryScript;
import org.elasticsearch.search.lookup.SearchLookup;

Expand Down Expand Up @@ -108,7 +109,12 @@ public String getType() {
}

@Override
public <T> T compile(String scriptName, String scriptSource, ScriptContext<T> context, Map<String, String> params) {
public <T extends ScriptFactory> T compile(
String scriptName,
String scriptSource,
ScriptContext<T> context,
Map<String, String> params
) {
// classloader created here
final SecurityManager sm = System.getSecurityManager();
SpecialPermission.check();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.script.ScriptEngine;
import org.elasticsearch.script.ScriptException;
import org.elasticsearch.script.ScriptFactory;
import org.elasticsearch.script.TemplateScript;

import java.io.Reader;
Expand Down Expand Up @@ -64,7 +65,12 @@ public final class MustacheScriptEngine implements ScriptEngine {
* @return a compiled template object for later execution.
* */
@Override
public <T> T compile(String templateName, String templateSource, ScriptContext<T> context, Map<String, String> options) {
public <T extends ScriptFactory> T compile(
String templateName,
String templateSource,
ScriptContext<T> context,
Map<String, String> options
) {
if (context.instanceClazz.equals(TemplateScript.class) == false) {
throw new IllegalArgumentException("mustache engine does not know how to handle context [" + context.name + "]");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.script.ScriptEngine;
import org.elasticsearch.script.ScriptException;
import org.elasticsearch.script.ScriptFactory;
import org.objectweb.asm.ClassWriter;
import org.objectweb.asm.Opcodes;
import org.objectweb.asm.Type;
Expand Down Expand Up @@ -66,7 +67,7 @@ public final class PainlessScriptEngine implements ScriptEngine {
*/
private static final AccessControlContext COMPILATION_CONTEXT;

/**
/*
* Setup the allowed permissions.
*/
static {
Expand Down Expand Up @@ -122,7 +123,12 @@ public String getType() {
}

@Override
public <T> T compile(String scriptName, String scriptSource, ScriptContext<T> context, Map<String, String> params) {
public <T extends ScriptFactory> T compile(
String scriptName,
String scriptSource,
ScriptContext<T> context,
Map<String, String> params
) {
Compiler compiler = contextsToCompilers.get(context);

// Check we ourselves are not being called by unprivileged code.
Expand Down Expand Up @@ -162,12 +168,16 @@ public Set<ScriptContext<?>> getSupportedContexts() {
* @param <T> The factory class.
* @return A factory class that will return script instances.
*/
private <T> Type generateStatefulFactory(Loader loader, ScriptContext<T> context, Set<String> extractedVariables) {
private <T extends ScriptFactory> Type generateStatefulFactory(
Loader loader,
ScriptContext<T> context,
Set<String> extractedVariables
) {
int classFrames = ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS;
int classAccess = Opcodes.ACC_PUBLIC | Opcodes.ACC_SUPER | Opcodes.ACC_FINAL;
String interfaceBase = Type.getType(context.statefulFactoryClazz).getInternalName();
String className = interfaceBase + "$StatefulFactory";
String classInterfaces[] = new String[] { interfaceBase };
String[] classInterfaces = new String[] { interfaceBase };

ClassWriter writer = new ClassWriter(classFrames);
writer.visit(WriterConstants.CLASS_VERSION, classAccess, className, null, OBJECT_TYPE.getInternalName(), classInterfaces);
Expand Down Expand Up @@ -263,12 +273,17 @@ private <T> Type generateStatefulFactory(Loader loader, ScriptContext<T> context
* @param <T> The factory class.
* @return A factory class that will return script instances.
*/
private <T> T generateFactory(Loader loader, ScriptContext<T> context, Set<String> extractedVariables, Type classType) {
private <T extends ScriptFactory> T generateFactory(
Loader loader,
ScriptContext<T> context,
Set<String> extractedVariables,
Type classType
) {
int classFrames = ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS;
int classAccess = Opcodes.ACC_PUBLIC | Opcodes.ACC_SUPER| Opcodes.ACC_FINAL;
String interfaceBase = Type.getType(context.factoryClazz).getInternalName();
String className = interfaceBase + "$Factory";
String classInterfaces[] = new String[] { interfaceBase };
String[] classInterfaces = new String[] { interfaceBase };

ClassWriter writer = new ClassWriter(classFrames);
writer.visit(WriterConstants.CLASS_VERSION, classAccess, className, null, OBJECT_TYPE.getInternalName(), classInterfaces);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
import org.elasticsearch.script.ScoreScript;
import org.elasticsearch.script.Script;
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.script.ScriptFactory;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.script.ScriptType;
import org.elasticsearch.threadpool.ThreadPool;
Expand Down Expand Up @@ -415,7 +416,7 @@ public Map<String, Object> getParams() {

public abstract Object execute();

public interface Factory {
public interface Factory extends ScriptFactory {

PainlessTestScript newInstance(Map<String, Object> params);

Expand Down
Loading

0 comments on commit f2b233c

Please sign in to comment.