Skip to content
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.

Sign up for GitHub

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

Small refactor: use bulk mapping list merges instead of sequential #1

Merged
merged 2 commits into from
Aug 13, 2023
Merged

Small refactor: use bulk mapping list merges instead of sequential #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b45863e
Small refactor: switch from sequential to bulk component templates merge
eyalkoren Jul 31, 2023
849e624
Merge remote-tracking branch 'eyalkoren/fix2-subobjects-composable-te…
eyalkoren Aug 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions ...-streams/src/main/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,10 +158,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
4 changes: 1 addition & 3 deletions ...lasticsearch/action/admin/indices/template/post/TransportSimulateIndexTemplateAction.java
View file
Open in desktop
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
14 changes: 7 additions & 7 deletions server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ public void testDynamicTemplates() throws Exception {
}

public void testDynamicTemplatesForIndexTemplate() throws IOException {
String mapping = Strings.toString(
String mapping1 = Strings.toString(
XContentFactory.jsonBuilder()
.startObject()
.startArray("dynamic_templates")
Expand All @@ -333,11 +333,9 @@ public void testDynamicTemplatesForIndexTemplate() throws IOException {
.endArray()
.endObject()
);
MapperService mapperService = createMapperService(IndexVersion.current(), Settings.EMPTY, () -> true);
mapperService.merge(MapperService.SINGLE_MAPPING_NAME, new CompressedXContent(mapping), MapperService.MergeReason.INDEX_TEMPLATE);

// There should be no update if templates are not set.
mapping = Strings.toString(
String mapping2 = Strings.toString(
XContentFactory.jsonBuilder()
.startObject()
.startObject("properties")
Expand All @@ -347,9 +345,11 @@ public void testDynamicTemplatesForIndexTemplate() throws IOException {
.endObject()
.endObject()
);

MapperService mapperService = createMapperService(IndexVersion.current(), Settings.EMPTY, () -> true);
DocumentMapper mapper = mapperService.merge(
MapperService.SINGLE_MAPPING_NAME,
new CompressedXContent(mapping),
List.of(new CompressedXContent(mapping1), new CompressedXContent(mapping2)),
MapperService.MergeReason.INDEX_TEMPLATE
);

Expand All @@ -361,7 +361,7 @@ public void testDynamicTemplatesForIndexTemplate() throws IOException {
assertEquals("second", templates[1].pathMatch().get(0));

// Dynamic templates should be appended and deduplicated.
mapping = Strings.toString(
String mapping3 = Strings.toString(
XContentFactory.jsonBuilder()
.startObject()
.startArray("dynamic_templates")
Expand All @@ -386,7 +386,7 @@ public void testDynamicTemplatesForIndexTemplate() throws IOException {
);
mapper = mapperService.merge(
MapperService.SINGLE_MAPPING_NAME,
new CompressedXContent(mapping),
new CompressedXContent(mapping3),
MapperService.MergeReason.INDEX_TEMPLATE
);

Expand Down
11 changes: 7 additions & 4 deletions server/src/test/java/org/elasticsearch/index/mapper/ObjectMapperTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.elasticsearch.xcontent.XContentType;

import java.io.IOException;
import java.util.List;

import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.instanceOf;
Expand Down Expand Up @@ -199,7 +200,6 @@ public void testFieldReplacementForIndexTemplates() throws IOException {
.endObject()
.endObject()
);
mapperService.merge(MapperService.SINGLE_MAPPING_NAME, new CompressedXContent(mapping), MergeReason.INDEX_TEMPLATE);

String update = Strings.toString(
XContentFactory.jsonBuilder()
Expand All @@ -220,7 +220,7 @@ public void testFieldReplacementForIndexTemplates() throws IOException {
);
DocumentMapper mapper = mapperService.merge(
MapperService.SINGLE_MAPPING_NAME,
new CompressedXContent(update),
List.of(new CompressedXContent(mapping), new CompressedXContent(update)),
MergeReason.INDEX_TEMPLATE
);

Expand Down Expand Up @@ -269,7 +269,6 @@ public void testDisallowFieldReplacementForIndexTemplates() throws IOException {
.endObject()
.endObject()
);
mapperService.merge(MapperService.SINGLE_MAPPING_NAME, new CompressedXContent(mapping), MergeReason.INDEX_TEMPLATE);

String firstUpdate = Strings.toString(
XContentFactory.jsonBuilder()
Expand All @@ -287,7 +286,11 @@ public void testDisallowFieldReplacementForIndexTemplates() throws IOException {
);
IllegalArgumentException e = expectThrows(
IllegalArgumentException.class,
() -> mapperService.merge(MapperService.SINGLE_MAPPING_NAME, new CompressedXContent(firstUpdate), MergeReason.INDEX_TEMPLATE)
() -> mapperService.merge(
MapperService.SINGLE_MAPPING_NAME,
List.of(new CompressedXContent(mapping), new CompressedXContent(firstUpdate)),
MergeReason.INDEX_TEMPLATE
)
);
assertThat(e.getMessage(), containsString("can't merge a non object mapping [object.field2] with an object mapping"));

Expand Down