Expose the index age in ILM explain output. #81273

gmarouli · 2021-12-02T20:48:32Z

ILM already exposes the age that ILM will use to transition to the next phase, based on that phase's min_age. The index_age is based only on the index creation date and it's used to trigger a rollover.

Resolves #64429

ILM already exposes the `age` that ILM will use to transition to the next phase, based on that phase's `min_age`. The `index_age` is based only on the index creation date and it's used to trigger a rollover. Resolves elastic#64429

elasticmachine · 2021-12-03T16:22:28Z

Pinging @elastic/es-data-management (Team:Data Management)

gmarouli · 2021-12-03T16:25:14Z

@elasticmachine run elasticsearch-ci/part-2

gmarouli · 2021-12-03T16:31:13Z

@elasticmachine update branch

gmarouli · 2021-12-06T09:11:51Z

@elasticmachine update branch

andreidan

Thanks for working on this Mary.

I think this generally looks good, I've left a few comments.

...lugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/IndexLifecycleExplainResponse.java

docs/reference/ilm/error-handling.asciidoc

docs/reference/ilm/apis/explain.asciidoc

docs/reference/ilm/error-handling.asciidoc

docs/reference/ilm/apis/explain.asciidoc

docs/reference/ilm/ilm-tutorial.asciidoc

.../ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/ExplainLifecycleIT.java

andreidan · 2021-12-08T09:29:55Z

docs/reference/ilm/apis/explain.asciidoc

+      "index_creation_date_millis": 1538475653281,
+      "index_age": "15s",                       <1>


@jrodewig @dakrone what do you think about the naming of these fields? We'll document they will be used by rollover in the age math for conditions, whilst the existing age for phase transition timings.

Do you think this is clear enough?

Tagging @debadair, who has more ILM experience and can probably give better feedback. :)

Instead of index_age, I'd go with something a little more descriptive like time_since_index_creation or time_since_creation. That avoids conflation with age and min_age.

Ideally, we would change age to a more descriptive key too, but that would require a breaking change.

I agree that I think index_age is a little confusing, perhaps something like creation_age, or lifetime would be my suggestions.

I do like that name, it does feel a bit weird that it is not inline with the other age though. Is there any convention we use for such fields in our APIs?

I think I could justify a breaking change to the age key name assuming

there are not any known consumers of this API (other then humans )

it can help make a meaningful difference how we express timings.

For example, if makes sense to keep index_age here but change age to phase_age in 8.0.0 I think that should be on the table too.

Thank you for the ideas, I will bring these options to the team and come back with a proposal.

Our proposal is:

age remains the same because it's already used by kibana so breaking backwards compatibility is not as painless.

time_since_index_creation will be the name of the new exposed field to make it as clear as possible and we intentionally want to avoid the word age so the two will not be easier distinguishable.

Thank you all for helping out :).

gmarouli · 2021-12-09T08:35:59Z

@elasticmachine update branch

andreidan

LGTM, thanks for iterating on this @gmarouli

This makes the ages math a lot clearer.

I've left 2 minor suggestions.

andreidan · 2021-12-13T09:57:49Z

docs/reference/ilm/apis/explain.asciidoc

-      "policy": "my_policy", <2>
-      "lifecycle_date_millis": 1538475653281, <3>
-      "age": "15s", <4>
+      "index_creation_date_millis": 1538475653281,


shall we document this as well?

Yes, documentation added

.../core/src/test/java/org/elasticsearch/xpack/core/ilm/IndexLifecycleExplainResponseTests.java

...lugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/IndexLifecycleExplainResponse.java

gmarouli requested a review from andreidan December 2, 2021 20:48

elasticsearchmachine added the v8.1.0 label Dec 2, 2021

gmarouli force-pushed the add-index-age-ilm-eplain branch from 3ffb124 to eeeeee8 Compare December 2, 2021 21:01

gmarouli removed the request for review from andreidan December 2, 2021 21:01

gmarouli force-pushed the add-index-age-ilm-eplain branch from eeeeee8 to 2ca89b0 Compare December 2, 2021 21:06

Expose the index age in ILM explain output.

beb09b1

ILM already exposes the `age` that ILM will use to transition to the next phase, based on that phase's `min_age`. The `index_age` is based only on the index creation date and it's used to trigger a rollover. Resolves elastic#64429

gmarouli force-pushed the add-index-age-ilm-eplain branch from 2ca89b0 to beb09b1 Compare December 2, 2021 21:14

gmarouli added 3 commits December 3, 2021 10:03

Unify the references of the index creation date

6cb6a44

Reinstate index_creation_date, needed for the XContent parser

8ea9d8a

Skip tests with the new index age from bwc tests

4a6aaf4

gmarouli requested a review from andreidan December 3, 2021 16:20

andreidan added :Data Management/ILM+SLM Index and Snapshot lifecycle management >enhancement labels Dec 3, 2021

elasticmachine added the Team:Data Management Meta label for data/management team label Dec 3, 2021

Merge branch 'master' into add-index-age-ilm-eplain

230f376

elasticmachine and others added 2 commits December 6, 2021 01:11

Merge branch 'master' into add-index-age-ilm-eplain

6b6b308

Only expose the index age if the index is managed

f27c126

andreidan requested changes Dec 8, 2021

View reviewed changes

andreidan reviewed Dec 8, 2021

View reviewed changes

gmarouli added 2 commits December 8, 2021 15:25

Revert formatting changes

1bd344b

Fixed documentation

cb8f936

elasticmachine and others added 3 commits December 9, 2021 00:36

Merge branch 'master' into add-index-age-ilm-eplain

33e46ff

Move age testing to the unit test level

804b1c2

Rename index_age to time_since_index_creation

7d45116

gmarouli requested a review from andreidan December 9, 2021 20:30

andreidan approved these changes Dec 13, 2021

View reviewed changes

andreidan reviewed Dec 13, 2021

View reviewed changes

...lugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/IndexLifecycleExplainResponse.java Outdated Show resolved Hide resolved

Small fixes in age calculation and testing

8cbb2d3

gmarouli merged commit 175c479 into elastic:master Dec 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose the index age in ILM explain output. #81273

Expose the index age in ILM explain output. #81273

gmarouli commented Dec 2, 2021

elasticmachine commented Dec 3, 2021

gmarouli commented Dec 3, 2021

gmarouli commented Dec 3, 2021

gmarouli commented Dec 6, 2021

andreidan left a comment

andreidan Dec 8, 2021

jrodewig Dec 8, 2021 •

edited

dakrone Dec 8, 2021

gmarouli Dec 8, 2021

jakelandis Dec 8, 2021

gmarouli Dec 9, 2021

gmarouli Dec 9, 2021

gmarouli commented Dec 9, 2021

andreidan left a comment

andreidan Dec 13, 2021

gmarouli Dec 13, 2021

		"index_creation_date_millis": 1538475653281,
		"index_age": "15s", <1>

Expose the index age in ILM explain output. #81273

Expose the index age in ILM explain output. #81273

Conversation

gmarouli commented Dec 2, 2021

elasticmachine commented Dec 3, 2021

gmarouli commented Dec 3, 2021

gmarouli commented Dec 3, 2021

gmarouli commented Dec 6, 2021

andreidan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrodewig Dec 8, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gmarouli commented Dec 9, 2021

andreidan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrodewig Dec 8, 2021 •

edited