Remove usages of DataStream#getDefaultBackingIndexName#129466
Merged
nielsbauman merged 3 commits intoelastic:mainfrom Jun 17, 2025
Merged
Remove usages of DataStream#getDefaultBackingIndexName#129466nielsbauman merged 3 commits intoelastic:mainfrom
DataStream#getDefaultBackingIndexName#129466nielsbauman merged 3 commits intoelastic:mainfrom
Conversation
These usages had the potential of causing test failures when a data stream was created before midnight and the backing index name generation ran the next day - which would be millisecconds apart. To avoid these failures, we update the tests to be robust to these time differences. Resolves elastic#123376
Collaborator
|
Pinging @elastic/es-data-management (Team:Data Management) |
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR updates test code to remove direct usages of DataStream#getDefaultBackingIndexName in favor of more robust index name resolution methods, reducing the risk of test failures caused by time differences. Key changes include replacing direct calls with projectMetadata.dataStreams() lookups, using DataStreamTestHelper for assertions, and passing explicit time parameters where needed.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolverTests.java | Replaced default backing index name call with data stream lookup |
| x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/TransportPutFollowActionTests.java | Switched to DataStreamTestHelper assertions for backing index names |
| server/src/test/java/org/elasticsearch/cluster/metadata/MetadataTests.java | Updated error messages to use index name from Index object |
| server/src/test/java/org/elasticsearch/cluster/metadata/MetadataDeleteIndexServiceTests.java | Updated index deletion logic with decreased index lookup via data stream |
| server/src/test/java/org/elasticsearch/cluster/metadata/MetadataCreateDataStreamServiceTests.java | Replaced usages of getDefaultBackingIndexName with write index lookup |
| server/src/test/java/org/elasticsearch/cluster/metadata/DataStreamTests.java | Updated index lookups to use getIndices() calls |
| server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTests.java | Updated test assertions with backing index objects |
| server/src/test/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java | Injected explicit time parameter to backing index name resolution |
| server/src/test/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverServiceAutoShardingTests.java | Updated source & rollover index name assertions |
| modules/data-streams/src/test/java/org/elasticsearch/datastreams/lifecycle/downsampling/DeleteSourceAndAddDownsampleToDSTests.java | Swapped legacy index name calls with data stream index lookups |
| modules/data-streams/src/test/java/org/elasticsearch/datastreams/lifecycle/DataStreamLifecycleServiceTests.java | Replaced legacy index name resolution with DataStreamTestHelper comparisons |
| modules/data-streams/src/test/java/org/elasticsearch/datastreams/MetadataDataStreamRolloverServiceTests.java | Updated rollover tests with explicit time providers and time parameters |
server/src/test/java/org/elasticsearch/cluster/metadata/MetadataDeleteIndexServiceTests.java
Outdated
Show resolved
Hide resolved
…taDeleteIndexServiceTests.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
lukewhiting
approved these changes
Jun 17, 2025
Contributor
lukewhiting
left a comment
There was a problem hiding this comment.
One question more than comment but otherwise looks good 👍🏻
| dataStreamName, | ||
| List.of(new Index(DataStream.getDefaultBackingIndexName(dataStreamName, 1, now.toEpochMilli()), "uuid")) | ||
| ).setIndexMode(IndexMode.TIME_SERIES).build(); | ||
| ).setIndexMode(IndexMode.TIME_SERIES).setTimeProvider(now::toEpochMilli).build(); |
Contributor
There was a problem hiding this comment.
Bit of a nitpick not specifically about this PR but I'm interested in why we went for this TimeProvider pattern Vs using the Clock interface?
Contributor
Author
There was a problem hiding this comment.
I'm afraid I have no idea... This time provider predates my time at Elastic
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
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.
These usages had the potential of causing test failures when a data stream was created before midnight and the backing index name generation ran the next day - which would be millisecconds apart. To avoid these failures, we update the tests to be robust to these time differences.
Resolves #123376