Remove test usages of DataStream#getDefaultBackingIndexName in ILM integration tests#124319
Merged
gmarouli merged 6 commits intoelastic:mainfrom Mar 10, 2025
Merged
Conversation
…csearch.x-pack.plugin.ilm.qa.multi-node.internalClusterTest`
…csearch.x-pack.plugin.ilm.qa.multi-node.javaRestTest`
Collaborator
|
Pinging @elastic/es-data-management (Team:Data Management) |
14 tasks
nielsbauman
approved these changes
Mar 10, 2025
Contributor
nielsbauman
left a comment
There was a problem hiding this comment.
Left some minor comments, but other than that LGTM. Thanks, Mary! One big step closer to never having these annoying test failures again :)
I think we should backport this because the failures occurred on 8.16 too.
Also, I see #124439 got opened after you opened this PR, so you can close that one with this PR too. Four closed issues with one PR is not too bad at all 😄
Comment on lines
+110
to
+111
| String backingIndexName = getDataStreamBackingIndexNames(dataStream).getFirst(); | ||
| assertThat(backingIndexName, endsWith("1")); |
Contributor
There was a problem hiding this comment.
How do you feel about asserting the size instead of the last character?
I'm thinking far in the future here, but what if we come to a point where we lazily initialize data streams and we first ingest to the failure store, then the failure store will end with a 1 instead of the backing index. That's super hypothetical, so I'm also fine to ignore it.
I do feel like asserting there is only one backing index is slightly more relevant than asserting the last character, but it's not a blocker for me. What do you think?
Contributor
Author
There was a problem hiding this comment.
I do feel like asserting there is only one backing index is slightly more relevant than asserting the last character, but it's not a blocker for me. What do you think?
I only added this assertion after a rollover was performed, so we would have to assert that size is 2. I can do that. No strong feelings about it.
test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java
Outdated
Show resolved
Hide resolved
...lti-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java
Outdated
Show resolved
Hide resolved
...lti-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java
Show resolved
Hide resolved
...de/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java
Outdated
Show resolved
Hide resolved
x-pack/plugin/ilm/src/internalClusterTest/java/org/elasticsearch/xpack/ilm/ILMMultiNodeIT.java
Outdated
Show resolved
Hide resolved
.../src/internalClusterTest/java/org/elasticsearch/xpack/ilm/ILMMultiNodeWithCCRDisabledIT.java
Outdated
Show resolved
Hide resolved
Contributor
Author
|
Thank you for the review @nielsbauman , comments addressed :) |
Contributor
|
LGTM! Thanks! |
Collaborator
gmarouli
added a commit
to gmarouli/elasticsearch
that referenced
this pull request
Mar 10, 2025
…integration tests (elastic#124319) * Incorporate review comments
gmarouli
added a commit
to gmarouli/elasticsearch
that referenced
this pull request
Mar 10, 2025
…integration tests (elastic#124319) * Incorporate review comments (cherry picked from commit 44dd44b) # Conflicts: # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesDataStreamsIT.java # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java
elasticsearchmachine
pushed a commit
that referenced
this pull request
Mar 10, 2025
…integration tests (#124319) (#124467) * Incorporate review comments (cherry picked from commit 44dd44b) # Conflicts: # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesDataStreamsIT.java # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java
gmarouli
added a commit
to gmarouli/elasticsearch
that referenced
this pull request
Mar 10, 2025
…integration tests (elastic#124319) (elastic#124467) * Incorporate review comments (cherry picked from commit 44dd44b) # Conflicts: # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesDataStreamsIT.java # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java
gmarouli
added a commit
to gmarouli/elasticsearch
that referenced
this pull request
Mar 10, 2025
…integration tests (elastic#124319) (elastic#124467) * Incorporate review comments (cherry picked from commit 44dd44b) # Conflicts: # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesDataStreamsIT.java # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java (cherry picked from commit 474d223) # Conflicts: # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java
gmarouli
added a commit
to gmarouli/elasticsearch
that referenced
this pull request
Mar 10, 2025
…integration tests (elastic#124319) (elastic#124467) * Incorporate review comments (cherry picked from commit 44dd44b) # Conflicts: # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesDataStreamsIT.java # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java (cherry picked from commit 474d223) # Conflicts: # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java
elasticsearchmachine
pushed a commit
that referenced
this pull request
Mar 10, 2025
…integration tests (#124319) (#124467) (#124473) * Incorporate review comments (cherry picked from commit 44dd44b) # Conflicts: # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesDataStreamsIT.java # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java
elasticsearchmachine
pushed a commit
that referenced
this pull request
Mar 10, 2025
…integration tests (#124319) (#124467) (#124476) * Incorporate review comments (cherry picked from commit 44dd44b) # Conflicts: # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesDataStreamsIT.java # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java (cherry picked from commit 474d223) # Conflicts: # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java
georgewallace
pushed a commit
to georgewallace/elasticsearch
that referenced
this pull request
Mar 11, 2025
…integration tests (elastic#124319) * Incorporate review comments
elasticsearchmachine
pushed a commit
that referenced
this pull request
Mar 12, 2025
…integration tests (#124319) (#124467) (#124482) * Incorporate review comments (cherry picked from commit 44dd44b) # Conflicts: # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesDataStreamsIT.java # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java (cherry picked from commit 474d223) # Conflicts: # x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java
jfreden
pushed a commit
to jfreden/elasticsearch
that referenced
this pull request
Mar 13, 2025
…integration tests (elastic#124319) * Incorporate review comments
nielsbauman
added a commit
to nielsbauman/elasticsearch
that referenced
this pull request
Mar 27, 2025
This test failure was already fixed by elastic#124319 but wasn't unmuted yet.
Merged
nielsbauman
added a commit
to nielsbauman/elasticsearch
that referenced
this pull request
Mar 27, 2025
This test failure was already fixed by elastic#124319 but wasn't unmuted yet.
elasticsearchmachine
pushed a commit
that referenced
this pull request
Mar 28, 2025
omricohenn
pushed a commit
to omricohenn/elasticsearch
that referenced
this pull request
Mar 28, 2025
This test failure was already fixed by elastic#124319 but wasn't unmuted yet.
nielsbauman
added a commit
to nielsbauman/elasticsearch
that referenced
this pull request
Jun 26, 2025
This test was already fixed in elastic#124319.
Merged
nielsbauman
added a commit
that referenced
this pull request
Jun 26, 2025
mridula-s109
pushed a commit
to mridula-s109/elasticsearch
that referenced
this pull request
Jul 3, 2025
This test was already fixed in elastic#124319.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Relates to #123376.
In this PR we change the way we retrieve the name of the backing index of data streams. We replace usages of time sensitive
DataStream#getDefaultBackingIndexNamewith the retrieval of the name via an API call. The problem with using the time sensitive method is that we can have test failures around midnight.Because this is part of a larger effort #123376 we think it's a good idea to add a helper method to facilitate the retrieval of the backing index names to the base test classes.
Fixes: #117403
Fixes: #123203
Fixes: #123202
Fixes: #124439