Skip to content

Adds usage data for ILM#33377

Merged
colings86 merged 6 commits intoelastic:index-lifecyclefrom
colings86:ilm/usage-data
Oct 24, 2018
Merged

Adds usage data for ILM#33377
colings86 merged 6 commits intoelastic:index-lifecyclefrom
colings86:ilm/usage-data

Conversation

@colings86
Copy link
Copy Markdown
Contributor

@colings86 colings86 commented Sep 4, 2018 •
edited
Loading

Currently the response from the x-pack usage API would be:

"ilm": {
    "policy_count": 4,
    "policy_stats": [
      {
        "phases": {
          "warm": {
            "after": "0ms",
            "actions": [
              "allocate",
              "forcemerge"
            ]
          },
          "hot": {
            "after": "0ms",
            "actions": [
              "rollover"
            ]
          },
          "delete": {
            "after": "2m",
            "actions": [
              "delete"
            ]
          }
        }
      },
      {
        "phases": {
          "hot": {
            "after": "0ms",
            "actions": [
              "rollover"
            ]
          },
          "delete": {
            "after": "2m",
            "actions": [
              "delete"
            ]
          }
        }
      },
      {
        "phases": {
          "warm": {
            "after": "0ms",
            "actions": [
              "allocate",
              "shrink",
              "forcemerge"
            ]
          },
          "cold": {
            "after": "1m",
            "actions": [
              "allocate"
            ]
          },
          "hot": {
            "after": "0ms",
            "actions": [
              "rollover"
            ]
          },
          "delete": {
            "after": "2m",
            "actions": [
              "delete"
            ]
          }
        }
      },
      {
        "phases": {
          "warm": {
            "after": "0ms",
            "actions": [
              "allocate",
              "forcemerge"
            ]
          },
          "cold": {
            "after": "1m",
            "actions": [
              "allocate"
            ]
          },
          "hot": {
            "after": "0ms",
            "actions": [
              "rollover"
            ]
          },
          "delete": {
            "after": "2m",
            "actions": [
              "delete"
            ]
          }
        }
      }
    ]
  },

@colings86 colings86 added review :Data Management/ILM+SLM DO NOT USE. Use ":StorageEngine/ILM" or ":Distributed Coordination/SLM" instead. labels Sep 4, 2018
@colings86 colings86 self-assigned this Sep 4, 2018
@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Sep 4, 2018

Pinging @elastic/es-core-infra

@elasticmachine elasticmachine mentioned this pull request Sep 4, 2018
Closed
@colings86
Copy link
Copy Markdown
Contributor Author

colings86 commented Sep 4, 2018

@elasticmachine test this please

1 similar comment
@colings86
Copy link
Copy Markdown
Contributor Author

colings86 commented Sep 7, 2018

@elasticmachine test this please

@talevy
Copy link
Copy Markdown
Contributor

talevy commented Sep 13, 2018

Do we know how people are expected to use this data? I believe it does a great job providing all the policy usage info on a per-policy basis for users to group and keep track of things.

What I am not clear on is, who is expected to consume this data? is it other Elasticsearch toolings, or users?
For example, if counts of action usages are important, maybe this structure would pre-calculate things for the user:

{
 <phase_name>: {
     "after": ["1m", "5d", "13d"],
     "actions": {
        <action_name>: <num_times_action_used_in_this_phase>,
        ...
     }
 }
}

@eskibars, since Colin is out, I would love your input on this since I am missing some context. thanks!

@talevy
Copy link
Copy Markdown
Contributor

talevy commented Sep 14, 2018

After speaking with @eskibars in a different channel, I learned that these documents are intended to be aggregated as documents in ES, and so their structure should reflect that intention.

If that is the case, should the phase be contained in a field value that can be used in term aggregations?

@rjernst rjernst removed the review label Oct 10, 2018
talevy
talevy suggested changes Oct 23, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@talevy talevy left a comment

Choose a reason for hiding this comment

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

I have left a few comments

...src/main/java/org/elasticsearch/xpack/core/indexlifecycle/IndexLifecycleFeatureSetUsage.java Outdated
Copy link
Copy Markdown
Contributor

@talevy talevy Oct 23, 2018

Choose a reason for hiding this comment

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

after is now minimum_age

...src/main/java/org/elasticsearch/xpack/core/indexlifecycle/IndexLifecycleFeatureSetUsage.java Outdated
Copy link
Copy Markdown
Contributor

@talevy talevy Oct 23, 2018

Choose a reason for hiding this comment

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

I think it makes sense to store this in milliseconds so that we can do statistics on it?

...src/main/java/org/elasticsearch/xpack/core/indexlifecycle/IndexLifecycleFeatureSetUsage.java Outdated
Copy link
Copy Markdown
Contributor

@talevy talevy Oct 23, 2018

Choose a reason for hiding this comment

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

at first I thought this format would make it difficult to know how many policies didn't use a certain field, but after a few test queries, it looks like this is sufficient

@colings86
Copy link
Copy Markdown
Contributor Author

colings86 commented Oct 24, 2018

@talevy I addressed your comment, could you take another look?

talevy
talevy approved these changes Oct 24, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@talevy talevy left a comment

Choose a reason for hiding this comment

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

I think this will be good!

should we add yaml tests?

@colings86 colings86 merged commit e7fddb5 into elastic:index-lifecycle Oct 24, 2018
colings86 added a commit that referenced this pull request Oct 24, 2018
@colings86
Adds usage data for ILM (#33377)
04798e2 
* Adds usage data for ILM

* Adds tests for IndexLifecycleFeatureSetUsage and friends

* Adds tests for IndexLifecycleFeatureSet

* Fixes merge errors

* Add number of indices managed to usage stats

Also adds more tests

* Addresses Review comments
@colings86 colings86 deleted the ilm/usage-data branch November 12, 2018 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eskibars eskibars Awaiting requested review from eskibars

@jasontedor jasontedor Awaiting requested review from jasontedor

1 more reviewer

@talevy talevy talevy approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@colings86 colings86

Labels

:Data Management/ILM+SLM DO NOT USE. Use ":StorageEngine/ILM" or ":Distributed Coordination/SLM" instead.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@colings86 @elasticmachine @talevy @rjernst