-
Notifications
You must be signed in to change notification settings - Fork 25.5k
add _retry API to index lifecycle policies #30769
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
Conversation
Pinging @elastic/es-core-infra |
lifecycle: "my_lifecycle" | ||
|
||
--- | ||
"Test Basic Re-run": |
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.
Hey @colings86, do you have an idea of which real policy configuration can deterministically lead us into an error-step? This test will never pass since the steps-registry will not return the error unless it actually happened.
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.
One way that I found accidentally is to configure a rollover action to rollover an alias that does not exist. I would expect that trying to shrink an index to a number of shards that it not a factor of the original number of shards would cause the error step too (e.g. try to shrink a 5 shard index to 2 shards).
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.
thanks, this scenario does work, but I cannot get the timing to work.
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'll skip this for now
|
||
public class ReRunAction extends Action<ReRunAction.Request, ReRunAction.Response, ReRunAction.RequestBuilder> { | ||
public static final ReRunAction INSTANCE = new ReRunAction(); | ||
public static final String NAME = "cluster:admin/xpack/index_lifecycle/_rerun/post"; |
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 think we will want this to be an indices action rather than a cluster action. Also the request can then implement IndicesRequest
so its API is consistent with how we do other requests that take an index on the url path
thanks for the initial review @colings86, I've updated to have more test coverage and updated to support multiple indices |
return new Response(); | ||
} | ||
|
||
ClusterState innerExecute(ClusterState currentState, String[] indices) { |
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.
Can we put this method in IndexLifecycleRunner to keep all the step transition logic in the same place?
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.
yes
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.
The change itself LGTM but I think we need a better name then rerun (I know I proposed that name to begin with). The problem I see it that rerun sounds like its an API to re-run the entire lifecycle for an index (especially when though of in the context of the URL pattern), so I think we need a name that is a bit more intuitive as to what the API actually does.
This API adds the ability to re-run a policy for an index that encountered an error (moved to error step)