Skip to content

Conversation

@nielsbauman
Copy link
Contributor

This avoids subclasses from having to look up the index metadata themselves, which in turn saves some null-checks.

…d of `Index`

This avoids subclasses from having to look up the index metadata
themselves, which in turn saves some null-checks.
@nielsbauman nielsbauman added >refactoring :Data Management/ILM+SLM Index and Snapshot lifecycle management auto-backport Automatically create backport pull requests when merged v8.19.3 labels Aug 21, 2025
@elasticsearchmachine elasticsearchmachine added v9.2.0 Team:Data Management Meta label for data/management team labels Aug 21, 2025
@elasticsearchmachine
Copy link
Collaborator

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

@lukewhiting lukewhiting requested a review from Copilot August 21, 2025 07:53
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the AsyncWaitStep#evaluateCondition method to accept an IndexMetadata parameter instead of an Index parameter. This change eliminates the need for subclasses to perform index metadata lookups and associated null-checks, simplifying the implementation.

Key changes:

  • Modified the abstract method signature in AsyncWaitStep to take IndexMetadata instead of Index
  • Updated all concrete implementations to use the provided IndexMetadata directly
  • Updated test files to pass IndexMetadata objects to the method calls

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.

Show a summary per file
File Description
AsyncWaitStep.java Changed abstract method signature to accept IndexMetadata parameter
IndexLifecycleRunner.java Updated method call to pass IndexMetadata instead of Index
WaitUntilTimeSeriesEndTimePassesStep.java Removed index metadata lookup and null-check, uses provided parameter
WaitUntilReplicateForTimePassesStep.java Removed index metadata lookup and null-check, uses provided parameter
WaitForSnapshotStep.java Removed index metadata lookup and null-check, uses provided parameter
WaitForRolloverReadyStep.java Updated to use provided IndexMetadata parameter directly
WaitForNoFollowersStep.java Simplified to use IndexMetadata parameter and extract index name
WaitForFollowShardTasksStep.java Removed index metadata lookup, uses provided parameter
SegmentCountStep.java Updated to use IndexMetadata parameter and extract Index when needed
Multiple test files Updated test method calls to pass IndexMetadata objects

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

@lukewhiting lukewhiting 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 enabled auto-merge (squash) August 21, 2025 07:56
@nielsbauman nielsbauman merged commit b0c4ca4 into elastic:main Aug 21, 2025
34 checks passed
@nielsbauman nielsbauman deleted the ilm-async-wait-refactor branch August 21, 2025 08:46
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
8.19 Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 133292

@nielsbauman
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
8.19

Questions ?

Please refer to the Backport tool documentation

elasticsearchmachine pushed a commit that referenced this pull request Aug 21, 2025
… instead of `Index` (#133292) (#133300)

* Make `AsyncWaitStep#evaluateCondition` take an `IndexMetadata` instead of `Index` (#133292)

This avoids subclasses from having to look up the index metadata
themselves, which in turn saves some null-checks.

(cherry picked from commit b0c4ca4)

# Conflicts:
#	x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/AsyncWaitStep.java
#	x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/SegmentCountStep.java
#	x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/WaitForFollowShardTasksStep.java
#	x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/WaitForNoFollowersStep.java
#	x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/WaitForRolloverReadyStep.java
#	x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/WaitForSnapshotStep.java
#	x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/WaitUntilReplicateForTimePassesStep.java
#	x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/WaitUntilTimeSeriesEndTimePassesStep.java
#	x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/SegmentCountStepTests.java
#	x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForFollowShardTasksStepTests.java
#	x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForNoFollowersStepTests.java
#	x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForRolloverReadyStepTests.java
#	x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForSnapshotStepTests.java
#	x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitUntilReplicateForTimePassesStepTests.java
#	x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitUntilTimeSeriesEndTimePassesStepTests.java
#	x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleRunner.java
#	x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/IndexLifecycleRunnerTests.java

* [CI] Auto commit changes from spotless

---------

Co-authored-by: elasticsearchmachine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged :Data Management/ILM+SLM Index and Snapshot lifecycle management >refactoring Team:Data Management Meta label for data/management team v8.19.3 v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants