From 9e5924958b40560c1d0e829f5b1e601599122c5e Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sat, 25 Aug 2018 09:20:17 -0400 Subject: [PATCH 01/11] Introduce mapping version to index metadata This commit introduces mapping version to index metadata. This value is monotonically increasing and is updated on mapping updates. This will be useful in cross-cluster replication so that we can request mapping updates from the leader only when there is a mapping update as opposed to the strategy we employ today which is to request a mapping update any time there is an index metadata update. As index metadata updates can occur for many reasons other than mapping updates, this leads to some unnecessary requests and work in cross-cluster replication. --- .../elasticsearch/cluster/ClusterState.java | 1 + .../cluster/metadata/IndexMetaData.java | 37 ++++++++++++++++++- .../metadata/MetaDataMappingService.java | 8 ++++ .../snapshots/RestoreService.java | 1 + .../metadata/MetaDataMappingServiceTests.java | 37 ++++++++++++++++++- .../gateway/MetaDataStateFormatTests.java | 1 + .../index/mapper/DynamicMappingTests.java | 10 +++++ .../index/mapper/UpdateMappingTests.java | 29 +++++++++++++++ 8 files changed, 121 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/ClusterState.java b/server/src/main/java/org/elasticsearch/cluster/ClusterState.java index 276e00a2ba3db..8f3b99e5d2ffb 100644 --- a/server/src/main/java/org/elasticsearch/cluster/ClusterState.java +++ b/server/src/main/java/org/elasticsearch/cluster/ClusterState.java @@ -285,6 +285,7 @@ public String toString() { for (IndexMetaData indexMetaData : metaData) { sb.append(TAB).append(indexMetaData.getIndex()); sb.append(": v[").append(indexMetaData.getVersion()).append("]\n"); + sb.append(": mv[").append(indexMetaData.getMappingVersion()).append("]\n"); for (int shard = 0; shard < indexMetaData.getNumberOfShards(); shard++) { sb.append(TAB).append(TAB).append(shard).append(": "); sb.append("p_term [").append(indexMetaData.primaryTerm(shard)).append("], "); diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java index 18b89db72a391..d3ee7c2535ef2 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java @@ -291,6 +291,7 @@ public Iterator> settings() { public static final String KEY_IN_SYNC_ALLOCATIONS = "in_sync_allocations"; static final String KEY_VERSION = "version"; + static final String KEY_MAPPING_VERSION = "mapping_version"; static final String KEY_ROUTING_NUM_SHARDS = "routing_num_shards"; static final String KEY_SETTINGS = "settings"; static final String KEY_STATE = "state"; @@ -309,6 +310,9 @@ public Iterator> settings() { private final Index index; private final long version; + + private final long mappingVersion; + private final long[] primaryTerms; private final State state; @@ -336,7 +340,7 @@ public Iterator> settings() { private final ActiveShardCount waitForActiveShards; private final ImmutableOpenMap rolloverInfos; - private IndexMetaData(Index index, long version, long[] primaryTerms, State state, int numberOfShards, int numberOfReplicas, Settings settings, + private IndexMetaData(Index index, long version, long mappingVersion, long[] primaryTerms, State state, int numberOfShards, int numberOfReplicas, Settings settings, ImmutableOpenMap mappings, ImmutableOpenMap aliases, ImmutableOpenMap customs, ImmutableOpenIntMap> inSyncAllocationIds, DiscoveryNodeFilters requireFilters, DiscoveryNodeFilters initialRecoveryFilters, DiscoveryNodeFilters includeFilters, DiscoveryNodeFilters excludeFilters, @@ -345,6 +349,7 @@ private IndexMetaData(Index index, long version, long[] primaryTerms, State stat this.index = index; this.version = version; + this.mappingVersion = mappingVersion; this.primaryTerms = primaryTerms; assert primaryTerms.length == numberOfShards; this.state = state; @@ -394,6 +399,9 @@ public long getVersion() { return this.version; } + public long getMappingVersion() { + return mappingVersion; + } /** * The term of the current selected primary. This is a non-negative number incremented when @@ -644,6 +652,7 @@ private static class IndexMetaDataDiff implements Diff { private final String index; private final int routingNumShards; private final long version; + private final long mappingVersion; private final long[] primaryTerms; private final State state; private final Settings settings; @@ -656,6 +665,7 @@ private static class IndexMetaDataDiff implements Diff { IndexMetaDataDiff(IndexMetaData before, IndexMetaData after) { index = after.index.getName(); version = after.version; + mappingVersion = after.mappingVersion; routingNumShards = after.routingNumShards; state = after.state; settings = after.settings; @@ -672,6 +682,11 @@ private static class IndexMetaDataDiff implements Diff { index = in.readString(); routingNumShards = in.readInt(); version = in.readLong(); + if (in.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) { + mappingVersion = in.readVLong(); + } else { + mappingVersion = 1; + } state = State.fromId(in.readByte()); settings = Settings.readSettingsFromStream(in); primaryTerms = in.readVLongArray(); @@ -707,6 +722,9 @@ public void writeTo(StreamOutput out) throws IOException { out.writeString(index); out.writeInt(routingNumShards); out.writeLong(version); + if (out.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) { + out.writeVLong(mappingVersion); + } out.writeByte(state.id); Settings.writeSettingsToStream(settings, out); out.writeVLongArray(primaryTerms); @@ -723,6 +741,7 @@ public void writeTo(StreamOutput out) throws IOException { public IndexMetaData apply(IndexMetaData part) { Builder builder = builder(index); builder.version(version); + builder.mappingVersion(mappingVersion); builder.setRoutingNumShards(routingNumShards); builder.state(state); builder.settings(settings); @@ -821,6 +840,7 @@ public static class Builder { private String index; private State state = State.OPEN; private long version = 1; + private long mappingVersion = 1; private long[] primaryTerms = null; private Settings settings = Settings.Builder.EMPTY_SETTINGS; private final ImmutableOpenMap.Builder mappings; @@ -843,6 +863,7 @@ public Builder(IndexMetaData indexMetaData) { this.index = indexMetaData.getIndex().getName(); this.state = indexMetaData.state; this.version = indexMetaData.version; + this.mappingVersion = indexMetaData.mappingVersion; this.settings = indexMetaData.getSettings(); this.primaryTerms = indexMetaData.primaryTerms.clone(); this.mappings = ImmutableOpenMap.builder(indexMetaData.mappings); @@ -1009,6 +1030,15 @@ public Builder version(long version) { return this; } + public long mappingVersion() { + return mappingVersion; + } + + public Builder mappingVersion(final long mappingVersion) { + this.mappingVersion = mappingVersion; + return this; + } + /** * returns the primary term for the given shard. * See {@link IndexMetaData#primaryTerm(int)} for more information. @@ -1136,7 +1166,7 @@ public IndexMetaData build() { final String uuid = settings.get(SETTING_INDEX_UUID, INDEX_UUID_NA_VALUE); - return new IndexMetaData(new Index(index, uuid), version, primaryTerms, state, numberOfShards, numberOfReplicas, tmpSettings, mappings.build(), + return new IndexMetaData(new Index(index, uuid), version, mappingVersion, primaryTerms, state, numberOfShards, numberOfReplicas, tmpSettings, mappings.build(), tmpAliases.build(), customs.build(), filledInSyncAllocationIds.build(), requireFilters, initialRecoveryFilters, includeFilters, excludeFilters, indexCreatedVersion, indexUpgradedVersion, getRoutingNumShards(), routingPartitionSize, waitForActiveShards, rolloverInfos.build()); } @@ -1145,6 +1175,7 @@ public static void toXContent(IndexMetaData indexMetaData, XContentBuilder build builder.startObject(indexMetaData.getIndex().getName()); builder.field(KEY_VERSION, indexMetaData.getVersion()); + builder.field(KEY_MAPPING_VERSION, indexMetaData.getMappingVersion()); builder.field(KEY_ROUTING_NUM_SHARDS, indexMetaData.getRoutingNumShards()); builder.field(KEY_STATE, indexMetaData.getState().toString().toLowerCase(Locale.ENGLISH)); @@ -1316,6 +1347,8 @@ public static IndexMetaData fromXContent(XContentParser parser) throws IOExcepti builder.state(State.fromString(parser.text())); } else if (KEY_VERSION.equals(currentFieldName)) { builder.version(parser.longValue()); + } else if (KEY_MAPPING_VERSION.equals(currentFieldName)) { + builder.mappingVersion(parser.longValue()); } else if (KEY_ROUTING_NUM_SHARDS.equals(currentFieldName)) { builder.setRoutingNumShards(parser.intValue()); } else { diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java index 82d947b4158a2..b021ec701742e 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java @@ -329,6 +329,14 @@ private ClusterState applyRequest(ClusterState currentState, PutMappingClusterSt indexMetaDataBuilder.putMapping(new MappingMetaData(mapper.mappingSource())); } } + if (updated) { + indexMetaDataBuilder.mappingVersion(1 + indexMetaDataBuilder.mappingVersion()); + } + /* + * This implicitly increments the index metadata version and builds the index metadata. This means that we need to have + * already incremented the mapping version if necessary. Therefore, the mapping version increment must remain before this + * statement. + */ builder.put(indexMetaDataBuilder); } if (updated) { diff --git a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java index a7df9bdfdfd87..702d63d0d9401 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java @@ -292,6 +292,7 @@ public ClusterState execute(ClusterState currentState) { // Index exists and it's closed - open it in metadata and start recovery IndexMetaData.Builder indexMdBuilder = IndexMetaData.builder(snapshotIndexMetaData).state(IndexMetaData.State.OPEN); indexMdBuilder.version(Math.max(snapshotIndexMetaData.getVersion(), currentIndexMetaData.getVersion() + 1)); + indexMdBuilder.mappingVersion(Math.max(snapshotIndexMetaData.getMappingVersion(), currentIndexMetaData.getMappingVersion() + 1)); if (!request.includeAliases()) { // Remove all snapshot aliases if (!snapshotIndexMetaData.getAliases().isEmpty()) { diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataMappingServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataMappingServiceTests.java index 1e46c2c428663..67817847a2e7a 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataMappingServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataMappingServiceTests.java @@ -19,9 +19,12 @@ package org.elasticsearch.cluster.metadata; import org.elasticsearch.action.admin.indices.mapping.put.PutMappingClusterStateUpdateRequest; +import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.ClusterStateTaskExecutor; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.compress.CompressedXContent; +import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexService; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.test.ESSingleNodeTestCase; @@ -47,7 +50,8 @@ public void testMappingClusterStateUpdateDoesntChangeExistingIndices() throws Ex final ClusterService clusterService = getInstanceFromNode(ClusterService.class); // TODO - it will be nice to get a random mapping generator final PutMappingClusterStateUpdateRequest request = new PutMappingClusterStateUpdateRequest().type("type"); - request.source("{ \"properties\" { \"field\": { \"type\": \"text\" }}}"); + request.indices(new Index[] {indexService.index()}); + request.source("{ \"properties\": { \"field\": { \"type\": \"text\" }}}"); mappingService.putMappingExecutor.execute(clusterService.state(), Collections.singletonList(request)); assertThat(indexService.mapperService().documentMapper("type").mappingSource(), equalTo(currentMapping)); } @@ -69,4 +73,35 @@ public void testClusterStateIsNotChangedWithIdenticalMappings() throws Exception assertSame(result, result2); } + + public void testMappingVersion() throws Exception { + final IndexService indexService = createIndex("test", client().admin().indices().prepareCreate("test").addMapping("type")); + final long previousVersion = indexService.getMetaData().getMappingVersion(); + final MetaDataMappingService mappingService = getInstanceFromNode(MetaDataMappingService.class); + final ClusterService clusterService = getInstanceFromNode(ClusterService.class); + final PutMappingClusterStateUpdateRequest request = new PutMappingClusterStateUpdateRequest().type("type"); + request.indices(new Index[] {indexService.index()}); + request.source("{ \"properties\": { \"field\": { \"type\": \"text\" }}}"); + final ClusterStateTaskExecutor.ClusterTasksResult result = + mappingService.putMappingExecutor.execute(clusterService.state(), Collections.singletonList(request)); + assertThat(result.executionResults.size(), equalTo(1)); + assertTrue(result.executionResults.values().iterator().next().isSuccess()); + assertThat(result.resultingState.metaData().index("test").getMappingVersion(), equalTo(1 + previousVersion)); + } + + public void testMappingVersionUnchanged() throws Exception { + final IndexService indexService = createIndex("test", client().admin().indices().prepareCreate("test").addMapping("type")); + final long previousVersion = indexService.getMetaData().getMappingVersion(); + final MetaDataMappingService mappingService = getInstanceFromNode(MetaDataMappingService.class); + final ClusterService clusterService = getInstanceFromNode(ClusterService.class); + final PutMappingClusterStateUpdateRequest request = new PutMappingClusterStateUpdateRequest().type("type"); + request.indices(new Index[] {indexService.index()}); + request.source("{ \"properties\": {}}"); + final ClusterStateTaskExecutor.ClusterTasksResult result = + mappingService.putMappingExecutor.execute(clusterService.state(), Collections.singletonList(request)); + assertThat(result.executionResults.size(), equalTo(1)); + assertTrue(result.executionResults.values().iterator().next().isSuccess()); + assertThat(result.resultingState.metaData().index("test").getMappingVersion(), equalTo(previousVersion)); + } + } diff --git a/server/src/test/java/org/elasticsearch/gateway/MetaDataStateFormatTests.java b/server/src/test/java/org/elasticsearch/gateway/MetaDataStateFormatTests.java index d236d01f049dd..0bf80e5239874 100644 --- a/server/src/test/java/org/elasticsearch/gateway/MetaDataStateFormatTests.java +++ b/server/src/test/java/org/elasticsearch/gateway/MetaDataStateFormatTests.java @@ -267,6 +267,7 @@ public void testLoadState() throws IOException { IndexMetaData deserialized = indices.get(original.getIndex().getName()); assertThat(deserialized, notNullValue()); assertThat(deserialized.getVersion(), equalTo(original.getVersion())); + assertThat(deserialized.getMappingVersion(), equalTo(original.getMappingVersion())); assertThat(deserialized.getNumberOfReplicas(), equalTo(original.getNumberOfReplicas())); assertThat(deserialized.getNumberOfShards(), equalTo(original.getNumberOfShards())); } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DynamicMappingTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DynamicMappingTests.java index 7d022b5545443..cb2ed785699c8 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DynamicMappingTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DynamicMappingTests.java @@ -22,6 +22,7 @@ import org.elasticsearch.Version; import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsResponse; import org.elasticsearch.cluster.metadata.IndexMetaData; +import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.compress.CompressedXContent; @@ -741,4 +742,13 @@ public void testDynamicTemplateOrder() throws IOException { client().prepareIndex("test", "type", "1").setSource("foo", "abc").get(); assertThat(index.mapperService().fullName("foo"), instanceOf(KeywordFieldMapper.KeywordFieldType.class)); } + + public void testMappingVersionAfterDynamicMappingUpdate() { + createIndex("test", client().admin().indices().prepareCreate("test").addMapping("type")); + final ClusterService clusterService = getInstanceFromNode(ClusterService.class); + final long previousVersion = clusterService.state().metaData().index("test").getMappingVersion(); + client().prepareIndex("test", "type", "1").setSource("field", "text").get(); + assertThat(clusterService.state().metaData().index("test").getMappingVersion(), equalTo(1 + previousVersion)); + } + } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/UpdateMappingTests.java b/server/src/test/java/org/elasticsearch/index/mapper/UpdateMappingTests.java index 3f8e8e9efec39..d8650331d2323 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/UpdateMappingTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/UpdateMappingTests.java @@ -19,6 +19,8 @@ package org.elasticsearch.index.mapper; +import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; +import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.compress.CompressedXContent; @@ -30,6 +32,7 @@ import org.elasticsearch.plugins.Plugin; import org.elasticsearch.test.ESSingleNodeTestCase; import org.elasticsearch.test.InternalSettingsPlugin; +import org.hamcrest.Matchers; import java.io.IOException; import java.util.Collection; @@ -188,4 +191,30 @@ public void testRejectFieldDefinedTwice() throws IOException { () -> mapperService2.merge("type", new CompressedXContent(mapping1), MergeReason.MAPPING_UPDATE)); assertThat(e.getMessage(), equalTo("mapper [foo] of different type, current_type [long], merged_type [ObjectMapper]")); } + + public void testMappingVersion() { + createIndex("test", client().admin().indices().prepareCreate("test").addMapping("type")); + final ClusterService clusterService = getInstanceFromNode(ClusterService.class); + { + final long previousVersion = clusterService.state().metaData().index("test").getMappingVersion(); + final PutMappingRequest request = new PutMappingRequest(); + request.indices("test"); + request.type("type"); + request.source("field", "type=text"); + client().admin().indices().putMapping(request).actionGet(); + assertThat(clusterService.state().metaData().index("test").getMappingVersion(), Matchers.equalTo(1 + previousVersion)); + } + + { + final long previousVersion = clusterService.state().metaData().index("test").getMappingVersion(); + final PutMappingRequest request = new PutMappingRequest(); + request.indices("test"); + request.type("type"); + request.source("field", "type=text"); + client().admin().indices().putMapping(request).actionGet(); + // the version should be unchanged after putting the same mapping again + assertThat(clusterService.state().metaData().index("test").getMappingVersion(), Matchers.equalTo(previousVersion)); + } + } + } From f9e3f560e5d1b76a9ce4a6f18f4afd7aa5e0ea14 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sat, 25 Aug 2018 14:31:57 -0400 Subject: [PATCH 02/11] Remove import --- .../cluster/metadata/MetaDataMappingServiceTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataMappingServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataMappingServiceTests.java index 67817847a2e7a..88215d0aef0c9 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataMappingServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataMappingServiceTests.java @@ -19,7 +19,6 @@ package org.elasticsearch.cluster.metadata; import org.elasticsearch.action.admin.indices.mapping.put.PutMappingClusterStateUpdateRequest; -import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.ClusterStateTaskExecutor; import org.elasticsearch.cluster.service.ClusterService; From ad6a0487ebcff90ef263aedbf41da4a8889ce30f Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sat, 25 Aug 2018 15:09:29 -0400 Subject: [PATCH 03/11] Add assertion --- .../java/org/elasticsearch/cluster/metadata/IndexMetaData.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java index d3ee7c2535ef2..25ecd56e5da76 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java @@ -349,6 +349,7 @@ private IndexMetaData(Index index, long version, long mappingVersion, long[] pri this.index = index; this.version = version; + assert mappingVersion >= 0 : mappingVersion; this.mappingVersion = mappingVersion; this.primaryTerms = primaryTerms; assert primaryTerms.length == numberOfShards; From 755d4a511974a3c6601ca3b5756c5020e7a3c3d1 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sat, 25 Aug 2018 15:10:29 -0400 Subject: [PATCH 04/11] Add missing serialization --- .../org/elasticsearch/cluster/metadata/IndexMetaData.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java index 25ecd56e5da76..876ed60848fcc 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java @@ -759,6 +759,11 @@ public IndexMetaData apply(IndexMetaData part) { public static IndexMetaData readFrom(StreamInput in) throws IOException { Builder builder = new Builder(in.readString()); builder.version(in.readLong()); + if (in.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) { + builder.mappingVersion(in.readVLong()); + } else { + builder.mappingVersion(1); + } builder.setRoutingNumShards(in.readInt()); builder.state(State.fromId(in.readByte())); builder.settings(readSettingsFromStream(in)); @@ -798,6 +803,9 @@ public static IndexMetaData readFrom(StreamInput in) throws IOException { public void writeTo(StreamOutput out) throws IOException { out.writeString(index.getName()); // uuid will come as part of settings out.writeLong(version); + if (out.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) { + out.writeVLong(mappingVersion); + } out.writeInt(routingNumShards); out.writeByte(state.id()); writeSettingsToStream(settings, out); From e1c6fe665d495abab2b30b45cd784117181f53af Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sat, 25 Aug 2018 15:14:51 -0400 Subject: [PATCH 05/11] Assert mapping version present --- .../org/elasticsearch/cluster/metadata/IndexMetaData.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java index 876ed60848fcc..31bf260e90135 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java @@ -24,6 +24,7 @@ import com.carrotsearch.hppc.cursors.ObjectCursor; import com.carrotsearch.hppc.cursors.ObjectObjectCursor; +import org.elasticsearch.Assertions; import org.elasticsearch.Version; import org.elasticsearch.action.admin.indices.rollover.RolloverInfo; import org.elasticsearch.action.support.ActiveShardCount; @@ -1258,6 +1259,7 @@ public static IndexMetaData fromXContent(XContentParser parser) throws IOExcepti if (token != XContentParser.Token.START_OBJECT) { throw new IllegalArgumentException("expected object but got a " + token); } + boolean mappingVersion = false; while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { if (token == XContentParser.Token.FIELD_NAME) { currentFieldName = parser.currentName(); @@ -1357,6 +1359,7 @@ public static IndexMetaData fromXContent(XContentParser parser) throws IOExcepti } else if (KEY_VERSION.equals(currentFieldName)) { builder.version(parser.longValue()); } else if (KEY_MAPPING_VERSION.equals(currentFieldName)) { + mappingVersion = true; builder.mappingVersion(parser.longValue()); } else if (KEY_ROUTING_NUM_SHARDS.equals(currentFieldName)) { builder.setRoutingNumShards(parser.intValue()); @@ -1367,6 +1370,9 @@ public static IndexMetaData fromXContent(XContentParser parser) throws IOExcepti throw new IllegalArgumentException("Unexpected token " + token); } } + if (Assertions.ENABLED && Version.indexCreated(builder.settings).onOrAfter(Version.V_7_0_0_alpha1)) { + assert mappingVersion : "mapping version should be present for indices created on or after 7.0.0"; + } return builder.build(); } } From e7cdfd26bf489527bd6a1291c549b48b3b71e1b0 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sat, 25 Aug 2018 17:01:58 -0400 Subject: [PATCH 06/11] Fix toString --- .../src/main/java/org/elasticsearch/cluster/ClusterState.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/ClusterState.java b/server/src/main/java/org/elasticsearch/cluster/ClusterState.java index 8f3b99e5d2ffb..f7606d4bb061f 100644 --- a/server/src/main/java/org/elasticsearch/cluster/ClusterState.java +++ b/server/src/main/java/org/elasticsearch/cluster/ClusterState.java @@ -284,8 +284,7 @@ public String toString() { final String TAB = " "; for (IndexMetaData indexMetaData : metaData) { sb.append(TAB).append(indexMetaData.getIndex()); - sb.append(": v[").append(indexMetaData.getVersion()).append("]\n"); - sb.append(": mv[").append(indexMetaData.getMappingVersion()).append("]\n"); + sb.append(": v[").append(indexMetaData.getVersion()).append("], mv[").append(indexMetaData.getMappingVersion()).append("]\n"); for (int shard = 0; shard < indexMetaData.getNumberOfShards(); shard++) { sb.append(TAB).append(TAB).append(shard).append(": "); sb.append("p_term [").append(indexMetaData.primaryTerm(shard)).append("], "); From 0a6f7c9f3ba38cf1417b84e5adc663b4cd49f706 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sun, 26 Aug 2018 12:42:01 -0400 Subject: [PATCH 07/11] Add assertion --- .../org/elasticsearch/index/IndexService.java | 4 +- .../index/mapper/MapperService.java | 48 +++++++++++++++++-- .../cluster/IndicesClusterStateService.java | 6 +-- ...actIndicesClusterStateServiceTestCase.java | 2 +- 4 files changed, 50 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/IndexService.java b/server/src/main/java/org/elasticsearch/index/IndexService.java index 5e9e811bc32ec..6ffbc44676e0b 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexService.java +++ b/server/src/main/java/org/elasticsearch/index/IndexService.java @@ -522,8 +522,8 @@ List getSearchOperationListener() { // pkg private for } @Override - public boolean updateMapping(IndexMetaData indexMetaData) throws IOException { - return mapperService().updateMapping(indexMetaData); + public boolean updateMapping(final IndexMetaData currentIndexMetaData, final IndexMetaData newIndexMetaData) throws IOException { + return mapperService().updateMapping(currentIndexMetaData, newIndexMetaData); } private class StoreCloseListener implements Store.OnClose { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java index 9cd8ef1f6ac67..3d3257bde0e14 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java @@ -25,6 +25,7 @@ import org.apache.lucene.analysis.Analyzer; import org.apache.lucene.analysis.DelegatingAnalyzerWrapper; import org.apache.lucene.index.Term; +import org.elasticsearch.Assertions; import org.elasticsearch.ElasticsearchGenerationException; import org.elasticsearch.Version; import org.elasticsearch.cluster.metadata.IndexMetaData; @@ -192,8 +193,8 @@ public static Map parseMapping(NamedXContentRegistry xContentReg /** * Update mapping by only merging the metadata that is different between received and stored entries */ - public boolean updateMapping(IndexMetaData indexMetaData) throws IOException { - assert indexMetaData.getIndex().equals(index()) : "index mismatch: expected " + index() + " but was " + indexMetaData.getIndex(); + public boolean updateMapping(final IndexMetaData currentIndexMetaData, final IndexMetaData newIndexMetaData) throws IOException { + assert newIndexMetaData.getIndex().equals(index()) : "index mismatch: expected " + index() + " but was " + newIndexMetaData.getIndex(); // go over and add the relevant mappings (or update them) Set existingMappers = new HashSet<>(); if (mapper != null) { @@ -205,7 +206,7 @@ public boolean updateMapping(IndexMetaData indexMetaData) throws IOException { final Map updatedEntries; try { // only update entries if needed - updatedEntries = internalMerge(indexMetaData, MergeReason.MAPPING_RECOVERY, true); + updatedEntries = internalMerge(newIndexMetaData, MergeReason.MAPPING_RECOVERY, true); } catch (Exception e) { logger.warn(() -> new ParameterizedMessage("[{}] failed to apply mappings", index()), e); throw e; @@ -213,9 +214,11 @@ public boolean updateMapping(IndexMetaData indexMetaData) throws IOException { boolean requireRefresh = false; + assertMappingVersion(currentIndexMetaData, newIndexMetaData, updatedEntries); + for (DocumentMapper documentMapper : updatedEntries.values()) { String mappingType = documentMapper.type(); - CompressedXContent incomingMappingSource = indexMetaData.mapping(mappingType).source(); + CompressedXContent incomingMappingSource = newIndexMetaData.mapping(mappingType).source(); String op = existingMappers.contains(mappingType) ? "updated" : "added"; if (logger.isDebugEnabled() && incomingMappingSource.compressed().length < 512) { @@ -240,6 +243,43 @@ public boolean updateMapping(IndexMetaData indexMetaData) throws IOException { return requireRefresh; } + private void assertMappingVersion( + final IndexMetaData currentIndexMetaData, + final IndexMetaData newIndexMetaData, + final Map updatedEntries) { + if (Assertions.ENABLED && currentIndexMetaData != null) { + if (currentIndexMetaData.getMappingVersion() == newIndexMetaData.getMappingVersion()) { + // if the mapping version is unchanged, then there should not be any updates and all mappings should be the same + assert updatedEntries.isEmpty() : updatedEntries; + for (final ObjectCursor mapping : newIndexMetaData.getMappings().values()) { + final CompressedXContent currentSource = currentIndexMetaData.mapping(mapping.value.type()).source(); + final CompressedXContent newSource = mapping.value.source(); + assert currentSource.equals(newSource) : + "expected current mapping [" + currentSource + "] for type [" + mapping.value.type() + "] " + + "to be the same as new mapping [" + newSource + "]"; + } + } else { + // if the mapping version is changed, it should increase, there should be updates, and the mapping should be different + final long currentMappingVersion = currentIndexMetaData.getMappingVersion(); + final long newMappingVersion = newIndexMetaData.getMappingVersion(); + assert currentMappingVersion < newMappingVersion : + "expected current mapping version [" + currentMappingVersion + "] " + + "to be less than new mapping version [" + newMappingVersion + "]"; + assert updatedEntries.isEmpty() == false; + for (final DocumentMapper documentMapper : updatedEntries.values()) { + final MappingMetaData currentMapping = currentIndexMetaData.mapping(documentMapper.type()); + if (currentMapping != null) { + final CompressedXContent currentSource = currentMapping.source(); + final CompressedXContent newSource = documentMapper.mappingSource(); + assert currentSource.equals(newSource) == false : + "expected current mapping [" + currentSource + "] for type [" + documentMapper.type() + "] " + + "to be different than new mapping"; + } + } + } + } + } + public void merge(Map> mappings, MergeReason reason) { Map mappingSourcesCompressed = new LinkedHashMap<>(mappings.size()); for (Map.Entry> entry : mappings.entrySet()) { diff --git a/server/src/main/java/org/elasticsearch/indices/cluster/IndicesClusterStateService.java b/server/src/main/java/org/elasticsearch/indices/cluster/IndicesClusterStateService.java index e6a86d47f55c0..692010119dc2d 100644 --- a/server/src/main/java/org/elasticsearch/indices/cluster/IndicesClusterStateService.java +++ b/server/src/main/java/org/elasticsearch/indices/cluster/IndicesClusterStateService.java @@ -456,7 +456,7 @@ private void createIndices(final ClusterState state) { AllocatedIndex indexService = null; try { indexService = indicesService.createIndex(indexMetaData, buildInIndexListener); - if (indexService.updateMapping(indexMetaData) && sendRefreshMapping) { + if (indexService.updateMapping(null, indexMetaData) && sendRefreshMapping) { nodeMappingRefreshAction.nodeMappingRefresh(state.nodes().getMasterNode(), new NodeMappingRefreshAction.NodeMappingRefreshRequest(indexMetaData.getIndex().getName(), indexMetaData.getIndexUUID(), state.nodes().getLocalNodeId()) @@ -490,7 +490,7 @@ private void updateIndices(ClusterChangedEvent event) { if (ClusterChangedEvent.indexMetaDataChanged(currentIndexMetaData, newIndexMetaData)) { indexService.updateMetaData(newIndexMetaData); try { - if (indexService.updateMapping(newIndexMetaData) && sendRefreshMapping) { + if (indexService.updateMapping(currentIndexMetaData, newIndexMetaData) && sendRefreshMapping) { nodeMappingRefreshAction.nodeMappingRefresh(state.nodes().getMasterNode(), new NodeMappingRefreshAction.NodeMappingRefreshRequest(newIndexMetaData.getIndex().getName(), newIndexMetaData.getIndexUUID(), state.nodes().getLocalNodeId()) @@ -778,7 +778,7 @@ public interface AllocatedIndex extends Iterable, IndexCompo /** * Checks if index requires refresh from master. */ - boolean updateMapping(IndexMetaData indexMetaData) throws IOException; + boolean updateMapping(IndexMetaData currentIndexMetaData, IndexMetaData newIndexMetaData) throws IOException; /** * Returns shard with given id. diff --git a/server/src/test/java/org/elasticsearch/indices/cluster/AbstractIndicesClusterStateServiceTestCase.java b/server/src/test/java/org/elasticsearch/indices/cluster/AbstractIndicesClusterStateServiceTestCase.java index 580696264bdd4..c68e4870aaeb0 100644 --- a/server/src/test/java/org/elasticsearch/indices/cluster/AbstractIndicesClusterStateServiceTestCase.java +++ b/server/src/test/java/org/elasticsearch/indices/cluster/AbstractIndicesClusterStateServiceTestCase.java @@ -273,7 +273,7 @@ public IndexSettings getIndexSettings() { } @Override - public boolean updateMapping(IndexMetaData indexMetaData) throws IOException { + public boolean updateMapping(final IndexMetaData currentIndexMetaData, final IndexMetaData newIndexMetaData) throws IOException { failRandomly(); return false; } From 7aabd212ac55cda8344ce4ada5a25f14a57f8dbc Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sun, 26 Aug 2018 21:30:34 -0400 Subject: [PATCH 08/11] Tracking mapping updates per index --- .../cluster/metadata/MetaDataMappingService.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java index b021ec701742e..616fd13d1fadc 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java @@ -287,6 +287,7 @@ private ClusterState applyRequest(ClusterState currentState, PutMappingClusterSt MetaData.Builder builder = MetaData.builder(metaData); boolean updated = false; for (IndexMetaData indexMetaData : updateList) { + boolean updatedMapping = false; // do the actual merge here on the master, and update the mapping source // we use the exact same indexService and metadata we used to validate above here to actually apply the update final Index index = indexMetaData.getIndex(); @@ -303,7 +304,7 @@ private ClusterState applyRequest(ClusterState currentState, PutMappingClusterSt if (existingSource.equals(updatedSource)) { // same source, no changes, ignore it } else { - updated = true; + updatedMapping = true; // use the merged mapping source if (logger.isDebugEnabled()) { logger.debug("{} update_mapping [{}] with source [{}]", index, mergedMapper.type(), updatedSource); @@ -313,7 +314,7 @@ private ClusterState applyRequest(ClusterState currentState, PutMappingClusterSt } } else { - updated = true; + updatedMapping = true; if (logger.isDebugEnabled()) { logger.debug("{} create_mapping [{}] with source [{}]", index, mappingType, updatedSource); } else if (logger.isInfoEnabled()) { @@ -329,7 +330,7 @@ private ClusterState applyRequest(ClusterState currentState, PutMappingClusterSt indexMetaDataBuilder.putMapping(new MappingMetaData(mapper.mappingSource())); } } - if (updated) { + if (updatedMapping) { indexMetaDataBuilder.mappingVersion(1 + indexMetaDataBuilder.mappingVersion()); } /* @@ -338,6 +339,7 @@ private ClusterState applyRequest(ClusterState currentState, PutMappingClusterSt * statement. */ builder.put(indexMetaDataBuilder); + updated |= updatedMapping; } if (updated) { return ClusterState.builder(currentState).metaData(builder).build(); From 4ae4bd2077ffa42abe52bc93673b3e2c60acae00 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Mon, 27 Aug 2018 08:48:33 -0400 Subject: [PATCH 09/11] Disable assertions sometimes --- qa/mixed-cluster/build.gradle | 1 + qa/rolling-upgrade/build.gradle | 2 + .../index/mapper/MapperService.java | 39 +-------- .../index/mapper/MapperServiceAssertions.java | 86 +++++++++++++++++++ x-pack/qa/rolling-upgrade-basic/build.gradle | 2 + x-pack/qa/rolling-upgrade/build.gradle | 2 + 6 files changed, 94 insertions(+), 38 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/index/mapper/MapperServiceAssertions.java diff --git a/qa/mixed-cluster/build.gradle b/qa/mixed-cluster/build.gradle index ac57d51def7c6..4c1e66cbec531 100644 --- a/qa/mixed-cluster/build.gradle +++ b/qa/mixed-cluster/build.gradle @@ -44,6 +44,7 @@ for (Version version : bwcVersions.wireCompatible) { numNodes = 4 numBwcNodes = 2 bwcVersion = version + jvmArgs += ' -da:org.elasticsearch.index.mapper.MapperAssertions' } Task versionBwcTest = tasks.create(name: "${baseName}#bwcTest") { diff --git a/qa/rolling-upgrade/build.gradle b/qa/rolling-upgrade/build.gradle index bfd37863cc246..b29b5d00f77d4 100644 --- a/qa/rolling-upgrade/build.gradle +++ b/qa/rolling-upgrade/build.gradle @@ -61,6 +61,7 @@ for (Version version : bwcVersions.wireCompatible) { bwcVersion = version numBwcNodes = 3 numNodes = 3 + jvmArgs += ' -da:org.elasticsearch.index.mapper.MapperAssertions' clusterName = 'rolling-upgrade' setting 'repositories.url.allowed_urls', 'http://snapshot.test*' if (version.onOrAfter('5.3.0')) { @@ -76,6 +77,7 @@ for (Version version : bwcVersions.wireCompatible) { Closure configureUpgradeCluster = {String name, Task lastRunner, int stopNode, Closure unicastSeed -> configure(extensions.findByName("${baseName}#${name}")) { dependsOn lastRunner, "${baseName}#oldClusterTestCluster#node${stopNode}.stop" + jvmArgs += ' -da:org.elasticsearch.index.mapper.MapperAssertions' clusterName = 'rolling-upgrade' unicastTransportUri = { seedNode, node, ant -> unicastSeed() } minimumMasterNodes = { 3 } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java index 43d3352b39431..9fdcb4ce23c1f 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java @@ -25,7 +25,6 @@ import org.apache.lucene.analysis.Analyzer; import org.apache.lucene.analysis.DelegatingAnalyzerWrapper; import org.apache.lucene.index.Term; -import org.elasticsearch.Assertions; import org.elasticsearch.ElasticsearchGenerationException; import org.elasticsearch.Version; import org.elasticsearch.cluster.metadata.IndexMetaData; @@ -73,6 +72,7 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.unmodifiableMap; +import static org.elasticsearch.index.mapper.MapperServiceAssertions.assertMappingVersion; public class MapperService extends AbstractIndexComponent implements Closeable { @@ -243,43 +243,6 @@ public boolean updateMapping(final IndexMetaData currentIndexMetaData, final Ind return requireRefresh; } - private void assertMappingVersion( - final IndexMetaData currentIndexMetaData, - final IndexMetaData newIndexMetaData, - final Map updatedEntries) { - if (Assertions.ENABLED && currentIndexMetaData != null) { - if (currentIndexMetaData.getMappingVersion() == newIndexMetaData.getMappingVersion()) { - // if the mapping version is unchanged, then there should not be any updates and all mappings should be the same - assert updatedEntries.isEmpty() : updatedEntries; - for (final ObjectCursor mapping : newIndexMetaData.getMappings().values()) { - final CompressedXContent currentSource = currentIndexMetaData.mapping(mapping.value.type()).source(); - final CompressedXContent newSource = mapping.value.source(); - assert currentSource.equals(newSource) : - "expected current mapping [" + currentSource + "] for type [" + mapping.value.type() + "] " - + "to be the same as new mapping [" + newSource + "]"; - } - } else { - // if the mapping version is changed, it should increase, there should be updates, and the mapping should be different - final long currentMappingVersion = currentIndexMetaData.getMappingVersion(); - final long newMappingVersion = newIndexMetaData.getMappingVersion(); - assert currentMappingVersion < newMappingVersion : - "expected current mapping version [" + currentMappingVersion + "] " - + "to be less than new mapping version [" + newMappingVersion + "]"; - assert updatedEntries.isEmpty() == false; - for (final DocumentMapper documentMapper : updatedEntries.values()) { - final MappingMetaData currentMapping = currentIndexMetaData.mapping(documentMapper.type()); - if (currentMapping != null) { - final CompressedXContent currentSource = currentMapping.source(); - final CompressedXContent newSource = documentMapper.mappingSource(); - assert currentSource.equals(newSource) == false : - "expected current mapping [" + currentSource + "] for type [" + documentMapper.type() + "] " + - "to be different than new mapping"; - } - } - } - } - } - public void merge(Map> mappings, MergeReason reason) { Map mappingSourcesCompressed = new LinkedHashMap<>(mappings.size()); for (Map.Entry> entry : mappings.entrySet()) { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperServiceAssertions.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperServiceAssertions.java new file mode 100644 index 0000000000000..020fae0d990ba --- /dev/null +++ b/server/src/main/java/org/elasticsearch/index/mapper/MapperServiceAssertions.java @@ -0,0 +1,86 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.index.mapper; + +import com.carrotsearch.hppc.cursors.ObjectCursor; +import org.elasticsearch.Assertions; +import org.elasticsearch.cluster.metadata.IndexMetaData; +import org.elasticsearch.cluster.metadata.MappingMetaData; +import org.elasticsearch.common.compress.CompressedXContent; + +import java.util.Map; + +/** + * This class exists so that we can disable these assertions in a mixed-version cluster without disabling all assertions in + * {@link MapperService}. These assertions can not necessarily hold in a mixed-version cluster because older nodes will not be serializing + * mapping version. + */ +// TODO: this indirection can be removed when all nodes in a mixed-version cluster test understand mapping version +final class MapperServiceAssertions { + + private MapperServiceAssertions() { + + } + + /** + * Assertions regarding changes in the mapping version. + * + * @param currentIndexMetaData the current index metadata + * @param newIndexMetaData the new index metadata + * @param updatedEntries the updated document mappers + */ + static void assertMappingVersion( + final IndexMetaData currentIndexMetaData, + final IndexMetaData newIndexMetaData, + final Map updatedEntries) { + if (Assertions.ENABLED && currentIndexMetaData != null) { + if (currentIndexMetaData.getMappingVersion() == newIndexMetaData.getMappingVersion()) { + // if the mapping version is unchanged, then there should not be any updates and all mappings should be the same + assert updatedEntries.isEmpty() : updatedEntries; + for (final ObjectCursor mapping : newIndexMetaData.getMappings().values()) { + final CompressedXContent currentSource = currentIndexMetaData.mapping(mapping.value.type()).source(); + final CompressedXContent newSource = mapping.value.source(); + assert currentSource.equals(newSource) : + "expected current mapping [" + currentSource + "] for type [" + mapping.value.type() + "] " + + "to be the same as new mapping [" + newSource + "]"; + } + } else { + // if the mapping version is changed, it should increase, there should be updates, and the mapping should be different + final long currentMappingVersion = currentIndexMetaData.getMappingVersion(); + final long newMappingVersion = newIndexMetaData.getMappingVersion(); + assert currentMappingVersion < newMappingVersion : + "expected current mapping version [" + currentMappingVersion + "] " + + "to be less than new mapping version [" + newMappingVersion + "]"; + assert updatedEntries.isEmpty() == false; + for (final DocumentMapper documentMapper : updatedEntries.values()) { + final MappingMetaData currentMapping = currentIndexMetaData.mapping(documentMapper.type()); + if (currentMapping != null) { + final CompressedXContent currentSource = currentMapping.source(); + final CompressedXContent newSource = documentMapper.mappingSource(); + assert currentSource.equals(newSource) == false : + "expected current mapping [" + currentSource + "] for type [" + documentMapper.type() + "] " + + "to be different than new mapping"; + } + } + } + } + } + +} diff --git a/x-pack/qa/rolling-upgrade-basic/build.gradle b/x-pack/qa/rolling-upgrade-basic/build.gradle index 5774e5d78561d..c4730c6c70570 100644 --- a/x-pack/qa/rolling-upgrade-basic/build.gradle +++ b/x-pack/qa/rolling-upgrade-basic/build.gradle @@ -36,6 +36,7 @@ for (Version version : bwcVersions.wireCompatible) { numBwcNodes = 3 numNodes = 3 minimumMasterNodes = { 3 } + jvmArgs += ' -da:org.elasticsearch.index.mapper.MapperAssertions' clusterName = 'rolling-upgrade-basic' setting 'xpack.security.enabled', 'false' setting 'xpack.monitoring.enabled', 'false' @@ -52,6 +53,7 @@ for (Version version : bwcVersions.wireCompatible) { Closure configureUpgradeCluster = {String name, Task lastRunner, int stopNode, Closure unicastSeed -> configure(extensions.findByName("${baseName}#${name}")) { dependsOn lastRunner, "${baseName}#oldClusterTestCluster#node${stopNode}.stop" + jvmArgs += ' -da:org.elasticsearch.index.mapper.MapperAssertions' clusterName = 'rolling-upgrade-basic' unicastTransportUri = { seedNode, node, ant -> unicastSeed() } minimumMasterNodes = { 3 } diff --git a/x-pack/qa/rolling-upgrade/build.gradle b/x-pack/qa/rolling-upgrade/build.gradle index 548081a893881..2158db95ec573 100644 --- a/x-pack/qa/rolling-upgrade/build.gradle +++ b/x-pack/qa/rolling-upgrade/build.gradle @@ -132,6 +132,7 @@ subprojects { numBwcNodes = 3 numNodes = 3 minimumMasterNodes = { 3 } + jvmArgs += ' -da:org.elasticsearch.index.mapper.MapperAssertions' clusterName = 'rolling-upgrade' waitCondition = waitWithAuth setting 'xpack.monitoring.exporters._http.type', 'http' @@ -172,6 +173,7 @@ subprojects { configure(extensions.findByName("${baseName}#${name}")) { dependsOn lastRunner, "${baseName}#oldClusterTestCluster#node${stopNode}.stop" setupCommand 'setupTestUser', 'bin/elasticsearch-users', 'useradd', 'test_user', '-p', 'x-pack-test-password', '-r', 'superuser' + jvmArgs += ' -da:org.elasticsearch.index.mapper.MapperAssertions' clusterName = 'rolling-upgrade' unicastTransportUri = { seedNode, node, ant -> unicastSeed() } minimumMasterNodes = { 3 } From e7630049feb4d32d439b6c5bcc8380b03346030f Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Mon, 27 Aug 2018 09:16:47 -0400 Subject: [PATCH 10/11] Revert "Disable assertions sometimes" This reverts commit 4ae4bd2077ffa42abe52bc93673b3e2c60acae00. --- qa/mixed-cluster/build.gradle | 1 - qa/rolling-upgrade/build.gradle | 2 - .../index/mapper/MapperService.java | 39 ++++++++- .../index/mapper/MapperServiceAssertions.java | 86 ------------------- x-pack/qa/rolling-upgrade-basic/build.gradle | 2 - x-pack/qa/rolling-upgrade/build.gradle | 2 - 6 files changed, 38 insertions(+), 94 deletions(-) delete mode 100644 server/src/main/java/org/elasticsearch/index/mapper/MapperServiceAssertions.java diff --git a/qa/mixed-cluster/build.gradle b/qa/mixed-cluster/build.gradle index 4c1e66cbec531..ac57d51def7c6 100644 --- a/qa/mixed-cluster/build.gradle +++ b/qa/mixed-cluster/build.gradle @@ -44,7 +44,6 @@ for (Version version : bwcVersions.wireCompatible) { numNodes = 4 numBwcNodes = 2 bwcVersion = version - jvmArgs += ' -da:org.elasticsearch.index.mapper.MapperAssertions' } Task versionBwcTest = tasks.create(name: "${baseName}#bwcTest") { diff --git a/qa/rolling-upgrade/build.gradle b/qa/rolling-upgrade/build.gradle index b29b5d00f77d4..bfd37863cc246 100644 --- a/qa/rolling-upgrade/build.gradle +++ b/qa/rolling-upgrade/build.gradle @@ -61,7 +61,6 @@ for (Version version : bwcVersions.wireCompatible) { bwcVersion = version numBwcNodes = 3 numNodes = 3 - jvmArgs += ' -da:org.elasticsearch.index.mapper.MapperAssertions' clusterName = 'rolling-upgrade' setting 'repositories.url.allowed_urls', 'http://snapshot.test*' if (version.onOrAfter('5.3.0')) { @@ -77,7 +76,6 @@ for (Version version : bwcVersions.wireCompatible) { Closure configureUpgradeCluster = {String name, Task lastRunner, int stopNode, Closure unicastSeed -> configure(extensions.findByName("${baseName}#${name}")) { dependsOn lastRunner, "${baseName}#oldClusterTestCluster#node${stopNode}.stop" - jvmArgs += ' -da:org.elasticsearch.index.mapper.MapperAssertions' clusterName = 'rolling-upgrade' unicastTransportUri = { seedNode, node, ant -> unicastSeed() } minimumMasterNodes = { 3 } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java index 9fdcb4ce23c1f..43d3352b39431 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java @@ -25,6 +25,7 @@ import org.apache.lucene.analysis.Analyzer; import org.apache.lucene.analysis.DelegatingAnalyzerWrapper; import org.apache.lucene.index.Term; +import org.elasticsearch.Assertions; import org.elasticsearch.ElasticsearchGenerationException; import org.elasticsearch.Version; import org.elasticsearch.cluster.metadata.IndexMetaData; @@ -72,7 +73,6 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.unmodifiableMap; -import static org.elasticsearch.index.mapper.MapperServiceAssertions.assertMappingVersion; public class MapperService extends AbstractIndexComponent implements Closeable { @@ -243,6 +243,43 @@ public boolean updateMapping(final IndexMetaData currentIndexMetaData, final Ind return requireRefresh; } + private void assertMappingVersion( + final IndexMetaData currentIndexMetaData, + final IndexMetaData newIndexMetaData, + final Map updatedEntries) { + if (Assertions.ENABLED && currentIndexMetaData != null) { + if (currentIndexMetaData.getMappingVersion() == newIndexMetaData.getMappingVersion()) { + // if the mapping version is unchanged, then there should not be any updates and all mappings should be the same + assert updatedEntries.isEmpty() : updatedEntries; + for (final ObjectCursor mapping : newIndexMetaData.getMappings().values()) { + final CompressedXContent currentSource = currentIndexMetaData.mapping(mapping.value.type()).source(); + final CompressedXContent newSource = mapping.value.source(); + assert currentSource.equals(newSource) : + "expected current mapping [" + currentSource + "] for type [" + mapping.value.type() + "] " + + "to be the same as new mapping [" + newSource + "]"; + } + } else { + // if the mapping version is changed, it should increase, there should be updates, and the mapping should be different + final long currentMappingVersion = currentIndexMetaData.getMappingVersion(); + final long newMappingVersion = newIndexMetaData.getMappingVersion(); + assert currentMappingVersion < newMappingVersion : + "expected current mapping version [" + currentMappingVersion + "] " + + "to be less than new mapping version [" + newMappingVersion + "]"; + assert updatedEntries.isEmpty() == false; + for (final DocumentMapper documentMapper : updatedEntries.values()) { + final MappingMetaData currentMapping = currentIndexMetaData.mapping(documentMapper.type()); + if (currentMapping != null) { + final CompressedXContent currentSource = currentMapping.source(); + final CompressedXContent newSource = documentMapper.mappingSource(); + assert currentSource.equals(newSource) == false : + "expected current mapping [" + currentSource + "] for type [" + documentMapper.type() + "] " + + "to be different than new mapping"; + } + } + } + } + } + public void merge(Map> mappings, MergeReason reason) { Map mappingSourcesCompressed = new LinkedHashMap<>(mappings.size()); for (Map.Entry> entry : mappings.entrySet()) { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperServiceAssertions.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperServiceAssertions.java deleted file mode 100644 index 020fae0d990ba..0000000000000 --- a/server/src/main/java/org/elasticsearch/index/mapper/MapperServiceAssertions.java +++ /dev/null @@ -1,86 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.index.mapper; - -import com.carrotsearch.hppc.cursors.ObjectCursor; -import org.elasticsearch.Assertions; -import org.elasticsearch.cluster.metadata.IndexMetaData; -import org.elasticsearch.cluster.metadata.MappingMetaData; -import org.elasticsearch.common.compress.CompressedXContent; - -import java.util.Map; - -/** - * This class exists so that we can disable these assertions in a mixed-version cluster without disabling all assertions in - * {@link MapperService}. These assertions can not necessarily hold in a mixed-version cluster because older nodes will not be serializing - * mapping version. - */ -// TODO: this indirection can be removed when all nodes in a mixed-version cluster test understand mapping version -final class MapperServiceAssertions { - - private MapperServiceAssertions() { - - } - - /** - * Assertions regarding changes in the mapping version. - * - * @param currentIndexMetaData the current index metadata - * @param newIndexMetaData the new index metadata - * @param updatedEntries the updated document mappers - */ - static void assertMappingVersion( - final IndexMetaData currentIndexMetaData, - final IndexMetaData newIndexMetaData, - final Map updatedEntries) { - if (Assertions.ENABLED && currentIndexMetaData != null) { - if (currentIndexMetaData.getMappingVersion() == newIndexMetaData.getMappingVersion()) { - // if the mapping version is unchanged, then there should not be any updates and all mappings should be the same - assert updatedEntries.isEmpty() : updatedEntries; - for (final ObjectCursor mapping : newIndexMetaData.getMappings().values()) { - final CompressedXContent currentSource = currentIndexMetaData.mapping(mapping.value.type()).source(); - final CompressedXContent newSource = mapping.value.source(); - assert currentSource.equals(newSource) : - "expected current mapping [" + currentSource + "] for type [" + mapping.value.type() + "] " - + "to be the same as new mapping [" + newSource + "]"; - } - } else { - // if the mapping version is changed, it should increase, there should be updates, and the mapping should be different - final long currentMappingVersion = currentIndexMetaData.getMappingVersion(); - final long newMappingVersion = newIndexMetaData.getMappingVersion(); - assert currentMappingVersion < newMappingVersion : - "expected current mapping version [" + currentMappingVersion + "] " - + "to be less than new mapping version [" + newMappingVersion + "]"; - assert updatedEntries.isEmpty() == false; - for (final DocumentMapper documentMapper : updatedEntries.values()) { - final MappingMetaData currentMapping = currentIndexMetaData.mapping(documentMapper.type()); - if (currentMapping != null) { - final CompressedXContent currentSource = currentMapping.source(); - final CompressedXContent newSource = documentMapper.mappingSource(); - assert currentSource.equals(newSource) == false : - "expected current mapping [" + currentSource + "] for type [" + documentMapper.type() + "] " + - "to be different than new mapping"; - } - } - } - } - } - -} diff --git a/x-pack/qa/rolling-upgrade-basic/build.gradle b/x-pack/qa/rolling-upgrade-basic/build.gradle index c4730c6c70570..5774e5d78561d 100644 --- a/x-pack/qa/rolling-upgrade-basic/build.gradle +++ b/x-pack/qa/rolling-upgrade-basic/build.gradle @@ -36,7 +36,6 @@ for (Version version : bwcVersions.wireCompatible) { numBwcNodes = 3 numNodes = 3 minimumMasterNodes = { 3 } - jvmArgs += ' -da:org.elasticsearch.index.mapper.MapperAssertions' clusterName = 'rolling-upgrade-basic' setting 'xpack.security.enabled', 'false' setting 'xpack.monitoring.enabled', 'false' @@ -53,7 +52,6 @@ for (Version version : bwcVersions.wireCompatible) { Closure configureUpgradeCluster = {String name, Task lastRunner, int stopNode, Closure unicastSeed -> configure(extensions.findByName("${baseName}#${name}")) { dependsOn lastRunner, "${baseName}#oldClusterTestCluster#node${stopNode}.stop" - jvmArgs += ' -da:org.elasticsearch.index.mapper.MapperAssertions' clusterName = 'rolling-upgrade-basic' unicastTransportUri = { seedNode, node, ant -> unicastSeed() } minimumMasterNodes = { 3 } diff --git a/x-pack/qa/rolling-upgrade/build.gradle b/x-pack/qa/rolling-upgrade/build.gradle index 2158db95ec573..548081a893881 100644 --- a/x-pack/qa/rolling-upgrade/build.gradle +++ b/x-pack/qa/rolling-upgrade/build.gradle @@ -132,7 +132,6 @@ subprojects { numBwcNodes = 3 numNodes = 3 minimumMasterNodes = { 3 } - jvmArgs += ' -da:org.elasticsearch.index.mapper.MapperAssertions' clusterName = 'rolling-upgrade' waitCondition = waitWithAuth setting 'xpack.monitoring.exporters._http.type', 'http' @@ -173,7 +172,6 @@ subprojects { configure(extensions.findByName("${baseName}#${name}")) { dependsOn lastRunner, "${baseName}#oldClusterTestCluster#node${stopNode}.stop" setupCommand 'setupTestUser', 'bin/elasticsearch-users', 'useradd', 'test_user', '-p', 'x-pack-test-password', '-r', 'superuser' - jvmArgs += ' -da:org.elasticsearch.index.mapper.MapperAssertions' clusterName = 'rolling-upgrade' unicastTransportUri = { seedNode, node, ant -> unicastSeed() } minimumMasterNodes = { 3 } From b577264ca4597d46a254aba3104a899f0c22dbe2 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Mon, 27 Aug 2018 09:26:23 -0400 Subject: [PATCH 11/11] Disable assertions based on index version --- .../java/org/elasticsearch/index/mapper/MapperService.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java index 43d3352b39431..5ebfc5bb51e7e 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java @@ -247,7 +247,9 @@ private void assertMappingVersion( final IndexMetaData currentIndexMetaData, final IndexMetaData newIndexMetaData, final Map updatedEntries) { - if (Assertions.ENABLED && currentIndexMetaData != null) { + if (Assertions.ENABLED + && currentIndexMetaData != null + && currentIndexMetaData.getCreationVersion().onOrAfter(Version.V_7_0_0_alpha1)) { if (currentIndexMetaData.getMappingVersion() == newIndexMetaData.getMappingVersion()) { // if the mapping version is unchanged, then there should not be any updates and all mappings should be the same assert updatedEntries.isEmpty() : updatedEntries;