Skip to content

Commit bd24685

Browse files
Fix deletion failure/error of unused index template when the index template matches a data stream but has lower priority (#20102)
* Fix delete not using index template failed when the index template matches a data stream but has low priority Signed-off-by: Binlong Gao <[email protected]> * Modify change log Signed-off-by: Binlong Gao <[email protected]> * Optimize change log Signed-off-by: Binlong Gao <[email protected]> --------- Signed-off-by: Binlong Gao <[email protected]> Signed-off-by: Sandesh Kumar <[email protected]> Co-authored-by: Sandesh Kumar <[email protected]>
1 parent 3734978 commit bd24685

File tree

4 files changed

+129
-22
lines changed

4 files changed

+129
-22
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
8989
- Fix ClassCastException in FlightClientChannel for requests larger than 16KB ([#20010](https://github.com/opensearch-project/OpenSearch/pull/20010))
9090
- Fix GRPC Bulk ([#19937](https://github.com/opensearch-project/OpenSearch/pull/19937))
9191
- Fix node bootstrap error when enable stream transport and remote cluster state ([#19948](https://github.com/opensearch-project/OpenSearch/pull/19948))
92+
- Fix deletion failure/error of unused index template; case when an index template matches a data stream but has a lower priority. ([#20102](https://github.com/opensearch-project/OpenSearch/pull/20102))
9293
- Fix toBuilder method in EngineConfig to include mergedSegmentTransferTracker([20105](https://github.com/opensearch-project/OpenSearch/pull/20105))
9394

9495
### Dependencies

rest-api-spec/src/main/resources/rest-api-spec/test/indices.delete_index_template/10_basic.yml

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,30 @@ setup:
2828
number_of_replicas: 0
2929
"priority": 51
3030

31+
- do:
32+
allowed_warnings:
33+
- "index template [test_template_3] has index patterns [test-*] matching patterns from existing older templates [global] with patterns (global => [*]); this template [test_template_3] will take precedence during new index creation"
34+
indices.put_index_template:
35+
name: test_template_3
36+
body:
37+
index_patterns: test-*
38+
data_stream: {}
39+
template:
40+
settings:
41+
number_of_shards: 1
42+
number_of_replicas: 0
43+
"priority": 49
44+
3145
---
3246
teardown:
3347
- do:
3448
indices.delete_data_stream:
3549
name: test-1
3650
ignore: 404
51+
- do:
52+
indices.delete_data_stream:
53+
name: test-2
54+
ignore: 404
3755
- do:
3856
indices.delete_index_template:
3957
name: test_template_1
@@ -42,6 +60,10 @@ teardown:
4260
indices.delete_index_template:
4361
name: test_template_2
4462
ignore: 404
63+
- do:
64+
indices.delete_index_template:
65+
name: test_template_3
66+
ignore: 404
4567

4668
---
4769
"Delete index template which is not used by data stream but index pattern matches":
@@ -56,3 +78,18 @@ teardown:
5678
- do:
5779
indices.delete_index_template:
5880
name: test_template_1
81+
82+
# issue: https://github.com/opensearch-project/OpenSearch/issues/20078
83+
---
84+
"Delete not using index template for data stream but index pattern matches":
85+
- skip:
86+
version: " - 3.3.99"
87+
reason: "fixed in 3.4.0"
88+
89+
- do:
90+
indices.create_data_stream:
91+
name: test-2
92+
93+
- do:
94+
indices.delete_index_template:
95+
name: test_template_3

server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -957,12 +957,13 @@ static Set<String> dataStreamsUsingTemplate(final ClusterState state, final Stri
957957
}
958958
final Set<String> dataStreams = state.metadata().dataStreams().keySet();
959959
Set<String> matches = new HashSet<>();
960-
template.indexPatterns()
961-
.forEach(
962-
indexPattern -> matches.addAll(
963-
dataStreams.stream().filter(stream -> Regex.simpleMatch(indexPattern, stream)).collect(Collectors.toList())
964-
)
965-
);
960+
template.indexPatterns().forEach(indexPattern -> {
961+
dataStreams.stream().filter(stream -> Regex.simpleMatch(indexPattern, stream)).filter(stream -> {
962+
// Check if this template is the highest priority template and used by this data stream
963+
final String usingTemplate = findV2Template(state.metadata(), stream, false);
964+
return templateName.equals(usingTemplate);
965+
}).forEach(matches::add);
966+
});
966967
return matches;
967968
}
968969

@@ -1156,36 +1157,38 @@ public static List<IndexTemplateMetadata> findV1Templates(Metadata metadata, Str
11561157
@Nullable
11571158
public static String findV2Template(Metadata metadata, String indexName, boolean isHidden) {
11581159
final Predicate<String> patternMatchPredicate = pattern -> Regex.simpleMatch(pattern, indexName);
1159-
final Map<ComposableIndexTemplate, String> matchedTemplates = new HashMap<>();
1160+
ComposableIndexTemplate winner = null;
1161+
String winnerName = null;
1162+
long highestPriority = -1;
1163+
11601164
for (Map.Entry<String, ComposableIndexTemplate> entry : metadata.templatesV2().entrySet()) {
11611165
final String name = entry.getKey();
11621166
final ComposableIndexTemplate template = entry.getValue();
1167+
boolean matched = false;
1168+
11631169
if (isHidden == false) {
1164-
final boolean matched = template.indexPatterns().stream().anyMatch(patternMatchPredicate);
1165-
if (matched) {
1166-
matchedTemplates.put(template, name);
1167-
}
1170+
matched = template.indexPatterns().stream().anyMatch(patternMatchPredicate);
11681171
} else {
11691172
final boolean isNotMatchAllTemplate = template.indexPatterns().stream().noneMatch(Regex::isMatchAllPattern);
11701173
if (isNotMatchAllTemplate) {
1171-
if (template.indexPatterns().stream().anyMatch(patternMatchPredicate)) {
1172-
matchedTemplates.put(template, name);
1173-
}
1174+
matched = template.indexPatterns().stream().anyMatch(patternMatchPredicate);
1175+
}
1176+
}
1177+
1178+
if (matched) {
1179+
long priority = template.priorityOrZero();
1180+
if (winner == null || priority > highestPriority) {
1181+
winner = template;
1182+
winnerName = name;
1183+
highestPriority = priority;
11741184
}
11751185
}
11761186
}
11771187

1178-
if (matchedTemplates.size() == 0) {
1188+
if (winner == null) {
11791189
return null;
11801190
}
11811191

1182-
final List<ComposableIndexTemplate> candidates = new ArrayList<>(matchedTemplates.keySet());
1183-
CollectionUtil.timSort(candidates, Comparator.comparing(ComposableIndexTemplate::priorityOrZero, Comparator.reverseOrder()));
1184-
1185-
assert candidates.size() > 0 : "we should have returned early with no candidates";
1186-
ComposableIndexTemplate winner = candidates.get(0);
1187-
String winnerName = matchedTemplates.get(winner);
1188-
11891192
// if the winner template is a global template that specifies the `index.hidden` setting (which is not allowed, so it'd be due to
11901193
// a restored index cluster state that modified a component template used by this global template such that it has this setting)
11911194
// we will fail and the user will have to update the index template and remove this setting or update the corresponding component

server/src/test/java/org/opensearch/cluster/metadata/MetadataIndexTemplateServiceTests.java

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,72 @@ public void testRemoveIndexTemplateV2() throws Exception {
625625
assertNull(updatedState.metadata().templatesV2().get("foo"));
626626
}
627627

628+
public void testRemoveIndexTemplateV2ForDataStream() throws Exception {
629+
final MetadataIndexTemplateService service = getMetadataIndexTemplateService();
630+
ClusterState state = ClusterState.EMPTY_STATE;
631+
632+
// Create two templates with different priorities that match the same data stream
633+
ComposableIndexTemplate lowPriorityTemplate = new ComposableIndexTemplate(
634+
List.of("logs-*"),
635+
null,
636+
null,
637+
10L,
638+
1L,
639+
null,
640+
new ComposableIndexTemplate.DataStreamTemplate()
641+
);
642+
643+
ComposableIndexTemplate highPriorityTemplate = new ComposableIndexTemplate(
644+
List.of("logs-*"),
645+
null,
646+
null,
647+
20L,
648+
1L,
649+
null,
650+
new ComposableIndexTemplate.DataStreamTemplate()
651+
);
652+
653+
state = service.addIndexTemplateV2(state, true, "low-priority-template", lowPriorityTemplate);
654+
state = service.addIndexTemplateV2(state, true, "high-priority-template", highPriorityTemplate);
655+
656+
// Add a data stream
657+
state = ClusterState.builder(state)
658+
.metadata(
659+
Metadata.builder(state.metadata())
660+
.put(
661+
new DataStream(
662+
"logs-mysql",
663+
new DataStream.TimestampField("@timestamp"),
664+
Collections.singletonList(new Index(".ds-logs-mysql-000001", "uuid"))
665+
)
666+
)
667+
.put(
668+
IndexMetadata.builder(".ds-logs-mysql-000001")
669+
.settings(
670+
Settings.builder()
671+
.put(IndexMetadata.SETTING_INDEX_UUID, "uuid")
672+
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
673+
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
674+
.put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
675+
.build()
676+
)
677+
)
678+
.build()
679+
)
680+
.build();
681+
682+
// Test that only the high priority template reports the data stream as using it
683+
Set<String> lowPriorityDataStreams = MetadataIndexTemplateService.dataStreamsUsingTemplate(state, "low-priority-template");
684+
Set<String> highPriorityDataStreams = MetadataIndexTemplateService.dataStreamsUsingTemplate(state, "high-priority-template");
685+
686+
assertThat(lowPriorityDataStreams, empty());
687+
assertThat(highPriorityDataStreams, contains("logs-mysql"));
688+
689+
// Test remove the low priority template
690+
ClusterState updatedState = MetadataIndexTemplateService.innerRemoveIndexTemplateV2(state, "low-priority-template");
691+
assertNull(updatedState.metadata().templatesV2().get("low-priority-template"));
692+
}
693+
628694
/**
629695
* Test that if we have a pre-existing v1 template and put a v2 template that would match the same indices, we generate a warning
630696
*/

0 commit comments

Comments
 (0)