-
Notifications
You must be signed in to change notification settings - Fork 24.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Scripting: Cache script results if deterministic #50106
Scripting: Cache script results if deterministic #50106
Conversation
…e/painless-caching-part-2
…e/painless-caching-part-2
Pinging @elastic/es-core-infra (:Core/Infra/Scripting) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM once we address the couple of comments I left that require further discussion.
.../spi/src/main/java/org/elasticsearch/painless/spi/annotation/NonDeterministicAnnotation.java
Show resolved
Hide resolved
modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.util.txt
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Left a few suggestions.
.../spi/src/main/java/org/elasticsearch/painless/spi/annotation/NonDeterministicAnnotation.java
Show resolved
Hide resolved
...rc/main/java/org/elasticsearch/painless/spi/annotation/NonDeterministicAnnotationParser.java
Show resolved
Hide resolved
modules/lang-painless/src/main/java/org/elasticsearch/painless/ScriptRoot.java
Outdated
Show resolved
Hide resolved
Currently debugging NPE when using scripts for weighted avg, fieldName coming in as null. |
Ok, now that scripts are cachable, here's what's causing the NPE
calls into
which calls which eventually calls
which nulls out because there is no Here's the series of calls to reproduce:
|
This just happened to be caught by a test. Are there other places where |
@elasticmachine run elasticsearch-ci/1 |
@stu-elastic Thanks again for all the work getting this done. The tests look especially good. |
Cache results from queries that use scripts if they use only deterministic API calls. Nondeterministic API calls are marked in the whitelist with the `@nondeterministic` annotation. Examples are `Math.random()` and `new Date()`. Refs: elastic#49466
Cache results from queries that use scripts if they use only deterministic API calls. Nondeterministic API calls are marked in the whitelist with the `@nondeterministic` annotation. Examples are `Math.random()` and `new Date()`. Refs: #49466
Cache results from queries that use scripts if they use only deterministic API calls. Nondeterministic API calls are marked in the whitelist with the `@nondeterministic` annotation. Examples are `Math.random()` and `new Date()`. Refs: elastic#49466
Cache results from queries that use scripts if they use only
deterministic API calls. Nondeterministic API calls are marked in the
whitelist with the
@nondeterministic
annotation. Examples areMath.random()
andnew Date()
.Refs: #49466