#111893 Add Warnings For Missing Index Templates #114589
Conversation
lukewhiting
added
>bug
>docs
|
Documentation preview: |
elasticsearchmachine
added
Team:Data Management
|
Pinging @elastic/es-docs (Team:Docs) |
|
Pinging @elastic/es-data-management (Team:Data Management) |
|
es-docs people, Do you need pinging for small changes like this or should I omit the |
|
@lukewhiting you don't have to wait for our reviews for small things like these, but keeping the label is a good idea. we don't have the capacity to review every PR, but we try to keep an eye out and provide feedback when possible. this PR incidentally LGTM |
There was a problem hiding this comment.
Great work @lukewhiting , I learnt a lot reviewing this code. It's touching some parts I am unfamiliar with. I added some comments and some questions that will help me understand the code better as well.
...reams/src/internalClusterTest/java/org/elasticsearch/datastreams/DataStreamsSnapshotsIT.java
Outdated
Show resolved
Hide resolved
...ams/src/main/java/org/elasticsearch/datastreams/action/PromoteDataStreamTransportAction.java
Outdated
Show resolved
Hide resolved
| var templatePatterns = clusterService.state() | ||
| .metadata() | ||
| .templatesV2() | ||
| .values() | ||
| .stream() | ||
| .filter(cit -> cit.getDataStreamTemplate() != null) | ||
| .flatMap(cit -> cit.indexPatterns().stream()) | ||
| .collect(Collectors.toSet()); |
There was a problem hiding this comment.
If I understand this correctly, this code checks only the current cluster state for a matching template while the template could be restored as part of the snapshot. Should we consider checking the metadataBuilder or the globalMetadata, that are set here:
This should be verified, I haven't tested it, but I think it might be a valid scenario.
There was a problem hiding this comment.
I think you are right... There might be a few failed rollover attempts while the restore is in progress (if the DS gets restored before the templates) but I believe it would rollover successfully in the end so it's a valid scenario and should not warn.
I'll get that logic in and add some tests for these senario.
There was a problem hiding this comment.
Logic and tests added however the test is affected by #107515 so I have had to mute it :-/ I did some manual runs of the test with org/elasticsearch/test/ESIntegTestCase.java:1305 commented out to prevent the cluster state size mismatch failing the test and it all passed.
x-pack/plugin/ccr/qa/multi-cluster/src/test/java/org/elasticsearch/xpack/ccr/AutoFollowIT.java
Outdated
Show resolved
Hide resolved
...reams/src/internalClusterTest/java/org/elasticsearch/datastreams/DataStreamsSnapshotsIT.java
Show resolved
Hide resolved
x-pack/plugin/ccr/qa/multi-cluster/src/test/java/org/elasticsearch/xpack/ccr/AutoFollowIT.java
Outdated
Show resolved
Hide resolved
x-pack/plugin/ccr/qa/multi-cluster/src/test/java/org/elasticsearch/xpack/ccr/AutoFollowIT.java
Show resolved
Hide resolved
x-pack/plugin/ccr/qa/multi-cluster/src/test/java/org/elasticsearch/xpack/ccr/AutoFollowIT.java
Show resolved
Hide resolved
...ams/src/main/java/org/elasticsearch/datastreams/action/PromoteDataStreamTransportAction.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
LGTM, I did some minor comments, but apart from these, it's good :) Thank you @lukewhiting 🚀
lukewhiting
removed
auto-backport
to snapshot restore
to data stream promotion endpoint
Add a new assertion to synchronously execute a request and check the response contains a specific warning header
When missing templates
This checks if the snapshot contains a matching template for the DS
lukewhiting
force-pushed
the
111893-ds-restore-template-warning
branch
from
2ab424c to
5bcfb5e
Compare
* Add data stream template validation to snapshot restore * Add data stream template validation to data stream promotion endpoint * Add new assertion for response headers Add a new assertion to synchronously execute a request and check the response contains a specific warning header * Test for warning header on snapshot restore When missing templates * Test for promotion warnings * Add documentation for the potential error states * PR changes * Spotless reformatting * Add logic to look in snapshot global metadata This checks if the snapshot contains a matching template for the DS * Comment on test cleanup to explain it was copied * Removed cluster service field
* Add data stream template validation to snapshot restore * Add data stream template validation to data stream promotion endpoint * Add new assertion for response headers Add a new assertion to synchronously execute a request and check the response contains a specific warning header * Test for warning header on snapshot restore When missing templates * Test for promotion warnings * Add documentation for the potential error states * PR changes * Spotless reformatting * Add logic to look in snapshot global metadata This checks if the snapshot contains a matching template for the DS * Comment on test cleanup to explain it was copied * Removed cluster service field
4 participants
Fixes ES#111893 - Data stream restoration/promotion without a matching template
This PR adds warnings headers and log messages for the following conditions:
In both these senarios, the action will succede however future rollovers of the data stream will fail due to the missing templates. By adding a warning header and log, we make the user explicitly aware of this pending failure.
Documentation for both these actions has also been updated to warn of this scenario