Don't ignore empty index template that have no template definition. #98840
Conversation
A composable index template with no template defined in the body is mistakingly always assumed to not be a time series template. Even if it refers to a component template that has the `index.mode` setting set to `time_series` and the component template defines mappings with dimension fields or routing paths. Closes elastic#98834
martijnvg
added
>bug
:StorageEngine/TSDB
|
Pinging @elastic/es-analytics-geo (Team:Analytics) |
elasticsearchmachine
added
the
Team:Analytics
|
Hi @martijnvg, I've created a changelog YAML for you. |
| "dynamic_templates": [ | ||
| { | ||
| "labels": { | ||
| "path_match": "pod.labels.*", |
There was a problem hiding this comment.
is this matching something?
There was a problem hiding this comment.
I just copied the template part from the index template at the top of this source file.
I think it is based on mapping used in tsdb Rally track.
There was a problem hiding this comment.
I will remove the dynamic mapping here. No data gets actually indexed in this test.
| } | ||
| // Settings in composable index template: | ||
| { | ||
| var componentTemplate = new ComponentTemplate(new Template(null, null, null), null, null); |
There was a problem hiding this comment.
From the code it looks to me the template (as a whole object) is expected to be not null even if it is not enforced with something like Objects.requireNotNull(...), for instance in the ComponentTemplate constructor, while a template with all null fields is allowed. Should we require that the template in ComponentTemplate is not null using an Objects.requireNotNull(...)?
There was a problem hiding this comment.
Should we require that the template in ComponentTemplate is not null using an Objects.requireNotNull(...)?
👍
There was a problem hiding this comment.
I will change this in another PR. It could require changes in other places as well.
|
I left two comments but I think the rest is ok...we just prevent early exit if template is null. |
martijnvg
added
the
auto-backport
|
Unrelated failure: #98863 |
|
@elasticmachine run elasticsearch-ci/part-2 |
…te used by other yaml test.
💚 Backport successful
|
…lastic#98840) A composable index template with no template defined in the body is mistakingly always assumed to not be a time series template. Even if it refers to a component template that has the index.mode setting set to time_series and the component template defines mappings with dimension fields or routing paths. Closes elastic#98834
…lastic#98840) A composable index template with no template defined in the body is mistakingly always assumed to not be a time series template. Even if it refers to a component template that has the index.mode setting set to time_series and the component template defines mappings with dimension fields or routing paths. Closes elastic#98834
## Summary Fixes #163971 finally. This PR keep tracks of ES fix for the TSDB downsampling and will be ready to restore the tests once the bug has been fixed. The issue has been fixed with elastic/elasticsearch#98840 on ES side. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
## Summary Fixes elastic#163971 finally. This PR keep tracks of ES fix for the TSDB downsampling and will be ready to restore the tests once the bug has been fixed. The issue has been fixed with elastic/elasticsearch#98840 on ES side. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios (cherry picked from commit 3aa9d7b)
A composable index template with no template defined in the body is mistakingly always assumed to not be a time series template. Even if it refers to a component template that has the
index.modesetting set totime_seriesand the component template defines mappings with dimension fields or routing paths.Closes #98834