Parameterizing Painless script

[ruflin]

Filebeat modules use ingest pipelines with scripts inside. Parameterised scripts are preferred and more efficent as they only have to be compiled ones (see https://www.elastic.co/guide/en/elasticsearch/reference/6.x/modules-scripting-using.html#prefer-params). To have efficient pipelines in all modules each pipeline should be reviewed to assure we only use parameterised scripts.

• Fix tests by increasing max_compilations_rate Increase max compilation rate for Filebeat modules tests #9777 This should be revert again when fixed

Related:

• To make reviews of script changes easier, we should tackled Support stored scripts #5339

[jsoriano]

I don't fully understand the example in the docs:

If you need to pass variables into the script, you should pass them in as named params instead of hard-coding values into the script itself. For example, if you want to be able to multiply a field value by different multipliers, don’t hard-code the multiplier into the script:

 "source": "doc['my_field'] * 2"

Instead, pass it in as a named parameter:

  "source": "doc['my_field'] * multiplier",
  "params": {
    "multiplier": 2
  }

The first version has to be recompiled every time the multiplier changes. The second version is only compiled once.

In this case the multiplier is never going to change, it is always going to be 2, why would it be recompiled?

[kaiyan-sheng]

@jsoriano Seems like scripts has params will by default be only compiled once. Found this here: https://qbox.io/blog/advanced-methods-painless-scripting-elasticsearch-syntax-params

[jsoriano]

@kaiyan-sheng yes, this is what I don't understand, why is params required even for values that don't change.

[ruflin]

I think we need @jakelandis to comment on this one. I'm also curious why we need to parametrise constants.

[jakelandis]

You don't need to parameterize constants for ingest node scripts (unless you are constantly changing the pipeline, which you don't want to do for other reasons).

You may want to parameterize if your testing uses randomized constants but not needed for production code. Parameterize the arguments is imo and arguably a best practice for consistency with update scripts.

I don't fully understand the example in the docs

The keyword in there is "different multipliers". The script's cache keys are basically by the script itself as a string, so if that multiplier keeps changing, the string changes and it is cache miss. This maybe a concern for update/upsert scripts but not really for ingest node scripts.

The recent increase in circuit breaker exceptions is likely due to adding support for more template'd fields (starting in '6.4') and the 'if' condition in '6.5'. Once elastic/elasticsearch#37120 is fixed, it should help considerably with the circuit breaker exceptions.

(Also, somewhere, can't find it now, I provided a less concrete answer to a similar question... sorry about that. I have looked at this abit more since and ^^ is the correct answer).

[jsoriano]

@jakelandis thanks for the explanation. Let's see if I understand it now...

So, parametrization is only needed if the script is used with different multipliers. Then, following with the example, recompilations are triggered if there are scripts like these ones:

 "source": "doc['my_field'] * 2"

And

 "source": "doc['my_field'] * 10"

But not if they are defined like:

  "source": "doc['my_field'] * multiplier",
  "params": {
    "multiplier": 2
  }

And

  "source": "doc['my_field'] * multiplier",
  "params": {
    "multiplier": 10
  }

This makes sense.

But if the script is always used as "source": "doc['my_field'] * 2", then recompilations are not triggered because the multiplier (and the script) is always the same. Is this correct?

But then I don't understand why we need to parametrize literals that are never different, like in these cases.

Or maybe we don't need to parametrize so many things? To check recompilations, is there some metric exposing the count of compilations done?

[webmat]

@jsoriano But the real issue here is that all options that can be templated currently trigger a compilation, even if there's no Mustache interpolation in the value. This is what elastic/elasticsearch#37120 will address.

The reason we're seeing this problem on and off is likely that we're hovering close to that limit. Sometimes we trip it up, sometimes we don't. Our attempts at parameterizing just mix things up, they don't really fix anything.

But if you think about all of the processor options that support templating where we don't actually do templating most of the time... I think ES issue will be the game changer that brings everything back to a sane level.

[botelastic]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[jsoriano]

The fix for elastic/elasticsearch#37120 was already merged. I guess we can close this then?

[botelastic]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.