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.

Sign up for GitHub

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

Jump to bottom

Scripting: stats per context in nodes stats #54008

Merged
merged 13 commits into from
Mar 26, 2020
Merged

Scripting: stats per context in nodes stats #54008

merged 13 commits into from
Mar 26, 2020

Conversation

stu-elastic
Copy link
Contributor

@stu-elastic stu-elastic commented Mar 23, 2020 •
edited
Loading

Adds script cache stats to _node/stats.
If using the general cache:

      "script_cache": {
        "sum": {
          "compilations": 12,
          "cache_evictions": 9,
          "compilation_limit_triggered": 5
        }
      }

If using context caches:

      "script_cache": {       
        "sum": {                 
          "compilations": 13,                
          "cache_evictions": 9,
          "compilation_limit_triggered": 5
        },                                
        "contexts": [         
          {                      
            "context": "aggregation_selector",
            "compilations": 8,
            "cache_evictions": 6,
            "compilation_limit_triggered": 3
          },                  
          {                      
            "context": "aggs",              
            "compilations": 5,
            "cache_evictions": 3,
            "compilation_limit_triggered": 2
          },

Refs: #50152

@stu-elastic stu-elastic added >enhancement WIP :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache labels Mar 23, 2020
@stu-elastic stu-elastic requested review from rjernst and jdconrad March 23, 2020 18:18
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (:Core/Infra/Scripting)

williamrandolph
williamrandolph suggested changes Mar 24, 2020
View reviewed changes
Copy link
Contributor

@williamrandolph williamrandolph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. One question, one small change request.

server/src/main/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStats.java Outdated Show resolved Hide resolved
server/src/main/java/org/elasticsearch/script/ScriptCacheStats.java Outdated Show resolved Hide resolved
@stu-elastic stu-elastic removed the WIP label Mar 26, 2020
williamrandolph
williamrandolph approved these changes Mar 26, 2020
View reviewed changes
Copy link
Contributor

@williamrandolph williamrandolph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, though I haven't dug into how the tests are set up.

jdconrad
jdconrad approved these changes Mar 26, 2020
View reviewed changes
Copy link
Contributor

@jdconrad jdconrad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@stu-elastic stu-elastic merged commit 32f46f2 into elastic:master Mar 26, 2020
stu-elastic added a commit to stu-elastic/elasticsearch that referenced this pull request Mar 26, 2020
@stu-elastic
Scripting: stats per context in nodes stats (elastic#54008)
8f24ca7 
Adds script cache stats to `_node/stats`.
If using the general cache:
```
      "script_cache": {
        "sum": {
          "compilations": 12,
          "cache_evictions": 9,
          "compilation_limit_triggered": 5
        }
      }

```
If using context caches:
```
      "script_cache": {
        "sum": {
          "compilations": 13,
          "cache_evictions": 9,
          "compilation_limit_triggered": 5
        },
        "contexts": [
          {
            "context": "aggregation_selector",
            "compilations": 8,
            "cache_evictions": 6,
            "compilation_limit_triggered": 3
          },
          {
            "context": "aggs",
            "compilations": 5,
            "cache_evictions": 3,
            "compilation_limit_triggered": 2
          },
```
Refs: elastic#50152
@dnhatn dnhatn mentioned this pull request Mar 27, 2020
Closed
original-brownbear added a commit to original-brownbear/elasticsearch that referenced this pull request Mar 27, 2020
@original-brownbear
Correct Version Constant in NodeStats
163eaac 
In elastic#54008 we used v7.8 here but never backported to 7.x.
now that 7.x has moved to 7.8 BwC serialization is broken => adjusting the constant here until we backport.
@original-brownbear original-brownbear mentioned this pull request Mar 27, 2020
Merged
original-brownbear added a commit that referenced this pull request Mar 27, 2020
@original-brownbear
Correct Version Constant in NodeStats (#54321)
1aa3537 
In #54008 we used v7.8 here but never backported to 7.x.
now that 7.x has moved to 7.8 BwC serialization is broken => adjusting the constant here until we backport.
Closes #54313
stu-elastic added a commit to stu-elastic/elasticsearch that referenced this pull request Mar 27, 2020
@stu-elastic
Scripting: stats per context in nodes stats (elastic#54008)
a5b7627 
Adds script cache stats to `_node/stats`.
If using the general cache:
```
      "script_cache": {
        "sum": {
          "compilations": 12,
          "cache_evictions": 9,
          "compilation_limit_triggered": 5
        }
      }

```
If using context caches:
```
      "script_cache": {
        "sum": {
          "compilations": 13,
          "cache_evictions": 9,
          "compilation_limit_triggered": 5
        },
        "contexts": [
          {
            "context": "aggregation_selector",
            "compilations": 8,
            "cache_evictions": 6,
            "compilation_limit_triggered": 3
          },
          {
            "context": "aggs",
            "compilations": 5,
            "cache_evictions": 3,
            "compilation_limit_triggered": 2
          },
```
Refs: elastic#50152
stu-elastic added a commit that referenced this pull request Mar 27, 2020
@stu-elastic
Scripting: stats per context in nodes stats (#54008) (#54357)
1630de4 
Adds script cache stats to `_node/stats`.
If using the general cache:
```
      "script_cache": {
        "sum": {
          "compilations": 12,
          "cache_evictions": 9,
          "compilation_limit_triggered": 5
        }
      }

```
If using context caches:
```
      "script_cache": {
        "sum": {
          "compilations": 13,
          "cache_evictions": 9,
          "compilation_limit_triggered": 5
        },
        "contexts": [
          {
            "context": "aggregation_selector",
            "compilations": 8,
            "cache_evictions": 6,
            "compilation_limit_triggered": 3
          },
          {
            "context": "aggs",
            "compilations": 5,
            "cache_evictions": 3,
            "compilation_limit_triggered": 2
          },
```
Backport of: 32f46f2
Refs: #50152
@stu-elastic
Copy link
Contributor Author

master: 32f46f2
7.x: 1630de4

@russcam russcam mentioned this pull request May 29, 2020
Closed
17 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@williamrandolph williamrandolph williamrandolph approved these changes

@jdconrad jdconrad jdconrad approved these changes

@rjernst rjernst Awaiting requested review from rjernst

Assignees
No one assigned
Labels
:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >enhancement v7.8.0 v8.0.0-alpha1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@stu-elastic @elasticmachine @rjernst @jdconrad @williamrandolph @jakelandis