Skip to content

Commit

Permalink
Merge branch 'main' into releasable_block
Browse files Browse the repository at this point in the history
  • Loading branch information
ChrisHegarty committed Sep 13, 2023
2 parents f26b195 + af89554 commit 06064d3
Show file tree
Hide file tree
Showing 24 changed files with 1,697 additions and 135 deletions.
6 changes: 6 additions & 0 deletions docs/changelog/97317.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 97317
summary: "Fix merges of mappings with `subobjects: false` for composable index templates"
area: Mapping
type: bug
issues:
- 96768
6 changes: 6 additions & 0 deletions docs/changelog/98244.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 98244
summary: Optimize ContentPath#pathAsText
area: Search
type: enhancement
issues:
- 94544
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ public void testTimeStampValidationInvalidFieldMapping() throws Exception {
{
"properties": {
"@timestamp": {
"type": "keyword"
"type": "long"
}
}
}""";
Expand All @@ -538,7 +538,7 @@ public void testTimeStampValidationInvalidFieldMapping() throws Exception {
);
assertThat(
e.getCause().getCause().getMessage(),
equalTo("data stream timestamp field [@timestamp] is of type [keyword], but [date,date_nanos] is expected")
equalTo("data stream timestamp field [@timestamp] is of type [long], but [date,date_nanos] is expected")
);
}

Expand Down Expand Up @@ -1988,18 +1988,13 @@ public void testPartitionedTemplate() throws IOException {
).actionGet();

/**
* routing enable with allow custom routing false
* routing settings with allow custom routing false
*/
template = new ComposableIndexTemplate(
List.of("logs"),
new Template(
Settings.builder().put("index.number_of_shards", "3").put("index.routing_partition_size", "2").build(),
new CompressedXContent("""
{
"_routing": {
"required": true
}
}"""),
null,
null
),
null,
Expand All @@ -2024,6 +2019,36 @@ public void testPartitionedTemplate() throws IOException {
);
}

public void testRoutingEnabledInMappingDisabledInDataStreamTemplate() throws IOException {
ComposableIndexTemplate template = new ComposableIndexTemplate(
List.of("logs"),
new Template(
Settings.builder().put("index.number_of_shards", "3").put("index.routing_partition_size", "2").build(),
new CompressedXContent("""
{
"_routing": {
"required": true
}
}"""),
null
),
null,
null,
null,
null,
new ComposableIndexTemplate.DataStreamTemplate(false, false)
);
Exception e = expectThrows(
IllegalArgumentException.class,
() -> client().execute(
PutComposableIndexTemplateAction.INSTANCE,
new PutComposableIndexTemplateAction.Request("my-it").indexTemplate(template)
).actionGet()
);
Exception actualException = (Exception) e.getCause();
assertTrue(Throwables.getRootCause(actualException).getMessage().contains("contradicting `_routing.required` settings"));
}

public void testSearchWithRouting() throws IOException, ExecutionException, InterruptedException {
/**
* partition size with routing required
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,7 @@ private List<String> findRoutingPaths(String indexName, Settings allSettings, Li
tmpIndexMetadata.settings(finalResolvedSettings);
// Create MapperService just to extract keyword dimension fields:
try (var mapperService = mapperServiceFactory.apply(tmpIndexMetadata.build())) {
for (var mapping : combinedTemplateMappings) {
mapperService.merge(MapperService.SINGLE_MAPPING_NAME, mapping, MapperService.MergeReason.INDEX_TEMPLATE);
}

mapperService.merge(MapperService.SINGLE_MAPPING_NAME, combinedTemplateMappings, MapperService.MergeReason.INDEX_TEMPLATE);
List<String> routingPaths = new ArrayList<>();
for (var fieldMapper : mapperService.documentMapper().mappers().fieldMappers()) {
extractPath(routingPaths, fieldMapper);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -349,3 +349,146 @@
index_patterns: ["purple-index"]
composed_of: ["red", "blue"]
ignore_missing_component_templates: ["foo"]

---
"Composable index templates that include subobjects: false at root":
- skip:
version: ' - 8.10.99'
reason: 'https://github.com/elastic/elasticsearch/issues/96768 fixed at 8.11.0'
features: allowed_warnings

- do:
cluster.put_component_template:
name: test-subobjects
body:
template:
mappings:
subobjects: false

- do:
cluster.put_component_template:
name: test-field
body:
template:
mappings:
properties:
parent.subfield:
type: keyword

- do:
allowed_warnings:
- "index template [test-composable-template] has index patterns [test-*] matching patterns from existing older templates [global] with patterns (global => [*]); this template [test-composable-template] will take precedence during new index creation"
indices.put_index_template:
name: test-composable-template
body:
index_patterns:
- test-*
composed_of:
- test-subobjects
- test-field
- is_true: acknowledged

- do:
indices.create:
index: test-generic

- do:
indices.get_mapping:
index: test-generic
- match: { test-generic.mappings.properties.parent\.subfield.type: "keyword" }

---
"Composable index templates that include subobjects: false on arbitrary field":
- skip:
version: ' - 8.10.99'
reason: 'https://github.com/elastic/elasticsearch/issues/96768 fixed at 8.11.0'
features: allowed_warnings

- do:
cluster.put_component_template:
name: test-subobjects
body:
template:
mappings:
properties:
parent:
type: object
subobjects: false

- do:
cluster.put_component_template:
name: test-subfield
body:
template:
mappings:
properties:
parent:
properties:
child.grandchild:
type: keyword

- do:
allowed_warnings:
- "index template [test-composable-template] has index patterns [test-*] matching patterns from existing older templates [global] with patterns (global => [*]); this template [test-composable-template] will take precedence during new index creation"
indices.put_index_template:
name: test-composable-template
body:
index_patterns:
- test-*
composed_of:
- test-subobjects
- test-subfield
- is_true: acknowledged

- do:
indices.create:
index: test-generic

- do:
indices.get_mapping:
index: test-generic
- match: { test-generic.mappings.properties.parent.properties.child\.grandchild.type: "keyword" }

---
"Composition of component templates with different legal field mappings":
- skip:
features: allowed_warnings

- do:
cluster.put_component_template:
name: mapping
body:
template:
mappings:
properties:
field:
type: long
coerce: true

- do:
allowed_warnings:
- "index template [test-composable-template] has index patterns [test-*] matching patterns from existing older templates [global] with patterns (global => [*]); this template [test-composable-template] will take precedence during new index creation"
indices.put_index_template:
name: test-composable-template
body:
index_patterns:
- test-*
composed_of:
- mapping
template:
mappings:
properties:
field:
type: keyword
ignore_above: 1024
- is_true: acknowledged

- do:
indices.create:
index: test-generic

- do:
indices.get_mapping:
index: test-generic
- match: { test-generic.mappings.properties.field.type: "keyword" }
- match: { test-generic.mappings.properties.field.ignore_above: 1024 }
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ reject @timestamp with wrong type:
reason: introduced in 8.1.0

- do:
catch: /data stream timestamp field \[@timestamp\] is of type \[keyword\], but \[date,date_nanos\] is expected/
catch: /data stream timestamp field \[@timestamp\] is of type \[long\], but \[date,date_nanos\] is expected/
indices.create:
index: test
body:
Expand All @@ -163,7 +163,7 @@ reject @timestamp with wrong type:
mappings:
properties:
"@timestamp":
type: keyword
type: long

---
reject timestamp meta field with wrong type:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -294,9 +294,7 @@ public static Template resolveTemplate(
indexMetadata,
tempIndexService -> {
MapperService mapperService = tempIndexService.mapperService();
for (CompressedXContent mapping : mappings) {
mapperService.merge(MapperService.SINGLE_MAPPING_NAME, mapping, MapperService.MergeReason.INDEX_TEMPLATE);
}
mapperService.merge(MapperService.SINGLE_MAPPING_NAME, mappings, MapperService.MergeReason.INDEX_TEMPLATE);

DocumentMapper documentMapper = mapperService.documentMapper();
return documentMapper != null ? documentMapper.mappingSource() : null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1331,13 +1331,14 @@ private static void updateIndexMappingsAndBuildSortOrder(
) throws IOException {
MapperService mapperService = indexService.mapperService();
IndexMode indexMode = indexService.getIndexSettings() != null ? indexService.getIndexSettings().getMode() : IndexMode.STANDARD;
List<CompressedXContent> allMappings = new ArrayList<>();
final CompressedXContent defaultMapping = indexMode.getDefaultMapping();
if (defaultMapping != null) {
mapperService.merge(MapperService.SINGLE_MAPPING_NAME, defaultMapping, MergeReason.INDEX_TEMPLATE);
}
for (CompressedXContent mapping : mappings) {
mapperService.merge(MapperService.SINGLE_MAPPING_NAME, mapping, MergeReason.INDEX_TEMPLATE);
allMappings.add(defaultMapping);
}
allMappings.addAll(mappings);
mapperService.merge(MapperService.SINGLE_MAPPING_NAME, allMappings, MergeReason.INDEX_TEMPLATE);

indexMode.validateTimestampFieldMapping(request.dataStreamName() != null, mapperService.mappingLookup());

if (sourceMetadata == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@
public class MetadataIndexTemplateService {

public static final String DEFAULT_TIMESTAMP_FIELD = "@timestamp";
public static final CompressedXContent DEFAULT_TIMESTAMP_MAPPING;
public static final CompressedXContent DEFAULT_TIMESTAMP_MAPPING_WITHOUT_ROUTING;

private static final CompressedXContent DEFAULT_TIMESTAMP_MAPPING_WITH_ROUTING;

Expand All @@ -96,8 +96,14 @@ public class MetadataIndexTemplateService {
Map.of("type", DateFieldMapper.CONTENT_TYPE, "ignore_malformed", "false")
);
try {
DEFAULT_TIMESTAMP_MAPPING = new CompressedXContent(
DEFAULT_TIMESTAMP_MAPPING_WITHOUT_ROUTING = new CompressedXContent(
(builder, params) -> builder.startObject(MapperService.SINGLE_MAPPING_NAME)
// adding explicit "_routing": {"required": false}, even though this is the default, because this snippet is used
// later for resolving a RoutingFieldMapper, where we need this information to validate that does not conflict with
// any mapping.
.startObject(RoutingFieldMapper.NAME)
.field("required", false)
.endObject()
.field("properties", defaultTimestampField)
.endObject()
);
Expand Down Expand Up @@ -1303,7 +1309,7 @@ public static List<CompressedXContent> collectMappings(
if (template.getDataStreamTemplate().isAllowCustomRouting()) {
mappings.add(0, DEFAULT_TIMESTAMP_MAPPING_WITH_ROUTING);
} else {
mappings.add(0, DEFAULT_TIMESTAMP_MAPPING);
mappings.add(0, DEFAULT_TIMESTAMP_MAPPING_WITHOUT_ROUTING);
}
}

Expand Down Expand Up @@ -1599,9 +1605,7 @@ private static void validateCompositeTemplate(
List<CompressedXContent> mappings = collectMappings(stateWithIndex, templateName, indexName);
try {
MapperService mapperService = tempIndexService.mapperService();
for (CompressedXContent mapping : mappings) {
mapperService.merge(MapperService.SINGLE_MAPPING_NAME, mapping, MapperService.MergeReason.INDEX_TEMPLATE);
}
mapperService.merge(MapperService.SINGLE_MAPPING_NAME, mappings, MapperService.MergeReason.INDEX_TEMPLATE);

if (template.getDataStreamTemplate() != null) {
validateTimestampFieldMapping(mapperService.mappingLookup());
Expand Down
Loading

0 comments on commit 06064d3

Please sign in to comment.