Clear the GroovyClassLoader cache before compiling #8062
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
++ I think this is a a bug really no? should it go to 1.3.5 and 1.4 too? LGTM |
|
Refactored this after talking with @kimchy, this is now implemented as a listener where the classloader is only cleared when scripts are removed from the cache. I tested this with the script from #7658 and I was able to send/compile 10,000 unique scripts on both Java 7 and 8 without running into PermGen issues. |
| if (logger.isDebugEnabled()) { | ||
| logger.debug("notifying script services of script removal due to: [{}]", notification.getCause()); | ||
| } | ||
| for (ScriptEngineService service : scriptEngines.values()) { |
There was a problem hiding this comment.
maybe catch exceptions here and put them into an arraylist, you can then just use ExceptionHelper to rerthrow and surpress....it has a utility for this
|
left minor comments - LGTM |
dakrone
force-pushed
the
clear-groovy-permgen
branch
2 times, most recently
from
8f11d42
to
2c6d31d
Compare
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
|
This one breaks script plugins for elasticsearch 1.3.5, 1.4.0 and above. A new method in void scriptRemoved(@Nullable CompiledScript script); |
dadoonet
added a commit
to elastic/elasticsearch-lang-javascript
that referenced
this pull request
Oct 15, 2014
…Service` This [PR](elastic/elasticsearch#8062) broke ScriptEngineService by adding a new method `scriptRemoved(CompiledScript)`. Closes #29. (cherry picked from commit 1fd05bc)
dadoonet
added a commit
to elastic/elasticsearch-lang-javascript
that referenced
this pull request
Oct 15, 2014
…Service` This [PR](elastic/elasticsearch#8062) broke ScriptEngineService by adding a new method `scriptRemoved(CompiledScript)`. Closes #29. (cherry picked from commit 1fd05bc)
dadoonet
added a commit
to elastic/elasticsearch-lang-javascript
that referenced
this pull request
Oct 15, 2014
…Service` This [PR](elastic/elasticsearch#8062) broke ScriptEngineService by adding a new method `scriptRemoved(CompiledScript)`. Closes #29.
dadoonet
added a commit
to elastic/elasticsearch-lang-javascript
that referenced
this pull request
Oct 15, 2014
…Service` This [PR](elastic/elasticsearch#8062) broke ScriptEngineService by adding a new method `scriptRemoved(CompiledScript)`. Closes #29. (cherry picked from commit 1fd05bc)
dadoonet
added a commit
to elastic/elasticsearch-lang-mvel
that referenced
this pull request
Oct 15, 2014
…Service` This [PR](elastic/elasticsearch#8062) broke ScriptEngineService by adding a new method `scriptRemoved(CompiledScript)`. Closes #6. (cherry picked from commit 3985cec)
dadoonet
added a commit
to elastic/elasticsearch-lang-mvel
that referenced
this pull request
Oct 15, 2014
…Service` This [PR](elastic/elasticsearch#8062) broke ScriptEngineService by adding a new method `scriptRemoved(CompiledScript)`. Closes #6. (cherry picked from commit 3985cec)
dadoonet
added a commit
to elastic/elasticsearch-lang-mvel
that referenced
this pull request
Oct 15, 2014
…Service` This [PR](elastic/elasticsearch#8062) broke ScriptEngineService by adding a new method `scriptRemoved(CompiledScript)`. Closes #6.
dadoonet
added a commit
to elastic/elasticsearch-lang-python
that referenced
this pull request
Oct 15, 2014
…Service` This [PR](elastic/elasticsearch#8062) broke ScriptEngineService by adding a new method `scriptRemoved(CompiledScript`. Closes #23. (cherry picked from commit 8afd168)
dadoonet
added a commit
to elastic/elasticsearch-lang-python
that referenced
this pull request
Oct 15, 2014
…Service` This [PR](elastic/elasticsearch#8062) broke ScriptEngineService by adding a new method `scriptRemoved(CompiledScript`. Closes #23. (cherry picked from commit 8afd168)
dadoonet
added a commit
to elastic/elasticsearch-lang-python
that referenced
this pull request
Oct 15, 2014
…Service` This [PR](elastic/elasticsearch#8062) broke ScriptEngineService by adding a new method `scriptRemoved(CompiledScript)`. Closes #23. (cherry picked from commit 8afd168)
dadoonet
added a commit
to elastic/elasticsearch-lang-python
that referenced
this pull request
Oct 15, 2014
…Service` This [PR](elastic/elasticsearch#8062) broke ScriptEngineService by adding a new method `scriptRemoved(CompiledScript)`. Closes #23.
clintongormley
changed the title
clintongormley
added
the
:Core/Infra/Scripting
clintongormley
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since we don't use the cache, it's okay to clear it entirely if needed,
Elasticsearch maintains its own cache for compiled scripts.
Fixes #7658
Fixes #8073