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

[8.6] [Security Solution][Alerts] improves performance of new terms multi fields (#145167) #145697

Merged
merged 1 commit into from
Nov 18, 2022

Conversation

kibanamachine
Copy link
Contributor

Backport

This will backport the following commits from main to 8.6:

Questions ?

Please refer to the Backport tool documentation

…ields (elastic#145167)

## Summary

This PR improves performance of new terms multiple fields implementation
released in elastic#143943
In comparison with single value fields it's faster in 2-2.5 times
In comparison with array field the win even more significant: 3-4 times

### Technical implementation:
Value for runtime field is emitted only if any of new terms fields
present in `include` clause of terms aggregation. It's achieved by
passing a values map of new terms fields to the runtime script and
checking new terms values in it. So, there is a significant performance
wins if runtime field is not matched with includes values:
   - new terms fields are not encoded in base64 within runtime script
   - runtime script doesn't emit any values
- runtime field doesn't have any value to be compared against during
aggregation, as its empty
- it eliminates possible unyielding execution branches early: if one of
items in array of first new terms field is not present in values map, no
need to run through the rest of combinations

As a result, this implementation overcomes the issue with non exhaustive
results due to the large number of emitted fields.
ES doesn't allow emitting more than 100 values in the runtime script, so
if the number of all combinations in new terms fields is greater than
100, only the first 100 combinations will be used for terms aggregation.
With this new implementation only matched fields will be emitting. Even
if its number is greater than 100, we will hit circuit breaker in
Security Solution: as rule run can't generate more than 100 alerts

### Performance measurements

Implementation | Shards | Docs per shard | Simultaneous Rule Executions
| Fields cardinality | Rule Execution Time (improved) | Rule Execution
Time (old)
-- | -- | -- | -- | -- | -- | --
Terms 1 field | 10 | 900,000 | 1 | 100,000 | 7s |  
Terms 2 fields | 10 | 900,000 | 1 | 100,000 | 17s | 33s
Terms 2 fields | 10 | 900,000 | 2 | 100,000 | 19s |  
Terms 3 fields | 10 | 900,000 | 1 | 100,000 | 18s | 46s
Terms 3 fields | 10 | 900,000 | 2 | 100,000 | 20s |  
  |   |   |   |   |   |  
Terms 1 field | 20 | 900,000 | 1 | 100,000 | 10.5s |  
Terms 2 fields | 20 | 900,000 | 1 | 100,000 | 28s | 55s
Terms 2 fields | 20 | 900,000 | 2 | 100,000 | 28.5s | 56s
Terms 3 fields | 20 | 900,000 | 1 | 100,000 | 30s | 75s
Terms 3 fields | 20 | 900,000 | 2 | 100,000 | 31s | 75s
  |   |   |   |   |   |  
Terms 1 field | 10 | 1,800,000 | 1 | 100,000 | 7s |  
Terms 2 fields | 10 | 1,800,000 | 1 | 100,000 | 24s | 50s
Terms 3 fields | 10 | 1,800,000 | 1 | 100,000 | 26s | 68s
  |   |   |   |   |   |  
array of unique values length 10 |   |   |   |   |   |  
Terms 1 field | 10 | 900,000 | 1 | 100,000 | 9.5s |  
Terms 2 fields | 10 | 900,000 | 1 | 100,000 | 75s | 3.5m
Terms 3 fields | 10 | 900,000 | 1 | 100,000 | 83s | 6m

### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Marshall Main <55718608+marshallmain@users.noreply.github.com>
(cherry picked from commit b985ec1)
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
fleet 59 65 +6
osquery 108 113 +5
securitySolution 441 447 +6
total +19

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
fleet 67 73 +6
osquery 109 115 +6
securitySolution 518 524 +6
total +20

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @vitaliidm

@kibanamachine kibanamachine merged commit d9d8b15 into elastic:8.6 Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants