Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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
Use ILM for Watcher history deletion #37443
Use ILM for Watcher history deletion #37443
Changes from 3 commits
f37b3c9
a01c0db
59b7ab7
474b5c3
9aa2f06
bbcfdb9
f51ca5d
87410da
31c69f5
827bcb0
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any concerns about multiple nodes entering this path and putting the policy multiple times ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 itThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahh.. thanks, I missed this will only run on the master.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.