Skip to content
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

Preserve backing index ordering for data streams #63749

Merged
merged 2 commits into from
Oct 16, 2020

Conversation

danhermann
Copy link
Contributor

The resolve index API incorrectly sorts the backing indices returned for data streams. The backing indices on a data stream are an ordered list in which the last index is always the write index so sorting them may produce incorrect results once migration of existing indices to data streams is permitted.

@danhermann danhermann added >bug :Data Management/Indices APIs APIs to create and manage indices and templates v8.0.0 v7.11.0 labels Oct 15, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (:Core/Features/Indices APIs)

@elasticmachine elasticmachine added the Team:Data Management Meta label for data/management team label Oct 15, 2020
Copy link
Contributor

@probakowski probakowski left a comment

Choose a reason for hiding this comment

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

LGTM (assuming CI is green), thanks Dan!
I left suggestion for refactoring in test but it can be ignored.

Comment on lines 176 to 178
backingIndices.add(createIndexMetadata("not-in-order-2", true));
backingIndices.add(createIndexMetadata("not-in-order-1", true));
backingIndices.add(createIndexMetadata(DataStream.getDefaultBackingIndexName(dataStreamName, 3), true));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
backingIndices.add(createIndexMetadata("not-in-order-2", true));
backingIndices.add(createIndexMetadata("not-in-order-1", true));
backingIndices.add(createIndexMetadata(DataStream.getDefaultBackingIndexName(dataStreamName, 3), true));
String[] names = new String[]{"not-in-order-2", "not-in-order-1", DataStream.getDefaultBackingIndexName(dataStreamName, 3)};
List<IndexMetadata> backingIndices = Arrays.stream(names).map(n -> createIndexMetadata(n, true)).collect(Collectors.toList());

Comment on lines 194 to 197
assertThat(dataStreams.get(0).getBackingIndices().length, equalTo(3));
assertThat(dataStreams.get(0).getBackingIndices()[0], equalTo(backingIndices.get(0).getIndex().getName()));
assertThat(dataStreams.get(0).getBackingIndices()[1], equalTo(backingIndices.get(1).getIndex().getName()));
assertThat(dataStreams.get(0).getBackingIndices()[2], equalTo(backingIndices.get(2).getIndex().getName()));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
assertThat(dataStreams.get(0).getBackingIndices().length, equalTo(3));
assertThat(dataStreams.get(0).getBackingIndices()[0], equalTo(backingIndices.get(0).getIndex().getName()));
assertThat(dataStreams.get(0).getBackingIndices()[1], equalTo(backingIndices.get(1).getIndex().getName()));
assertThat(dataStreams.get(0).getBackingIndices()[2], equalTo(backingIndices.get(2).getIndex().getName()));
assertThat(dataStreams.get(0).getBackingIndices(), arrayContaining(names));

@danhermann
Copy link
Contributor Author

Thanks, @probakowski! I took your suggestions for simplifying the test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Data Management/Indices APIs APIs to create and manage indices and templates Team:Data Management Meta label for data/management team v7.11.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants