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

Remove PROTO-serialization from IndexMetaData.Custom #32749

Merged
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
3f5d717
Remove PROTO-serialization from IndexMetaData.Custom (II)
imotov Aug 8, 2018
b8239f5
Remove automatic xcontent registry discovery
imotov Aug 14, 2018
9986710
Address @dakrone's review comments
imotov Aug 20, 2018
d85bd10
Merge remote-tracking branch 'elastic/master' into remove-proto-from-…
imotov Aug 20, 2018
290afc4
Merge remote-tracking branch 'origin/master' into remove-proto-from-i…
dakrone Aug 27, 2018
c1fc780
WIP
dakrone Aug 28, 2018
4a95041
Merge remote-tracking branch 'origin/master' into remove-proto-from-i…
dakrone Aug 28, 2018
df0f86a
Remove more vestiges
dakrone Aug 28, 2018
f4831f2
Fix some things
dakrone Aug 28, 2018
5a471f2
Fix a test
dakrone Aug 28, 2018
08d3a50
AwaitsFix a test until we decide
dakrone Aug 28, 2018
b8c9628
Remove most of the REST-facing custom metadata parts
dakrone Aug 28, 2018
db11af1
More removals
dakrone Aug 28, 2018
ea49b8c
Merge remote-tracking branch 'origin/master' into remove-proto-from-i…
dakrone Aug 29, 2018
cd46e81
Implement diffs for DiffableStringMap
dakrone Aug 29, 2018
af115fd
Fix NativeRolesStoreTests
dakrone Aug 29, 2018
cdb21f9
Fix serialization for pre-7.x versions
dakrone Aug 29, 2018
65c8165
Simplify DiffableStringMap
dakrone Aug 30, 2018
2643723
Return an immutable map from `getCustomData(String)`
dakrone Aug 30, 2018
3d15e60
Remove newline from SPI file
dakrone Aug 30, 2018
bf50d45
Remove logging from test
dakrone Aug 30, 2018
6523d40
Add random diffing and serialization test
dakrone Aug 30, 2018
81be184
Make receiving custom metadata throw real exception
dakrone Aug 30, 2018
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 @@ -55,8 +55,6 @@ public class CreateIndexClusterStateUpdateRequest extends ClusterStateUpdateRequ

private final Set<Alias> aliases = new HashSet<>();

private final Map<String, IndexMetaData.Custom> customs = new HashMap<>();

private final Set<ClusterBlock> blocks = new HashSet<>();

private ActiveShardCount waitForActiveShards = ActiveShardCount.DEFAULT;
Expand All @@ -83,11 +81,6 @@ public CreateIndexClusterStateUpdateRequest aliases(Set<Alias> aliases) {
return this;
}

public CreateIndexClusterStateUpdateRequest customs(Map<String, IndexMetaData.Custom> customs) {
this.customs.putAll(customs);
return this;
}

public CreateIndexClusterStateUpdateRequest blocks(Set<ClusterBlock> blocks) {
this.blocks.addAll(blocks);
return this;
Expand Down Expand Up @@ -146,10 +139,6 @@ public Set<Alias> aliases() {
return aliases;
}

public Map<String, IndexMetaData.Custom> customs() {
return customs;
}

public Set<ClusterBlock> blocks() {
return blocks;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import org.elasticsearch.action.support.ActiveShardCount;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.action.support.master.AcknowledgedRequest;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesArray;
Expand Down Expand Up @@ -58,9 +57,9 @@
import java.util.Set;

import static org.elasticsearch.action.ValidateActions.addValidationError;
import static org.elasticsearch.common.settings.Settings.Builder.EMPTY_SETTINGS;
import static org.elasticsearch.common.settings.Settings.readSettingsFromStream;
import static org.elasticsearch.common.settings.Settings.writeSettingsToStream;
import static org.elasticsearch.common.settings.Settings.Builder.EMPTY_SETTINGS;

/**
* A request to create an index. Best created with {@link org.elasticsearch.client.Requests#createIndexRequest(String)}.
Expand All @@ -87,8 +86,6 @@ public class CreateIndexRequest extends AcknowledgedRequest<CreateIndexRequest>

private final Set<Alias> aliases = new HashSet<>();

private final Map<String, IndexMetaData.Custom> customs = new HashMap<>();

private ActiveShardCount waitForActiveShards = ActiveShardCount.DEFAULT;

public CreateIndexRequest() {
Expand Down Expand Up @@ -388,18 +385,7 @@ public CreateIndexRequest source(Map<String, ?> source, DeprecationHandler depre
} else if (ALIASES.match(name, deprecationHandler)) {
aliases((Map<String, Object>) entry.getValue());
} else {
// maybe custom?
IndexMetaData.Custom proto = IndexMetaData.lookupPrototype(name);
if (proto != null) {
try {
customs.put(name, proto.fromMap((Map<String, Object>) entry.getValue()));
} catch (IOException e) {
throw new ElasticsearchParseException("failed to parse custom metadata for [{}]", name);
}
} else {
// found a key which is neither custom defined nor one of the supported ones
throw new ElasticsearchParseException("unknown key [{}] for create index", name);
}
throw new ElasticsearchParseException("unknown key [{}] for create index", name);
}
}
return this;
Expand All @@ -413,18 +399,6 @@ public Set<Alias> aliases() {
return this.aliases;
}

/**
* Adds custom metadata to the index to be created.
*/
public CreateIndexRequest custom(IndexMetaData.Custom custom) {
customs.put(custom.type(), custom);
return this;
}

public Map<String, IndexMetaData.Custom> customs() {
return this.customs;
}

public ActiveShardCount waitForActiveShards() {
return waitForActiveShards;
}
Expand Down Expand Up @@ -474,11 +448,10 @@ public void readFrom(StreamInput in) throws IOException {
}
mappings.put(type, source);
}
int customSize = in.readVInt();
for (int i = 0; i < customSize; i++) {
String type = in.readString();
IndexMetaData.Custom customIndexMetaData = IndexMetaData.lookupPrototypeSafe(type).readFrom(in);
customs.put(type, customIndexMetaData);
if (in.getVersion().before(Version.V_7_0_0_alpha1)) {
// This used to be the size of custom metadata classes
int customSize = in.readVInt();
assert customSize == 0 : "unexpected custom metadata when none is supported";
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm do we need to skip the size if we are in production? I mean that assert will not trip if we run without -ea

Copy link
Member

Choose a reason for hiding this comment

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

I'm not following, do you want it to be a regular exception instead of an assert? We still need to do the readVInt regardless of -ea or without, it's just the 0 size comparison that is an assert for tests

Copy link
Member

Choose a reason for hiding this comment

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

I think that we need to do a hard exception indeed, we won't be able to read the rest of the message at this point because we can't read the customs (no IndexMetaData#lookupPrototypeSafe anymore) when there are unexpected ones. That is, customSize > 0 is fatal for us.

}
int aliasesSize = in.readVInt();
for (int i = 0; i < aliasesSize; i++) {
Expand All @@ -501,10 +474,9 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeString(entry.getKey());
out.writeString(entry.getValue());
}
out.writeVInt(customs.size());
for (Map.Entry<String, IndexMetaData.Custom> entry : customs.entrySet()) {
out.writeString(entry.getKey());
entry.getValue().writeTo(out);
if (out.getVersion().before(Version.V_7_0_0_alpha1)) {
// Size of custom index metadata, which is removed
out.writeVInt(0);
}
out.writeVInt(aliases.size());
for (Alias alias : aliases) {
Expand Down Expand Up @@ -542,10 +514,6 @@ public XContentBuilder innerToXContent(XContentBuilder builder, Params params) t
alias.toXContent(builder, params);
}
builder.endObject();

for (Map.Entry<String, IndexMetaData.Custom> entry : customs.entrySet()) {
builder.field(entry.getKey(), entry.getValue(), params);
}
return builder;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import org.elasticsearch.action.support.ActiveShardCount;
import org.elasticsearch.action.support.master.AcknowledgedRequestBuilder;
import org.elasticsearch.client.ElasticsearchClient;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
Expand Down Expand Up @@ -224,14 +223,6 @@ public CreateIndexRequestBuilder setSource(Map<String, ?> source) {
return this;
}

/**
* Adds custom metadata to the index to be created.
*/
public CreateIndexRequestBuilder addCustom(IndexMetaData.Custom custom) {
request.custom(custom);
return this;
}

/**
* Sets the settings and mappings as a single source.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ protected void masterOperation(final CreateIndexRequest request, final ClusterSt
final CreateIndexClusterStateUpdateRequest updateRequest = new CreateIndexClusterStateUpdateRequest(request, cause, indexName, request.index())
.ackTimeout(request.timeout()).masterNodeTimeout(request.masterNodeTimeout())
.settings(request.settings()).mappings(request.mappings())
.aliases(request.aliases()).customs(request.customs())
.aliases(request.aliases())
.waitForActiveShards(request.waitForActiveShards());

createIndexService.createIndex(updateRequest, ActionListener.wrap(response ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,6 @@ static CreateIndexClusterStateUpdateRequest prepareCreateIndexRequest(final Resi
.masterNodeTimeout(targetIndex.masterNodeTimeout())
.settings(targetIndex.settings())
.aliases(targetIndex.aliases())
.customs(targetIndex.customs())
.waitForActiveShards(targetIndex.waitForActiveShards())
.recoverFrom(metaData.getIndex())
.resizeType(resizeRequest.getResizeType())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.action.support.master.MasterNodeRequest;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.bytes.BytesReference;
Expand Down Expand Up @@ -61,9 +60,9 @@
import java.util.stream.Collectors;

import static org.elasticsearch.action.ValidateActions.addValidationError;
import static org.elasticsearch.common.settings.Settings.Builder.EMPTY_SETTINGS;
import static org.elasticsearch.common.settings.Settings.readSettingsFromStream;
import static org.elasticsearch.common.settings.Settings.writeSettingsToStream;
import static org.elasticsearch.common.settings.Settings.Builder.EMPTY_SETTINGS;

/**
* A request to create an index template.
Expand All @@ -88,8 +87,6 @@ public class PutIndexTemplateRequest extends MasterNodeRequest<PutIndexTemplateR

private final Set<Alias> aliases = new HashSet<>();

private Map<String, IndexMetaData.Custom> customs = new HashMap<>();

private Integer version;

public PutIndexTemplateRequest() {
Expand Down Expand Up @@ -353,15 +350,7 @@ public PutIndexTemplateRequest source(Map<String, Object> templateSource) {
} else if (name.equals("aliases")) {
aliases((Map<String, Object>) entry.getValue());
} else {
// maybe custom?
IndexMetaData.Custom proto = IndexMetaData.lookupPrototype(name);
if (proto != null) {
try {
customs.put(name, proto.fromMap((Map<String, Object>) entry.getValue()));
} catch (IOException e) {
throw new ElasticsearchParseException("failed to parse custom metadata for [{}]", name);
}
}
throw new ElasticsearchParseException("unknown key [{}] in the template ", name);
}
}
return this;
Expand Down Expand Up @@ -395,15 +384,6 @@ public PutIndexTemplateRequest source(BytesReference source, XContentType xConte
return source(XContentHelper.convertToMap(source, true, xContentType).v2());
}

public PutIndexTemplateRequest custom(IndexMetaData.Custom custom) {
customs.put(custom.type(), custom);
return this;
}

public Map<String, IndexMetaData.Custom> customs() {
return this.customs;
}

public Set<Alias> aliases() {
return this.aliases;
}
Expand Down Expand Up @@ -494,11 +474,10 @@ public void readFrom(StreamInput in) throws IOException {
String mappingSource = in.readString();
mappings.put(type, mappingSource);
}
int customSize = in.readVInt();
for (int i = 0; i < customSize; i++) {
String type = in.readString();
IndexMetaData.Custom customIndexMetaData = IndexMetaData.lookupPrototypeSafe(type).readFrom(in);
customs.put(type, customIndexMetaData);
if (in.getVersion().before(Version.V_7_0_0_alpha1)) {
// Used to be used for custom index metadata
int customSize = in.readVInt();
assert customSize == 0 : "expected not to have any custom metadata";
Copy link
Member

Choose a reason for hiding this comment

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

This should be fatal.

}
int aliasesSize = in.readVInt();
for (int i = 0; i < aliasesSize; i++) {
Expand All @@ -525,10 +504,8 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeString(entry.getKey());
out.writeString(entry.getValue());
}
out.writeVInt(customs.size());
for (Map.Entry<String, IndexMetaData.Custom> entry : customs.entrySet()) {
out.writeString(entry.getKey());
entry.getValue().writeTo(out);
if (out.getVersion().before(Version.V_7_0_0_alpha1)) {
out.writeVInt(0);
}
out.writeVInt(aliases.size());
for (Alias alias : aliases) {
Expand Down Expand Up @@ -565,10 +542,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
}
builder.endObject();

for (Map.Entry<String, IndexMetaData.Custom> entry : customs.entrySet()) {
builder.field(entry.getKey(), entry.getValue(), params);
}

return builder;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ protected void masterOperation(final PutIndexTemplateRequest request, final Clus
.settings(templateSettingsBuilder.build())
.mappings(request.mappings())
.aliases(request.aliases())
.customs(request.customs())
.create(request.create())
.masterTimeout(request.masterNodeTimeout())
.version(request.version()),
Expand Down
Loading