Use ILM for Watcher history deletion #37443

dakrone · 2019-01-14T23:52:53Z

This commit adds an index lifecycle policy for the .watch-history-* indices.
This policy is automatically used for all new watch history indices.

This does not yet remove the automatic cleanup that the monitoring plugin does
for the .watch-history indices, and it does not touch the
xpack.watcher.history.cleaner_service.enabled setting.

Relates to #32041

There is one TODO about how much we should enforce parity with the on-disk policy, thoughts are appreciated.

This commit adds an index lifecycle policy for the `.watch-history-*` indices. This policy is automatically used for all new watch history indices. This does not yet remove the automatic cleanup that the monitoring plugin does for the .watch-history indices, and it does not touch the `xpack.watcher.history.cleaner_service.enabled` setting. Relates to elastic#32041

elasticmachine · 2019-01-14T23:52:54Z

Pinging @elastic/es-core-features

dakrone · 2019-01-15T16:04:06Z

In our discussion we decided to only put the policy if one didn't exist, not to replace a changed or previously existing policy with the on disk version.

I'll make the charge for this accordingly.

…cher-history

jakelandis · 2019-01-16T22:48:07Z

...cher/src/main/java/org/elasticsearch/xpack/watcher/support/WatcherIndexTemplateRegistry.java

+        return LifecyclePolicyUtils.loadPolicy(POLICY_WATCH_HISTORY.policyName, POLICY_WATCH_HISTORY.fileName, xContentRegistry);
+    }
+
+    private void addIndexLifecyclePolicyIfMissing(ClusterState state) {


Are there any concerns about multiple nodes entering this path and putting the policy multiple times ?

Since this only happens when the local node is master, it should only get called once with the atomic compareAndSet below. Also any subsequent calls would see that the policy already existed and not replace it

ahh.. thanks, I missed this will only run on the master.

minor nit: it is not only the master node, but is also put when this node is newer than the master node. The reason for this is, that with regards to the watcher history, a node that is not the master node might be updated first in a cluster, and that one could write the watch history in a format which requires a new watch history template.

I am not sure if this applies to ILM as well, as this is an administrative task compared to the fact that the watch history is written by non master nodes.

...cher/src/main/java/org/elasticsearch/xpack/watcher/support/WatcherIndexTemplateRegistry.java

...src/test/java/org/elasticsearch/xpack/watcher/support/WatcherIndexTemplateRegistryTests.java

jakelandis · 2019-01-16T23:12:31Z

...cher/src/main/java/org/elasticsearch/xpack/watcher/support/WatcherIndexTemplateRegistry.java

@@ -46,17 +53,23 @@
            TEMPLATE_CONFIG_TRIGGERED_WATCHES, TEMPLATE_CONFIG_WATCH_HISTORY, TEMPLATE_CONFIG_WATCHES
    };

+    public static final PolicyConfig POLICY_WATCH_HISTORY = new PolicyConfig("watch-history-policy", "/watch-history-policy.json");


I remember the discussion to not use a . in the name since this is something that we don't mind users changing. However, I wonder if in the UI there should be some kind of differentiation policies we add vs. policies they add. If so we may want the . after all.

@bmcconaghy - Do you see any reason that the UI for ILM (now or in the future) should differentiate between policies added by the system vs. policies added by a human ?

I remember the discussion to not use a . in the name since this is something that we don't mind users changing. However, I wonder if in the UI there should be some kind of differentiation policies we add vs. policies they add. If so we may want the . after all.

@bmcconaghy - Do you see any reason that the UI for ILM (now or in the future) should differentiate between policies added by the system vs. policies added by a human ?

When we're making amendments to config through the API, we expect system-created objects to being with a . character, and hence we avoid those.

I think it would make sense to adhere to that pattern; it's a bit surprising that you've deviated from it.

Moreover, if a user were to delete this policy because they thought it had been created by another user rather than the system, the system would consequently behave differently to how it's specified to behave in the docs.

hub-cap

Some general questions but looks pretty durn good to me

x-pack/plugin/core/src/main/resources/watch-history-policy.json

...src/main/java/org/elasticsearch/xpack/indexlifecycle/action/TransportPutLifecycleAction.java

...cher/src/main/java/org/elasticsearch/xpack/watcher/support/WatcherIndexTemplateRegistry.java

jakelandis

LGTM and thanks for setting the ground work for future system indices.

dakrone · 2019-01-17T20:14:37Z

@elasticmachine run gradle build tests 2

dakrone · 2019-01-17T22:08:21Z

@elasticmachine run gradle build tests 2

...gin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/LifecyclePolicyUtils.java

dakrone · 2019-01-22T23:03:41Z

@elasticmachine run elasticsearch-ci/2

…cher-history

dakrone · 2019-01-23T16:22:27Z

@elasticmachine run elasticsearch-ci/docs-check

* master: Liberalize StreamOutput#writeStringList (elastic#37768) Add PersistentTasksClusterService::unassignPersistentTask method (elastic#37576) Tests: disable testRandomGeoCollectionQuery on tiny polygons (elastic#37579) Use ILM for Watcher history deletion (elastic#37443) Make sure PutMappingRequest accepts content types other than JSON. (elastic#37720) Retry ILM steps that fail due to SnapshotInProgressException (elastic#37624) Use disassociate in preference to deassociate (elastic#37704) Delete Redundant RoutingServiceTests (elastic#37750) Always return metadata version if metadata is requested (elastic#37674)

* elastic/master: (85 commits) Use explicit version for build-tools in example plugin integ tests (elastic#37792) Change `rational` to `saturation` in script_score (elastic#37766) Deprecate types in get field mapping API (elastic#37667) Add ability to listen to group of affix settings (elastic#37679) Ensure changes requests return the latest mapping version (elastic#37633) Make Minio Setup more Reliable (elastic#37747) Liberalize StreamOutput#writeStringList (elastic#37768) Add PersistentTasksClusterService::unassignPersistentTask method (elastic#37576) Tests: disable testRandomGeoCollectionQuery on tiny polygons (elastic#37579) Use ILM for Watcher history deletion (elastic#37443) Make sure PutMappingRequest accepts content types other than JSON. (elastic#37720) Retry ILM steps that fail due to SnapshotInProgressException (elastic#37624) Use disassociate in preference to deassociate (elastic#37704) Delete Redundant RoutingServiceTests (elastic#37750) Always return metadata version if metadata is requested (elastic#37674) [TEST] Mute MlMappingsUpgradeIT testMappingsUpgrade Streamline skip_unavailable handling (elastic#37672) Only bootstrap and elect node in current voting configuration (elastic#37712) Ensure either success or failure path for SearchOperationListener is called (elastic#37467) Target only specific index in update settings test ...

willemveerman · 2019-08-16T10:25:25Z

Hey just an idea, should this policy's name start with a . character so it follows the pattern for other system objects?

cachedout · 2019-09-11T10:10:47Z

This does not yet remove the automatic cleanup that the monitoring plugin does
for the .watch-history indices, and it does not touch the
xpack.watcher.history.cleaner_service.enabled setting.

Is this work planned for a future release and if so, is there a GH issue that I can follow?

matschaffer · 2020-01-29T05:02:37Z

@willemveerman I had the same thought so opened #51589

dakrone added v7.0.0 :Data Management/Watcher labels Jan 14, 2019

dakrone requested a review from spinscale January 14, 2019 23:52

spinscale requested review from jakelandis and hub-cap January 15, 2019 09:17

dakrone added 2 commits January 15, 2019 09:46

Merge remote-tracking branch 'origin/master' into ilm-use-ilm-for-wat…

a01c0db

…cher-history

Resolve TODO, if an existing policy exists, don't overwrite it

59b7ab7

jakelandis reviewed Jan 16, 2019

View reviewed changes

hub-cap reviewed Jan 17, 2019

View reviewed changes

dakrone added 3 commits January 17, 2019 08:59

Rename policy to watch-history-ilm-policy

474b5c3

Remove unnecessary NamedXContentRegistry entries

9aa2f06

Unset atomicboolean in else clause

bbcfdb9

jakelandis approved these changes Jan 17, 2019

View reviewed changes

dakrone added 2 commits January 17, 2019 10:50

Fix doc test that retrieved all policies instead of specific one

f51ca5d

Fix another doc test

87410da

spinscale reviewed Jan 18, 2019

View reviewed changes

...gin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/LifecyclePolicyUtils.java Show resolved Hide resolved

dakrone added 2 commits January 22, 2019 16:22

Move from 30d -> 7d retention

31c69f5

Merge remote-tracking branch 'origin/master' into ilm-use-ilm-for-wat…

827bcb0

…cher-history

dakrone merged commit 427bc7f into elastic:master Jan 23, 2019

dakrone deleted the ilm-use-ilm-for-watcher-history branch February 4, 2019 14:42

gwbrown mentioned this pull request Feb 5, 2019

Use ILM to manage internal indices #38470

Closed

11 tasks

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

jimczi added the >enhancement label Feb 11, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use ILM for Watcher history deletion #37443

Use ILM for Watcher history deletion #37443

dakrone commented Jan 14, 2019

elasticmachine commented Jan 14, 2019

dakrone commented Jan 15, 2019

jakelandis Jan 16, 2019

dakrone Jan 16, 2019

jakelandis Jan 17, 2019

spinscale Jan 18, 2019

jakelandis Jan 16, 2019

willemveerman Aug 16, 2019

hub-cap left a comment

jakelandis left a comment

dakrone commented Jan 17, 2019

dakrone commented Jan 17, 2019

dakrone commented Jan 22, 2019

dakrone commented Jan 23, 2019

willemveerman commented Aug 16, 2019

cachedout commented Sep 11, 2019

matschaffer commented Jan 29, 2020

Use ILM for Watcher history deletion #37443

Use ILM for Watcher history deletion #37443

Conversation

dakrone commented Jan 14, 2019

elasticmachine commented Jan 14, 2019

dakrone commented Jan 15, 2019

jakelandis Jan 16, 2019

Choose a reason for hiding this comment

dakrone Jan 16, 2019

Choose a reason for hiding this comment

jakelandis Jan 17, 2019

Choose a reason for hiding this comment

spinscale Jan 18, 2019

Choose a reason for hiding this comment

jakelandis Jan 16, 2019

Choose a reason for hiding this comment

willemveerman Aug 16, 2019

Choose a reason for hiding this comment

hub-cap left a comment

Choose a reason for hiding this comment

jakelandis left a comment

Choose a reason for hiding this comment

dakrone commented Jan 17, 2019

dakrone commented Jan 17, 2019

dakrone commented Jan 22, 2019

dakrone commented Jan 23, 2019

willemveerman commented Aug 16, 2019

cachedout commented Sep 11, 2019

matschaffer commented Jan 29, 2020