Skip to content

Add explicit generation member to data streams#55342

Merged
danhermann merged 4 commits intoelastic:masterfrom
danhermann:data_stream_generations
Apr 17, 2020
Merged

Add explicit generation member to data streams#55342
danhermann merged 4 commits intoelastic:masterfrom
danhermann:data_stream_generations

Conversation

@danhermann
Copy link
Copy Markdown
Contributor

Adds an explicit generation attribute to data streams. The current write index is derived from the generation and the data stream validates that it contains a backing index corresponding to its generation.

Also DRYs up the logic for naming backing indices for data streams.

Related to #53100

@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-core-features (:Core/Features/Data streams)

Copy link
Copy Markdown
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@henningandersen henningandersen left a comment

Choose a reason for hiding this comment

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

I added a comment, I think the list of indices in the stream should have a defined order, making the write index always the last one?

@@ -1384,7 +1384,7 @@ private SortedMap<String, IndexAbstraction> buildIndicesLookup() {

IndexMetadata writeIndex = backingIndices.get(backingIndices.size() - 1);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wonder if we should simply keep this (and the IndexAbstraction.DataStream constructor) as it was, except to add an assertion that the generation we have should match the name of the last index in the list. I think we want that to hold true anyway?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I agree that the backing index list should have a defined order and that we can simplify the validation that the generation matches the last name in the list.

If you're also suggesting that we keep the separate writeIndex parameter on the IndexAbstraction.DataStream constructor, I don't see why that would continue to be necessary since the value for the writeIndex parameter would always be backingIndices.get(backingIndices.size() - 1)?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah yes, you are right, we do not need to keep that.

@danhermann
Copy link
Copy Markdown
Contributor Author

@elasticmachine run elasticsearch-ci/1

@danhermann danhermann merged commit e173045 into elastic:master Apr 17, 2020
@danhermann danhermann deleted the data_stream_generations branch April 17, 2020 14:02
danhermann added a commit to danhermann/elasticsearch that referenced this pull request Apr 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants