Skip to content

IndexTemplateMetaData should only store a single mapping #50753

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

Closed
wants to merge 4 commits into from
Closed

IndexTemplateMetaData should only store a single mapping #50753

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
28d21c6
IndexTemplateMetaData should only store a single mapping
romseygeek Jan 8, 2020
56cb66c
precommit; upgrade test
romseygeek Jan 8, 2020
8f948f6
Merge remote-tracking branch 'origin/master' into types-removal/index…
romseygeek Jan 8, 2020
0638186
Merge remote-tracking branch 'origin/master' into types-removal/index…
romseygeek Jan 9, 2020
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
8 changes: 4 additions & 4 deletions ...-level/src/test/java/org/elasticsearch/client/indices/GetIndexTemplatesResponseTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public void testParsingFromEsResponse() throws IOException {
randomAlphaOfLength(4));
esIMD.patterns(Arrays.asList(generateRandomStringArray(32, 4, false, false)));
esIMD.settings(randomIndexSettings());
esIMD.putMapping("_doc", new CompressedXContent(BytesReference.bytes(randomMapping("_doc", xContentType))));
esIMD.setMapping(new CompressedXContent(BytesReference.bytes(randomMapping("_doc", xContentType))));
int numAliases = randomIntBetween(0, 8);
for (int j = 0; j < numAliases; j++) {
esIMD.putAlias(randomAliasMetaData(String.format(Locale.ROOT, "%02d ", j) + randomAlphaOfLength(4)));
Expand Down Expand Up @@ -110,8 +110,8 @@ public void testParsingFromEsResponse() throws IOException {
assertThat(result.order(), equalTo(esIMD.order()));
assertThat(result.version(), equalTo(esIMD.version()));

assertThat(esIMD.mappings().size(), equalTo(1));
BytesArray mappingSource = new BytesArray(esIMD.mappings().valuesIt().next().uncompressed());
assertNotNull(esIMD.mappings());
BytesArray mappingSource = new BytesArray(esIMD.mappings().uncompressed());
Map<String, Object> expectedMapping =
XContentHelper.convertToMap(mappingSource, true, xContentBuilder.contentType()).v2();
assertThat(result.mappings().sourceAsMap(), equalTo(expectedMapping.get("_doc")));
Expand Down Expand Up @@ -205,7 +205,7 @@ static void toXContent(GetIndexTemplatesResponse response, XContentBuilder build
serverTemplateBuilder.order(clientITMD.order());
serverTemplateBuilder.version(clientITMD.version());
if (clientITMD.mappings() != null) {
serverTemplateBuilder.putMapping(MapperService.SINGLE_MAPPING_NAME, clientITMD.mappings().source());
serverTemplateBuilder.setMapping(clientITMD.mappings().source());
}
serverIndexTemplates.add(serverTemplateBuilder.build());

Expand Down
16 changes: 16 additions & 0 deletions qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/10_basic.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,3 +205,19 @@
wait_for_completion: true
task_id: $task

---
"Add an index template with a type in the old cluster"
- do:
indices.put_template:
include_type_name: true
name: typed-template
body:
index_patterns: typed-template-*
settings:
number_of_shards: 1
number_of_replicas: 0
mappings:
type1:
properties:
kw:
type: keyword
16 changes: 16 additions & 0 deletions qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/10_basic.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,4 +126,20 @@
task_id: $task_id
- match: { task.headers.X-Opaque-Id: "Reindexing Again" }

---
"Create an index that matches the typed template, and verify that the type is no longer there":
- do:
indices.create:
index: typed-template-test
mappings:
properties:
textfield:
type: text

- do:
indicies.get_mapping:
index: typed-template-test
- is_true: { 'typed-template-test' : mappings.properties.kw }



12 changes: 7 additions & 5 deletions server/src/main/java/org/elasticsearch/cluster/ClusterState.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.discovery.Discovery;
import org.elasticsearch.index.mapper.MapperService;

import java.io.IOException;
import java.util.EnumSet;
Expand Down Expand Up @@ -441,13 +442,14 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
builder.endObject();

builder.startObject("mappings");
for (ObjectObjectCursor<String, CompressedXContent> cursor1 : templateMetaData.mappings()) {
Map<String, Object> mapping = XContentHelper.convertToMap(new BytesArray(cursor1.value.uncompressed()), false).v2();
if (mapping.size() == 1 && mapping.containsKey(cursor1.key)) {
CompressedXContent compressed = templateMetaData.getMappings();
if (compressed != null) {
Map<String, Object> mapping = XContentHelper.convertToMap(new BytesArray(compressed.uncompressed()), false).v2();
if (mapping.size() == 1 && mapping.containsKey(MapperService.SINGLE_MAPPING_NAME)) {
// the type name is the root value, reduce it
mapping = (Map<String, Object>) mapping.get(cursor1.key);
mapping = (Map<String, Object>) mapping.get(MapperService.SINGLE_MAPPING_NAME);
}
builder.field(cursor1.key);
builder.field(MapperService.SINGLE_MAPPING_NAME);
builder.map(mapping);
}
builder.endObject();
Expand Down
94 changes: 58 additions & 36 deletions server/src/main/java/org/elasticsearch/cluster/metadata/IndexTemplateMetaData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.carrotsearch.hppc.cursors.ObjectCursor;
import com.carrotsearch.hppc.cursors.ObjectObjectCursor;
import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.Version;
import org.elasticsearch.cluster.AbstractDiffable;
import org.elasticsearch.cluster.Diff;
import org.elasticsearch.common.Nullable;
Expand All @@ -38,6 +39,7 @@
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.mapper.MapperService;

import java.io.IOException;
import java.util.ArrayList;
Expand Down Expand Up @@ -77,14 +79,13 @@ public class IndexTemplateMetaData extends AbstractDiffable<IndexTemplateMetaDat

private final Settings settings;

// the mapping source should always include the type as top level
private final ImmutableOpenMap<String, CompressedXContent> mappings;

private final ImmutableOpenMap<String, AliasMetaData> aliases;

public IndexTemplateMetaData(String name, int order, Integer version,
List<String> patterns, Settings settings,
ImmutableOpenMap<String, CompressedXContent> mappings,
CompressedXContent mappings,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not very clear if we expect CompressedXContent mappings to contain the type or not? From line 98, it looks like it should not contain type, but then how IndexTemplateMetaData is built fromXContent and builder it looks like mappings already contain a type?
Or we can expect both cases?

ImmutableOpenMap<String, AliasMetaData> aliases) {
if (patterns == null || patterns.isEmpty()) {
throw new IllegalArgumentException("Index patterns must not be null or empty; got " + patterns);
Expand All @@ -94,7 +95,8 @@ public IndexTemplateMetaData(String name, int order, Integer version,
this.version = version;
this.patterns = patterns;
this.settings = settings;
this.mappings = mappings;
this.mappings = ImmutableOpenMap.<String, CompressedXContent>builder()
.fPut(MapperService.SINGLE_MAPPING_NAME, mappings).build();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is not very clear, why we can't have mappings just as CompressedXContent and have this.mappings = mappings?

this.aliases = aliases;
}

Expand Down Expand Up @@ -132,12 +134,12 @@ public Settings settings() {
return this.settings;
}

public ImmutableOpenMap<String, CompressedXContent> mappings() {
return this.mappings;
public CompressedXContent mappings() {
return this.mappings.get(MapperService.SINGLE_MAPPING_NAME);
}

public ImmutableOpenMap<String, CompressedXContent> getMappings() {
return this.mappings;
public CompressedXContent getMappings() {
return this.mappings.get(MapperService.SINGLE_MAPPING_NAME);
}

public ImmutableOpenMap<String, AliasMetaData> aliases() {
Expand Down Expand Up @@ -184,9 +186,18 @@ public static IndexTemplateMetaData readFrom(StreamInput in) throws IOException
builder.order(in.readInt());
builder.patterns(in.readStringList());
builder.settings(Settings.readSettingsFromStream(in));
int mappingsSize = in.readVInt();
for (int i = 0; i < mappingsSize; i++) {
builder.putMapping(in.readString(), CompressedXContent.readCompressedString(in));
if (in.getVersion().before(Version.V_8_0_0)) {
int mappingsSize = in.readVInt();
for (int i = 0; i < mappingsSize; i++) {
// note we can't assert on type value because pre-8 templates may have arbitrary type names
in.readString(); // type - ignored
builder.setMapping(CompressedXContent.readCompressedString(in));
}
}
else {
if (in.readBoolean()) {
builder.setMapping(CompressedXContent.readCompressedString(in));
}
}
int aliasesSize = in.readVInt();
for (int i = 0; i < aliasesSize; i++) {
Expand All @@ -207,10 +218,21 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeInt(order);
out.writeStringCollection(patterns);
Settings.writeSettingsToStream(settings, out);
out.writeVInt(mappings.size());
for (ObjectObjectCursor<String, CompressedXContent> cursor : mappings) {
out.writeString(cursor.key);
cursor.value.writeTo(out);
if (out.getVersion().before(Version.V_8_0_0)) {
out.writeVInt(mappings.size());
for (ObjectObjectCursor<String, CompressedXContent> cursor : mappings) {
out.writeString(cursor.key);
cursor.value.writeTo(out);
}
}
else {
if (mappings.get(MapperService.SINGLE_MAPPING_NAME) == null) {
out.writeBoolean(false);
}
else {
out.writeBoolean(true);
mappings.get(MapperService.SINGLE_MAPPING_NAME).writeTo(out);
}
}
out.writeVInt(aliases.size());
for (ObjectCursor<AliasMetaData> cursor : aliases.values()) {
Expand All @@ -234,13 +256,12 @@ public static class Builder {

private Settings settings = Settings.Builder.EMPTY_SETTINGS;

private final ImmutableOpenMap.Builder<String, CompressedXContent> mappings;
private CompressedXContent mappings;

private final ImmutableOpenMap.Builder<String, AliasMetaData> aliases;

public Builder(String name) {
this.name = name;
mappings = ImmutableOpenMap.builder();
aliases = ImmutableOpenMap.builder();
}

Expand All @@ -251,7 +272,7 @@ public Builder(IndexTemplateMetaData indexTemplateMetaData) {
patterns(indexTemplateMetaData.patterns());
settings(indexTemplateMetaData.settings());

mappings = ImmutableOpenMap.builder(indexTemplateMetaData.mappings());
mappings = indexTemplateMetaData.mappings();
aliases = ImmutableOpenMap.builder(indexTemplateMetaData.aliases());
}

Expand Down Expand Up @@ -281,13 +302,13 @@ public Builder settings(Settings settings) {
return this;
}

public Builder putMapping(String mappingType, CompressedXContent mappingSource) {
mappings.put(mappingType, mappingSource);
public Builder setMapping(CompressedXContent mappingSource) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here it is not very clear if we expect mappingSource to contain a type or not. I saw examples of both

mappings = mappingSource;
return this;
}

public Builder putMapping(String mappingType, String mappingSource) throws IOException {
mappings.put(mappingType, new CompressedXContent(mappingSource));
public Builder setMapping(String mappingSource) throws IOException {
mappings = new CompressedXContent(mappingSource);
return this;
}

Expand All @@ -302,7 +323,7 @@ public Builder putAlias(AliasMetaData.Builder aliasMetaData) {
}

public IndexTemplateMetaData build() {
return new IndexTemplateMetaData(name, order, version, indexPatterns, settings, mappings.build(), aliases.build());
return new IndexTemplateMetaData(name, order, version, indexPatterns, settings, mappings, aliases.build());
}

/**
Expand Down Expand Up @@ -376,11 +397,11 @@ private static void toInnerXContent(IndexTemplateMetaData indexTemplateMetaData,
// always set to true. We therefore only check for include_type_name in this branch.
if (includeTypeName == false) {
Map<String, Object> documentMapping = null;
for (ObjectObjectCursor<String, CompressedXContent> cursor : indexTemplateMetaData.mappings()) {
assert documentMapping == null;
byte[] mappingSource = cursor.value.uncompressed();
CompressedXContent compressed = indexTemplateMetaData.getMappings();
if (compressed != null) {
byte[] mappingSource = compressed.uncompressed();
Map<String, Object> mapping = XContentHelper.convertToMap(new BytesArray(mappingSource), true).v2();
documentMapping = reduceMapping(cursor.key, mapping);
documentMapping = reduceMapping(MapperService.SINGLE_MAPPING_NAME, mapping);
}

if (documentMapping != null) {
Expand All @@ -390,19 +411,21 @@ private static void toInnerXContent(IndexTemplateMetaData indexTemplateMetaData,
}
} else {
builder.startObject("mappings");
for (ObjectObjectCursor<String, CompressedXContent> cursor : indexTemplateMetaData.mappings()) {
byte[] mappingSource = cursor.value.uncompressed();
CompressedXContent compressed = indexTemplateMetaData.getMappings();
if (compressed != null) {
byte[] mappingSource = compressed.uncompressed();
Map<String, Object> mapping = XContentHelper.convertToMap(new BytesArray(mappingSource), true).v2();
mapping = reduceMapping(cursor.key, mapping);
builder.field(cursor.key);
mapping = reduceMapping(MapperService.SINGLE_MAPPING_NAME, mapping);
builder.field(MapperService.SINGLE_MAPPING_NAME);
builder.map(mapping);
}
builder.endObject();
}
} else {
builder.startArray("mappings");
for (ObjectObjectCursor<String, CompressedXContent> cursor : indexTemplateMetaData.mappings()) {
byte[] data = cursor.value.uncompressed();
CompressedXContent compressed = indexTemplateMetaData.getMappings();
if (compressed != null) {
byte[] data = compressed.uncompressed();
builder.map(XContentHelper.convertToMap(new BytesArray(data), true).v2());
}
builder.endArray();
Expand Down Expand Up @@ -444,10 +467,10 @@ public static IndexTemplateMetaData fromXContent(XContentParser parser, String t
if (token == XContentParser.Token.FIELD_NAME) {
currentFieldName = parser.currentName();
} else if (token == XContentParser.Token.START_OBJECT) {
String mappingType = currentFieldName;
String mappingType = MapperService.SINGLE_MAPPING_NAME;
Map<String, Object> mappingSource =
MapBuilder.<String, Object>newMapBuilder().put(mappingType, parser.mapOrdered()).map();
builder.putMapping(mappingType, Strings.toString(XContentFactory.jsonBuilder().map(mappingSource)));
builder.setMapping(Strings.toString(XContentFactory.jsonBuilder().map(mappingSource)));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here looks like setMapping expects mapping with a type

}
}
} else if ("aliases".equals(currentFieldName)) {
Expand All @@ -462,13 +485,12 @@ public static IndexTemplateMetaData fromXContent(XContentParser parser, String t
while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
Map<String, Object> mapping = parser.mapOrdered();
if (mapping.size() == 1) {
String mappingType = mapping.keySet().iterator().next();
String mappingSource = Strings.toString(XContentFactory.jsonBuilder().map(mapping));

if (mappingSource == null) {
// crap, no mapping source, warn?
} else {
builder.putMapping(mappingType, mappingSource);
builder.setMapping(mappingSource);
}
}
}
Expand Down
13 changes: 3 additions & 10 deletions server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,17 +353,10 @@ static Map<String, Object> parseMappings(String mappingsJson, List<IndexTemplate
Map<String, Object> mappings = MapperService.parseMapping(xContentRegistry, mappingsJson);
// apply templates, merging the mappings into the request mapping if exists
for (IndexTemplateMetaData template : templates) {
for (ObjectObjectCursor<String, CompressedXContent> cursor : template.mappings()) {
String mappingString = cursor.value.string();
// Templates are wrapped with their _type names, which for pre-8x templates may not
// be _doc. For now, we unwrap them based on the _type name, and then re-wrap with
// _doc
// TODO in 9x these will all have a _type of _doc so no re-wrapping will be necessary
CompressedXContent compressed = template.getMappings();
if (compressed != null) {
String mappingString = compressed.string();
Map<String, Object> templateMapping = MapperService.parseMapping(xContentRegistry, mappingString);
assert templateMapping.size() == 1 : templateMapping;
assert cursor.key.equals(templateMapping.keySet().iterator().next()) : cursor.key + " != " + templateMapping;
templateMapping = Collections.singletonMap(MapperService.SINGLE_MAPPING_NAME,
templateMapping.values().iterator().next());
if (mappings.isEmpty()) {
mappings = templateMapping;
} else {
Expand Down
Loading