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.

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

Store Template's mappings as bytes for disk serialization #78746

Merged
merged 23 commits into from
Oct 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
a17e437
Store mappings as bytes for disk serialization
probakowski Oct 6, 2021
1bbccb6
Test fix
probakowski Oct 6, 2021
d490007
rollback changes
probakowski Oct 6, 2021
0880079
fix tests
probakowski Oct 6, 2021
25bb42d
Merge remote-tracking branch 'origin/master' into template-fix
probakowski Oct 6, 2021
256618f
fix serialization with json
probakowski Oct 6, 2021
f36868c
change serialization
probakowski Oct 7, 2021
1473a23
Merge branch 'master' into template-fix
probakowski Oct 7, 2021
f0457af
rework
probakowski Oct 11, 2021
a257b20
Merge remote-tracking branch 'origin/master' into template-fix
probakowski Oct 11, 2021
0d29911
fix compilation
probakowski Oct 11, 2021
579e139
Merge remote-tracking branch 'origin/master' into template-fix
probakowski Oct 11, 2021
fda4aa9
Merge branch 'master' into template-fix
elasticmachine Oct 12, 2021
1758f8d
parse ComposableTemplateMetadata with node tool
probakowski Oct 14, 2021
e1bd326
Merge branch 'master' into template-fix
elasticmachine Oct 14, 2021
58a691a
parse ComponentTemplate in node tool
probakowski Oct 14, 2021
5b0408f
rollback unneeded part
probakowski Oct 14, 2021
f45b195
rollback unneeded part
probakowski Oct 14, 2021
77f744d
rollback unneeded part
probakowski Oct 14, 2021
81265a8
Merge branch 'master' into template-fix
elasticmachine Oct 14, 2021
d8f4f86
Merge branch 'master' into template-fix
elasticmachine Oct 19, 2021
e4ccdaa
Merge branch 'master' into template-fix
elasticmachine Oct 19, 2021
58016b3
Merge branch 'master' into template-fix
elasticmachine Oct 19, 2021
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
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.elasticsearch.cluster.metadata.Template;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.xcontent.ToXContent;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.test.ESTestCase;

Expand Down Expand Up @@ -79,7 +80,7 @@ private static void toXContent(GetComponentTemplatesResponse response, XContentB
builder.startObject();
builder.field("name", e.getKey());
builder.field("component_template");
e.getValue().toXContent(builder, null);
e.getValue().toXContent(builder, ToXContent.EMPTY_PARAMS);
builder.endObject();
}
builder.endArray();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import org.elasticsearch.cluster.metadata.ComposableIndexTemplate;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.ToXContent;
import org.elasticsearch.test.ESTestCase;

import java.io.IOException;
Expand Down Expand Up @@ -53,7 +54,7 @@ private static void toXContent(GetComposableIndexTemplatesResponse response, XCo
builder.startObject();
builder.field("name", e.getKey());
builder.field("index_template");
e.getValue().toXContent(builder, null);
e.getValue().toXContent(builder, ToXContent.EMPTY_PARAMS);
builder.endObject();
}
builder.endArray();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import joptsimple.OptionParser;
import joptsimple.OptionSet;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.lucene.store.LockObtainFailedException;
Expand All @@ -22,20 +23,22 @@
import org.elasticsearch.cluster.ClusterName;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.Diff;
import org.elasticsearch.cluster.metadata.ComponentTemplateMetadata;
import org.elasticsearch.cluster.metadata.ComposableIndexTemplateMetadata;
import org.elasticsearch.cluster.metadata.DataStreamMetadata;
import org.elasticsearch.cluster.metadata.Metadata;
import org.elasticsearch.core.Tuple;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.settings.ClusterSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.BigArrays;
import org.elasticsearch.xcontent.NamedXContentRegistry;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentParser;
import org.elasticsearch.core.Tuple;
import org.elasticsearch.env.Environment;
import org.elasticsearch.env.NodeEnvironment;
import org.elasticsearch.env.NodeMetadata;
import org.elasticsearch.gateway.PersistedClusterStateService;
import org.elasticsearch.xcontent.NamedXContentRegistry;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentParser;

import java.io.IOException;
import java.nio.file.Files;
Expand Down Expand Up @@ -68,7 +71,8 @@ public abstract class ElasticsearchNodeCommand extends EnvironmentAwareCommand {
public <T, C> T parseNamedObject(Class<T> categoryClass, String name, XContentParser parser, C context) throws IOException {
// Currently, two unknown top-level objects are present
if (Metadata.Custom.class.isAssignableFrom(categoryClass)) {
if (DataStreamMetadata.TYPE.equals(name)) {
if (DataStreamMetadata.TYPE.equals(name) || ComposableIndexTemplateMetadata.TYPE.equals(name)
|| ComponentTemplateMetadata.TYPE.equals(name)) {
// DataStreamMetadata is used inside Metadata class for validation purposes and building the indicesLookup,
// therefor even es node commands need to be able to parse it.
return super.parseNamedObject(categoryClass, name, parser, context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ public String toString() {
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
builder.field(TEMPLATE.getPreferredName(), this.template);
builder.field(TEMPLATE.getPreferredName(), this.template, params);
if (this.version != null) {
builder.field(VERSION.getPreferredName(), this.version);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public static ComponentTemplateMetadata fromXContent(XContentParser parser) thro
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(COMPONENT_TEMPLATE.getPreferredName());
for (Map.Entry<String, ComponentTemplate> template : componentTemplates.entrySet()) {
builder.field(template.getKey(), template.getValue());
builder.field(template.getKey(), template.getValue(), params);
}
builder.endObject();
return builder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
builder.startObject();
builder.stringListField(INDEX_PATTERNS.getPreferredName(), this.indexPatterns);
if (this.template != null) {
builder.field(TEMPLATE.getPreferredName(), this.template);
builder.field(TEMPLATE.getPreferredName(), this.template, params);
}
if (this.componentTemplates != null) {
builder.stringListField(COMPOSED_OF.getPreferredName(), this.componentTemplates);
Expand All @@ -217,7 +217,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
builder.field(METADATA.getPreferredName(), metadata);
}
if (this.dataStreamTemplate != null) {
builder.field(DATA_STREAM.getPreferredName(), dataStreamTemplate);
builder.field(DATA_STREAM.getPreferredName(), dataStreamTemplate, params);
}
if (this.allowAutoCreate != null) {
builder.field(ALLOW_AUTO_CREATE.getPreferredName(), allowAutoCreate);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public void writeTo(StreamOutput out) throws IOException {
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(INDEX_TEMPLATE.getPreferredName());
for (Map.Entry<String, ComposableIndexTemplate> template : indexTemplates.entrySet()) {
builder.field(template.getKey(), template.getValue());
builder.field(template.getKey(), template.getValue(), params);
}
builder.endObject();
return builder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.elasticsearch.cluster.AbstractDiffable;
import org.elasticsearch.common.util.Maps;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.xcontent.ObjectParser;
import org.elasticsearch.xcontent.ParseField;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.compress.CompressedXContent;
Expand All @@ -27,6 +28,7 @@
import org.elasticsearch.index.mapper.MapperService;

import java.io.IOException;
import java.util.Base64;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
Expand All @@ -47,8 +49,18 @@ public class Template extends AbstractDiffable<Template> implements ToXContentOb

static {
PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), (p, c) -> Settings.fromXContent(p), SETTINGS);
PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), (p, c) ->
new CompressedXContent(Strings.toString(XContentFactory.jsonBuilder().map(p.mapOrdered()))), MAPPINGS);
PARSER.declareField(ConstructingObjectParser.optionalConstructorArg(), (p, c) -> {
XContentParser.Token token = p.currentToken();
if (token == XContentParser.Token.VALUE_STRING) {
Copy link
Member

Choose a reason for hiding this comment

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

Where do we encode the template like this? This seems broken?
We don't seem to run into a base64 JSON string anywhere for mappings so we shouldn't for templates?

Copy link
Contributor Author

@probakowski probakowski Oct 14, 2021

Choose a reason for hiding this comment

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

This shows up with combination of JsonXContent and binary parameter. I think we have that only in ToAndFromJsonMetadataTests#testSimpleJsonFromAndTo. Thing is we don't serialize IndexMetadata there because Metadata.toXContent will filter them out if context is not XContentContext.API so we don't have to serialize mappings for them as well. Test like below would fail exactly because we don't handle VALUE_STRING case in IndexMetadata, we just don't test it. It would work with SmileXContent.

IndexMetadata.Builder indexMetadataBuilder = IndexMetadata.builder("test12")
    .settings(settings(Version.CURRENT))
    .numberOfShards(1)
    .numberOfReplicas(0)
    .putMapping("{\"mapping1\":{\"text1\":{\"type\":\"string\"}}}");

Map<String, String> params = new HashMap<>(2);
params.put("binary", "true");
params.put(Metadata.CONTEXT_MODE_PARAM, Metadata.CONTEXT_MODE_GATEWAY);

XContentBuilder builder = JsonXContent.contentBuilder();
builder.startObject();
indexMetadataBuilder.build().toXContent(builder, new ToXContent.MapParams(params));
builder.endObject();

IndexMetadata indexMetadata = IndexMetadata.fromXContent(createParser(builder));
assertNotNull(indexMetadata.mapping());

return new CompressedXContent(Base64.getDecoder().decode(p.text()));
} else if(token == XContentParser.Token.VALUE_EMBEDDED_OBJECT){
return new CompressedXContent(p.binaryValue());
} else if (token == XContentParser.Token.START_OBJECT){
return new CompressedXContent(Strings.toString(XContentFactory.jsonBuilder().map(p.mapOrdered())));
} else {
throw new IllegalArgumentException("Unexpected token: " + token);
}
}, MAPPINGS, ObjectParser.ValueType.VALUE_OBJECT_ARRAY);
PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), (p, c) -> {
Map<String, AliasMetadata> aliasMap = new HashMap<>();
while ((p.nextToken()) != XContentParser.Token.END_OBJECT) {
Expand Down Expand Up @@ -160,11 +172,17 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
builder.endObject();
}
if (this.mappings != null) {
Map<String, Object> uncompressedMapping =
XContentHelper.convertToMap(this.mappings.uncompressed(), true, XContentType.JSON).v2();
if (uncompressedMapping.size() > 0) {
builder.field(MAPPINGS.getPreferredName());
builder.map(reduceMapping(uncompressedMapping));
String context = params.param(Metadata.CONTEXT_MODE_PARAM, Metadata.CONTEXT_MODE_API);
boolean binary = params.paramAsBoolean("binary", false);
if (Metadata.CONTEXT_MODE_API.equals(context) || binary == false) {
Map<String, Object> uncompressedMapping =
XContentHelper.convertToMap(this.mappings.uncompressed(), true, XContentType.JSON).v2();
if (uncompressedMapping.size() > 0) {
builder.field(MAPPINGS.getPreferredName());
builder.map(reduceMapping(uncompressedMapping));
}
} else {
builder.field(MAPPINGS.getPreferredName(), mappings.compressed());
}
}
if (this.aliases != null) {
Expand Down