Skip to content

Conversation

nielsbauman
Copy link
Contributor

This action solely needs the cluster state, it can run on any node. Additionally, it needs to be cancellable to avoid doing unnecessary work after a client failure or timeout.

Relates #101805

This action solely needs the cluster state, it can run on any node.
Additionally, it needs to be cancellable to avoid doing unnecessary work
after a client failure or timeout.

Relates elastic#101805
@nielsbauman nielsbauman added >enhancement :Data Management/ILM+SLM Index and Snapshot lifecycle management Team:Data Management Meta label for data/management team v9.1.0 labels Feb 18, 2025
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

Hi @nielsbauman, I've created a changelog YAML for you.

@nielsbauman
Copy link
Contributor Author

Related to #120982

* {@link #indices(String...)} method
*/
public class ExplainLifecycleRequest extends ClusterInfoRequest<ExplainLifecycleRequest> {
public class ExplainLifecycleRequest extends LocalClusterStateRequest implements IndicesRequest.Replaceable {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ClusterInfo classes feel a bit off; the only value they add are specifying indices and indicesOptions, but loads of other actions specify those manually. Therefore, we might as well get rid of the ClusterInfo abstraction and let every class define by themselves what fields they have.

// If this is requesting only errors, only include indices in the error step or which are using a nonexistent policy
if (onlyErrors == false
|| (ErrorStep.NAME.equals(lifecycleState.step()) || indexLifecycleService.policyExists(policyName) == false)) {
if (onlyErrors == false || (ErrorStep.NAME.equals(lifecycleState.step()) || policyExists == false)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The IndexLifecycleService only runs on the master node, which meant that policyExists was always false. I changed it to use the Metadata to check for policy existence.

Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one is missing the ((CancellableTask) task).ensureNotCancelled(); call somewhere, right?

@nielsbauman nielsbauman requested a review from dakrone March 7, 2025 14:42
Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nielsbauman nielsbauman merged commit 9cecc89 into elastic:main Mar 10, 2025
17 checks passed
@nielsbauman nielsbauman deleted the local-explain-lifecycle branch March 10, 2025 08:43
georgewallace pushed a commit to georgewallace/elasticsearch that referenced this pull request Mar 11, 2025
This action solely needs the cluster state, it can run on any node.
Additionally, it needs to be cancellable to avoid doing unnecessary work
after a client failure or timeout.

Relates elastic#101805
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Data Management/ILM+SLM Index and Snapshot lifecycle management >enhancement Team:Data Management Meta label for data/management team v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants