-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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
Migrate aliased indices to data stream #61525
Migrate aliased indices to data stream #61525
Conversation
@elasticmachine update branch |
merge conflict between base and head |
@elasticmachine update branch |
@elasticmachine update branch |
@elasticmachine update branch |
A side effect is moving logic that prepares indices to be backing indices to migrate service. * By using the mapper service directly in the migrate logic, mapping validation error occur before updating the cluster state. Whereas before the validation errors occurred when all mapper services in other nodes where updated in the cluster, which is too late, because the update already happened. * Updating the mapping using maps somehow changed the ordering, which caused assertions to trip.
@elasticmachine update branch |
@elasticmachine run elasticsearch-ci/2 |
@martijnvg, I'm marking this as ready for review. I've moved the test code around to accommodate the new location of the validation logic. Let me know if I missed something you were expecting. |
Pinging @elastic/es-core-features (:Core/Features/Data streams) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left two comments, but other than that, this change is looking good!
Metadata.Builder builder = Metadata.builder(currentState.metadata()).put(newDataStream); | ||
logger.info("adding data stream [{}]", request.name); | ||
logger.info("adding data stream [{}]", dataStreamName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe add what the write index is and backing indices (if any) to this log message?
@@ -277,17 +277,17 @@ public void testValidateNotDisallowedAttribute() throws IOException { | |||
assertThat(e.getMessage(), equalTo("data stream timestamp field [@timestamp] has disallowed attributes: [store]")); | |||
} | |||
|
|||
public void testCannotUpdateTimestampField() throws IOException { | |||
public void testCanUpdateTimestampField() throws IOException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to adjust test and the data stream timestamp field mapper.
Updating from false
to true
is allowed, but updating from true
to false
is disallowed.
`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This validation can be added by invoking setMergeValidator(...)
method when building enabled parameter in DataStreamTimestampFieldMapper
line 78.
Thanks, @martijnvg. I've made those changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks, @martijnvg! |
Updates the
MetadataCreateDataStreamService
to permit existing indices as backing indices when creating a new data stream.Relates to #61046