[ILM] Add date setting to calculate index age #46561

andreidan · 2019-09-10T17:23:53Z

Add the index.lifecycle.origination_date to allow users to configure a
custom date that'll be used to calculate the index age for the phase
transmissions (as opposed to the default index creation date).

This could be useful for users to create an index with an "older"
origination date when indexing old data.

Relates to #42449.

Have you signed the contributor license agreement?
Have you followed the contributor guidelines?
If submitting code, have you built your formula locally prior to submission with gradle check?
If submitting code, is your pull request against master? Unless there is a good reason otherwise, we prefer pull requests against master and will backport as needed.
If submitting code, have you checked that your submission is for an OS and architecture that we support?
If you are submitting this code for a class then read our policy for that.

Add the `index.lifecycle.origination_date` to allow users to configure a custom date that'll be used to calculate the index age for the phase transmissions (as opposed to the default index creation date). This could be useful for users to create an index with an "older" origination date when indexing old data. Relates to elastic#42449.

elasticmachine · 2019-09-10T17:24:31Z

Pinging @elastic/es-core-features

andreidan · 2019-09-10T17:27:21Z

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/LifecycleExecutionState.java

@@ -259,6 +269,7 @@ public int hashCode() {
        private Long phaseTime;
        private Long actionTime;
        private Long stepTime;
+        private Long indexOriginationDate;


As the index.creation_date and index.lifecycle.origination_date settings have default values, would it be worth working with primitives here?

dakrone

I left some comments on this, I think we should approach it more towards the "read time" rather than overwriting the creation date when initializing the policy.

server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/LifecycleExecutionState.java

dakrone · 2019-09-10T17:52:27Z

@andreidan can you also add version labels to this PR? Since this is an enhancement it should go to 8.0.0 and 7.5.0 (latest master and 7.x branch versions)

The initial approach we took was to override the lifecycle creation date if the `index.lifecycle.origination_date` setting was set. This had the disadvantage of the user not being able to update the `origination_date` anymore once set. This commit changes the way we makes use of the `index.lifecycle.origination_date` setting by checking its value when we calculate the index age (ie. at "read time") and, in case it's not set, default to the index creation date.

andreidan · 2019-09-11T12:30:17Z

retest this please

dakrone

This approach looks much better, thanks @andreidan, I left some more comments that should be addressed, but nothing major

dakrone · 2019-09-11T14:57:57Z

server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java

@@ -208,6 +208,7 @@ public void validate(Integer numRoutingShards, Map<Setting<Integer>, Integer> se
    public static final String SETTING_VERSION_UPGRADED = "index.version.upgraded";
    public static final String SETTING_VERSION_UPGRADED_STRING = "index.version.upgraded_string";
    public static final String SETTING_CREATION_DATE = "index.creation_date";
+


It'd be good to revert the changes to this file since we don't need it any more (we tend to try not to touch files that don't have code changes even if it would make it cleaner)

dakrone · 2019-09-11T14:58:55Z

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/LifecycleExecutionState.java

-            return new LifecycleExecutionState(phase, action, step, failedStep, stepInfo, phaseDefinition, indexCreationDate,
-                phaseTime, actionTime, stepTime);
+            return new LifecycleExecutionState(phase, action, step, failedStep, stepInfo, phaseDefinition,
+                indexCreationDate, phaseTime, actionTime, stepTime);


Same here we can probably undo this change (less changes of any conflicts in the future since this change likely won't go back to 7.4 and below)

dakrone · 2019-09-11T15:00:12Z

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/LifecycleSettings.java

@@ -17,6 +17,7 @@
    public static final String LIFECYCLE_POLL_INTERVAL = "indices.lifecycle.poll_interval";
    public static final String LIFECYCLE_NAME = "index.lifecycle.name";
    public static final String LIFECYCLE_INDEXING_COMPLETE = "index.lifecycle.indexing_complete";
+    public static final String SETTING_LIFECYCLE_ORIGINATION_DATE = "index.lifecycle.origination_date";


Minor nit, we can drop the SETTING_ prefix from the var since this is just the string rather than a Setting option

dakrone · 2019-09-11T16:33:36Z

docs/reference/ilm/policy-definitions.asciidoc

+case the `min_age` will be the time elapsed since that specified date. If the
+index is rolled over, then `min_age` is the time elapsed from the time the
+index is rolled over. The intention here is to execute following phases and
+actions relative to when data was written last to a rolled over index.


I think this documentation looks good, but can you also add some in ilm-settings.asciidoc for the index.lifecycle.origination_date setting

dakrone · 2019-09-11T16:35:06Z

.../plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/IndexLifecycleInitialisationTests.java

+                createIndexRequest("test").settings(
+                    Settings.builder()
+                        .put(settings)
+                        .put(LifecycleSettings.SETTING_LIFECYCLE_ORIGINATION_DATE, 1000L)


Can you add another check underneath this that setting this setting back to null goes back to using the original lifecycle creation date?

gwbrown · 2019-09-11T16:57:44Z

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/LifecycleSettings.java

@@ -29,6 +30,8 @@
        Setting.Property.Dynamic, Setting.Property.IndexScope);
    public static final Setting<Boolean> LIFECYCLE_INDEXING_COMPLETE_SETTING = Setting.boolSetting(LIFECYCLE_INDEXING_COMPLETE, false,
        Setting.Property.Dynamic, Setting.Property.IndexScope);
+    public static final Setting<Long> LIFECYCLE_ORIGINATION_DATE_SETTING =
+        Setting.longSetting(SETTING_LIFECYCLE_ORIGINATION_DATE, -1, -1, Setting.Property.IndexScope, Setting.Property.NodeScope);


Why is this setting NodeScope? I think it should be IndexScope and Dynamic (so that it's updateable after index creation).

Good catch :)

andreidan · 2019-09-12T12:33:32Z

retest this please

dakrone

LGTM, thanks for iterating on this @andreidan

dakrone · 2019-09-12T15:05:31Z

server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java

        public long settingsVersion() {
            return settingsVersion;
        }
-        
+


We could revert the changes to this file (just so it doesn't end up in the change list), but it is so minor it's up to you.

andreidan · 2019-09-12T15:23:28Z

@elasticmachine test this please

andreidan · 2019-09-12T15:41:18Z

@elasticmachine update branch

* [ILM] Add date setting to calculate index age Add the `index.lifecycle.origination_date` to allow users to configure a custom date that'll be used to calculate the index age for the phase transmissions (as opposed to the default index creation date). This could be useful for users to create an index with an "older" origination date when indexing old data. Relates to elastic#42449. * [ILM] Don't override creation date on policy init The initial approach we took was to override the lifecycle creation date if the `index.lifecycle.origination_date` setting was set. This had the disadvantage of the user not being able to update the `origination_date` anymore once set. This commit changes the way we makes use of the `index.lifecycle.origination_date` setting by checking its value when we calculate the index age (ie. at "read time") and, in case it's not set, default to the index creation date. * Make origination date setting index scope dynamic * Document orignation date setting in ilm settings (cherry picked from commit d5bd2bb) Signed-off-by: Andrei Dan <[email protected]>

* [ILM] Add date setting to calculate index age Add the `index.lifecycle.origination_date` to allow users to configure a custom date that'll be used to calculate the index age for the phase transmissions (as opposed to the default index creation date). This could be useful for users to create an index with an "older" origination date when indexing old data. Relates to #42449. * [ILM] Don't override creation date on policy init The initial approach we took was to override the lifecycle creation date if the `index.lifecycle.origination_date` setting was set. This had the disadvantage of the user not being able to update the `origination_date` anymore once set. This commit changes the way we makes use of the `index.lifecycle.origination_date` setting by checking its value when we calculate the index age (ie. at "read time") and, in case it's not set, default to the index creation date. * Make origination date setting index scope dynamic * Document orignation date setting in ilm settings (cherry picked from commit d5bd2bb) Signed-off-by: Andrei Dan <[email protected]>

andreidan added 7x :Data Management/ILM+SLM Index and Snapshot lifecycle management >enhancement labels Sep 10, 2019

andreidan removed the 7x label Sep 10, 2019

andreidan commented Sep 10, 2019

View reviewed changes

andreidan requested review from gwbrown and dakrone September 10, 2019 17:27

dakrone requested changes Sep 10, 2019

View reviewed changes

andreidan added v7.5.0 v8.0.0 labels Sep 10, 2019

andreidan force-pushed the ilm-origination-date branch from ca13da7 to 18ba82d Compare September 11, 2019 11:58

andreidan requested a review from dakrone September 11, 2019 12:58

dakrone reviewed Sep 11, 2019

View reviewed changes

gwbrown reviewed Sep 11, 2019

View reviewed changes

andreidan added 6 commits September 12, 2019 09:40

Drop empty line

99606c6

Undo constructor call arguments wrap

1a72e4d

Drop SETTING from variable name

cf3f431

Make origination date setting index scope dynamic

d0530cd

Document orignation date setting in ilm settings

cdb1d2f

Test setting the origination date back to null

7487999

Drop quotes from setting description

a087d41

andreidan requested review from dakrone and gwbrown September 12, 2019 13:16

dakrone approved these changes Sep 12, 2019

View reviewed changes

Merge branch 'master' into ilm-origination-date

4da8a76

andreidan merged commit d5bd2bb into elastic:master Sep 12, 2019

andreidan added the backport pending label Sep 12, 2019

andreidan mentioned this pull request Sep 13, 2019

[ILM] Add date setting to calculate index age (#46561) #46697

Merged

andreidan removed the backport pending label Sep 16, 2019

andreidan mentioned this pull request Sep 23, 2019

ILM: parse origination date from index name #46755

Merged

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ILM] Add date setting to calculate index age #46561

[ILM] Add date setting to calculate index age #46561

andreidan commented Sep 10, 2019

elasticmachine commented Sep 10, 2019

andreidan Sep 10, 2019

dakrone left a comment

dakrone commented Sep 10, 2019

andreidan commented Sep 11, 2019

dakrone left a comment

dakrone Sep 11, 2019

dakrone Sep 11, 2019

dakrone Sep 11, 2019

dakrone Sep 11, 2019

dakrone Sep 11, 2019

gwbrown Sep 11, 2019

dakrone Sep 11, 2019

andreidan commented Sep 12, 2019

dakrone left a comment

dakrone Sep 12, 2019

andreidan commented Sep 12, 2019

andreidan commented Sep 12, 2019

[ILM] Add date setting to calculate index age #46561

[ILM] Add date setting to calculate index age #46561

Conversation

andreidan commented Sep 10, 2019

elasticmachine commented Sep 10, 2019

Choose a reason for hiding this comment

dakrone left a comment

Choose a reason for hiding this comment

dakrone commented Sep 10, 2019

andreidan commented Sep 11, 2019

dakrone left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andreidan commented Sep 12, 2019

dakrone left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andreidan commented Sep 12, 2019

andreidan commented Sep 12, 2019