Skip to content
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

Painless: More Safety Features #30139

Closed
jdconrad opened this issue Apr 25, 2018 · 7 comments
Closed

Painless: More Safety Features #30139

jdconrad opened this issue Apr 25, 2018 · 7 comments
Labels
:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >feature Meta

Comments

@jdconrad
Copy link
Contributor

Painless currently utilizes an improvised statement counter in an attempt to prevent infinite loops. In some use cases this isn't really enough as each iteration of the loop can take an excessive amount of time. We should explore the possibility of adding additional safety features.

Some ideas that have been proposed already are the following:

  • add a sampling of nanoseconds where every n iterations of a loop, time is taken into account to possibly terminate a script early if it's taking too long
  • track memory usage of certain types of objects including Strings, and have a cap for allowed memory usage
  • possibly create some custom data structures to better track usage of memory and number of API calls

Given that one of Painless's primary design goals is performance, I would like to propose a final feature if we do decide we would like more safety features -- two modes, safety and performance. As each of the above described features would likely add a non-insignificant amount of overhead, we could have a mode where they are disabled if the user requires the best performance available. Or we could even have a selection of enabled/disabled safety features, but this may be overly complex.

I would be interested in hearing about everyone's ideas, thoughts, and concerns related to this issue. @simonw @nik9000 @rjernst

@jdconrad jdconrad added discuss :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache labels Apr 25, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@jdconrad
Copy link
Contributor Author

For regex this might be a good solution (https://www.exratione.com/2017/06/preventing-unbounded-regular-expression-operations-in-java/) -- we could use a counter in place of system time to avoid system calls or something along those lines instead.

@nik9000
Copy link
Member

nik9000 commented May 31, 2018 via email

@henningandersen
Copy link
Contributor

Using the scripted metric aggregation, it is possible to build up large maps that can OOM the node without any circuit breaking. It would be nice to have a way of circuit breaking during scripting when the data that the script holds on to grows too big (or at least give the parent circuit breaker a chance to trip).

@rjernst
Copy link
Member

rjernst commented Sep 24, 2019

@henningandersen Do you mean specifically with the map script, or with combine and/or reduce? In any case, I think this can be done by having "smart" objects which are passed into the script instead of plain Maps. This way after the agg calls execute for each document, it can update the circuit breaker with any change in the data size.

@henningandersen
Copy link
Contributor

@rjernst I think all 3 scripts could experience this. The map script is probably the primary way too much data is collected, but the combine (and reduce) script then risks gathering a similar amount (but in one script invocation).
A complexity in finding out the memory usage is shared objects, where combine in some cases returns the collected datastructure directly and in other cases a new structure, could be a map with same keys, but other values.

@stu-elastic
Copy link
Contributor

Closed in favor of #49871 #49872 #49873

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >feature Meta
Projects
None yet
Development

No branches or pull requests

8 participants