diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/mget/40_routing.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/mget/40_routing.yml index 0643982156b93..45f107d0ef1df 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/mget/40_routing.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/mget/40_routing.yml @@ -1,29 +1,27 @@ --- -"Routing": +routing: + - do: + indices.create: + index: test_1 + body: + settings: + index: + number_of_shards: 5 + number_of_routing_shards: 5 + number_of_replicas: 0 - - - do: - indices.create: - index: test_1 - body: - settings: - index: - number_of_shards: 5 - number_of_routing_shards: 5 - number_of_replicas: 0 - - - do: + - do: cluster.health: wait_for_status: green - - do: + - do: index: index: test_1 id: 1 routing: "5" body: { foo: bar } - - do: + - do: mget: index: test_1 stored_fields: [_routing] @@ -33,10 +31,59 @@ - { _id: 1, routing: "4" } - { _id: 1, routing: "5" } - - is_false: docs.0.found - - is_false: docs.1.found + - is_false: docs.0.found + - is_false: docs.1.found + - is_true: docs.2.found + - match: { docs.2._index: test_1 } + - match: { docs.2._id: "1" } + - match: { docs.2._routing: "5" } + +--- +requires routing: + - skip: + version: " - 7.99.99" + reason: "fails with an unexpected message in 7.x" + + - do: + indices.create: + index: test_1 + body: + settings: + index: + number_of_shards: 5 + number_of_replicas: 0 + mappings: + _routing: + required: true + + - do: + index: + index: test_1 + id: 1 + routing: "5" + body: { foo: bar } + + - do: + indices.put_alias: + index: test_1 + name: alias + + - do: + mget: + stored_fields: [_routing] + body: + docs: + - { _id: 1, _index: test_1 } + - { _id: 1, _index: alias } + - { _id: 1, _index: test_1, routing: "5" } - - is_true: docs.2.found - - match: { docs.2._index: test_1 } - - match: { docs.2._id: "1" } - - match: { docs.2._routing: "5" } + - is_false: docs.0.found + - match: { docs.0.error.reason: "routing is required for [test_1]/[1]" } + - match: { docs.0._index: test_1 } + - is_false: docs.1.found + - match: { docs.1.error.reason: "routing is required for [test_1]/[1]" } + - match: { docs.1._index: test_1 } + - is_true: docs.2.found + - match: { docs.2._index: test_1 } + - match: { docs.2._id: "1" } + - match: { docs.2._routing: "5" } diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/mtermvectors/30_routing.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/mtermvectors/30_routing.yml new file mode 100644 index 0000000000000..36374cfa2daac --- /dev/null +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/mtermvectors/30_routing.yml @@ -0,0 +1,91 @@ +--- +routing: + - do: + indices.create: + index: test_1 + body: + settings: + index: + number_of_shards: 5 + number_of_replicas: 0 + + - do: + cluster.health: + wait_for_status: green + + - do: + index: + index: test_1 + id: 1 + routing: "5" + body: { foo: bar baz } + + - do: + mtermvectors: + index: test_1 + fields: foo + body: + docs: + - { _id: 1 } + - { _id: 1, routing: "4" } + - { _id: 1, routing: "5" } + + - is_false: docs.0.found + - is_false: docs.1.found + - is_true: docs.2.found + - match: { docs.2._index: test_1 } + - match: { docs.2._id: "1" } + - match: { docs.2.term_vectors.foo.terms.bar.term_freq: 1 } + - match: { docs.2.term_vectors.foo.terms.baz.term_freq: 1 } + + +--- +requires routing: + - skip: + version: " - 7.99.99" + reason: "fails with an unexpected message in 7.x" + + - do: + indices.create: + index: test_1 + body: + settings: + index: + number_of_shards: 5 + number_of_replicas: 0 + mappings: + _routing: + required: true + + - do: + index: + index: test_1 + id: 1 + routing: "5" + body: { foo: bar baz } + + - do: + indices.put_alias: + index: test_1 + name: alias + + - do: + mtermvectors: + fields: foo + body: + docs: + - { _id: 1, _index: test_1 } + - { _id: 1, _index: alias } + - { _id: 1, _index: test_1, routing: "5" } + + - is_false: docs.0.found + - match: { docs.0.error.reason: "routing is required for [test_1]/[1]" } + - match: { docs.0._index: test_1 } + - is_false: docs.1.found + - match: { docs.1.error.reason: "routing is required for [test_1]/[1]" } + - match: { docs.1._index: test_1 } + - is_true: docs.2.found + - match: { docs.2._index: test_1 } + - match: { docs.2._id: "1" } + - match: { docs.2.term_vectors.foo.terms.bar.term_freq: 1 } + - match: { docs.2.term_vectors.foo.terms.baz.term_freq: 1 } diff --git a/server/src/internalClusterTest/java/org/elasticsearch/action/bulk/BulkIntegrationIT.java b/server/src/internalClusterTest/java/org/elasticsearch/action/bulk/BulkIntegrationIT.java index ddce693f8e09c..cde1c2e19f794 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/action/bulk/BulkIntegrationIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/action/bulk/BulkIntegrationIT.java @@ -9,7 +9,6 @@ package org.elasticsearch.action.bulk; import org.elasticsearch.ElasticsearchException; -import org.elasticsearch.Version; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.admin.indices.alias.Alias; import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsResponse; @@ -114,7 +113,7 @@ public void testBulkWithWriteIndexAndRouting() { // allowing the auto-generated timestamp to externally be set would allow making the index inconsistent with duplicate docs public void testExternallySetAutoGeneratedTimestamp() { IndexRequest indexRequest = new IndexRequest("index1").source(Collections.singletonMap("foo", "baz")); - indexRequest.process(Version.CURRENT, null, null); // sets the timestamp + indexRequest.process(); // sets the timestamp if (randomBoolean()) { indexRequest.id("test"); } diff --git a/server/src/internalClusterTest/java/org/elasticsearch/routing/SimpleRoutingIT.java b/server/src/internalClusterTest/java/org/elasticsearch/routing/SimpleRoutingIT.java index 5806722ccc626..996639aaef4e6 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/routing/SimpleRoutingIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/routing/SimpleRoutingIT.java @@ -26,11 +26,9 @@ import org.elasticsearch.action.update.UpdateResponse; import org.elasticsearch.client.Requests; import org.elasticsearch.cluster.ClusterState; -import org.elasticsearch.cluster.routing.OperationRouting; -import org.elasticsearch.common.settings.ClusterSettings; -import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.cluster.metadata.IndexMetadata; +import org.elasticsearch.cluster.routing.IndexRouting; import org.elasticsearch.index.query.QueryBuilders; -import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.xcontent.XContentFactory; @@ -48,18 +46,17 @@ protected int minimumNumberOfShards() { } public String findNonMatchingRoutingValue(String index, String id) { - OperationRouting operationRouting = new OperationRouting( - Settings.EMPTY, - new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) - ); ClusterState state = client().admin().cluster().prepareState().all().get().getState(); + IndexMetadata metadata = state.metadata().index(index); + IndexMetadata withoutRoutingRequired = IndexMetadata.builder(metadata).putMapping("{}").build(); + IndexRouting indexRouting = IndexRouting.fromIndexMetadata(withoutRoutingRequired); int routing = -1; - ShardId idShard; - ShardId routingShard; + int idShard; + int routingShard; do { - idShard = operationRouting.shardId(state, index, id, null); - routingShard = operationRouting.shardId(state, index, id, Integer.toString(++routing)); - } while (idShard.getId() == routingShard.id()); + idShard = indexRouting.getShard(id, null); + routingShard = indexRouting.getShard(id, Integer.toString(++routing)); + } while (idShard == routingShard); return Integer.toString(routing); } diff --git a/server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java b/server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java index df1a6bac1b939..1043107d4e7b4 100644 --- a/server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java +++ b/server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java @@ -28,7 +28,6 @@ import org.elasticsearch.action.ingest.IngestActionForwarder; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.HandledTransportAction; -import org.elasticsearch.action.update.TransportUpdateAction; import org.elasticsearch.action.update.UpdateRequest; import org.elasticsearch.action.update.UpdateResponse; import org.elasticsearch.client.node.NodeClient; @@ -40,7 +39,6 @@ import org.elasticsearch.cluster.metadata.IndexAbstraction; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; -import org.elasticsearch.cluster.metadata.MappingMetadata; import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.cluster.routing.IndexRouting; import org.elasticsearch.cluster.service.ClusterService; @@ -534,11 +532,8 @@ protected void doRun() { prohibitAppendWritesInBackingIndices(docWriteRequest, metadata); prohibitCustomRoutingOnDataStream(docWriteRequest, metadata); IndexRequest indexRequest = (IndexRequest) docWriteRequest; - final IndexMetadata indexMetadata = metadata.index(concreteIndex); - MappingMetadata mappingMd = indexMetadata.mapping(); - Version indexCreated = indexMetadata.getCreationVersion(); indexRequest.resolveRouting(metadata); - indexRequest.process(indexCreated, mappingMd, concreteIndex.getName()); + indexRequest.process(); shardId = indexRouting.indexShard( docWriteRequest.id(), docWriteRequest.routing(), @@ -547,19 +542,11 @@ protected void doRun() { ); break; case UPDATE: - TransportUpdateAction.resolveAndValidateRouting( - metadata, - concreteIndex.getName(), - (UpdateRequest) docWriteRequest - ); + docWriteRequest.routing(metadata.resolveWriteIndexRouting(docWriteRequest.routing(), docWriteRequest.index())); shardId = indexRouting.updateShard(docWriteRequest.id(), docWriteRequest.routing()); break; case DELETE: docWriteRequest.routing(metadata.resolveWriteIndexRouting(docWriteRequest.routing(), docWriteRequest.index())); - // check if routing is required, if so, throw error if routing wasn't specified - if (docWriteRequest.routing() == null && metadata.routingRequired(concreteIndex.getName())) { - throw new RoutingMissingException(concreteIndex.getName(), docWriteRequest.id()); - } shardId = indexRouting.deleteShard(docWriteRequest.id(), docWriteRequest.routing()); break; default: diff --git a/server/src/main/java/org/elasticsearch/action/bulk/TransportShardBulkAction.java b/server/src/main/java/org/elasticsearch/action/bulk/TransportShardBulkAction.java index 8c03506e87a41..91cfe14fab7c4 100644 --- a/server/src/main/java/org/elasticsearch/action/bulk/TransportShardBulkAction.java +++ b/server/src/main/java/org/elasticsearch/action/bulk/TransportShardBulkAction.java @@ -32,8 +32,6 @@ import org.elasticsearch.cluster.ClusterStateObserver; import org.elasticsearch.cluster.action.index.MappingUpdatedAction; import org.elasticsearch.cluster.action.shard.ShardStateAction; -import org.elasticsearch.cluster.metadata.IndexMetadata; -import org.elasticsearch.cluster.metadata.MappingMetadata; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.compress.CompressedXContent; @@ -288,9 +286,7 @@ static boolean executeBulkItemRequest( case CREATED: case UPDATED: IndexRequest indexRequest = updateResult.action(); - IndexMetadata metadata = context.getPrimary().indexSettings().getIndexMetadata(); - MappingMetadata mappingMd = metadata.mapping(); - indexRequest.process(metadata.getCreationVersion(), mappingMd, updateRequest.concreteIndex()); + indexRequest.process(); context.setRequestToExecute(indexRequest); break; case DELETED: diff --git a/server/src/main/java/org/elasticsearch/action/explain/TransportExplainAction.java b/server/src/main/java/org/elasticsearch/action/explain/TransportExplainAction.java index 1440bbeb181c6..18efb47c8dbcd 100644 --- a/server/src/main/java/org/elasticsearch/action/explain/TransportExplainAction.java +++ b/server/src/main/java/org/elasticsearch/action/explain/TransportExplainAction.java @@ -11,7 +11,6 @@ import org.apache.lucene.search.Explanation; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.RoutingMissingException; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.single.shard.TransportSingleShardAction; import org.elasticsearch.cluster.ClusterState; @@ -85,10 +84,6 @@ protected void resolveRequest(ClusterState state, InternalRequest request) { final Set indicesAndAliases = indexNameExpressionResolver.resolveExpressions(state, request.request().index()); final AliasFilter aliasFilter = searchService.buildAliasFilter(state, request.concreteIndex(), indicesAndAliases); request.request().filteringAlias(aliasFilter); - // Fail fast on the node that received the request. - if (request.request().routing() == null && state.getMetadata().routingRequired(request.concreteIndex())) { - throw new RoutingMissingException(request.concreteIndex(), request.request().id()); - } } @Override diff --git a/server/src/main/java/org/elasticsearch/action/get/TransportGetAction.java b/server/src/main/java/org/elasticsearch/action/get/TransportGetAction.java index 477cbb278b508..d9caec599b8a7 100644 --- a/server/src/main/java/org/elasticsearch/action/get/TransportGetAction.java +++ b/server/src/main/java/org/elasticsearch/action/get/TransportGetAction.java @@ -9,7 +9,6 @@ package org.elasticsearch.action.get; import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.RoutingMissingException; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.single.shard.TransportSingleShardAction; import org.elasticsearch.cluster.ClusterState; @@ -82,10 +81,6 @@ protected ShardIterator shards(ClusterState state, InternalRequest request) { protected void resolveRequest(ClusterState state, InternalRequest request) { // update the routing (request#index here is possibly an alias) request.request().routing(state.metadata().resolveIndexRouting(request.request().routing(), request.request().index())); - // Fail fast on the node that received the request. - if (request.request().routing() == null && state.getMetadata().routingRequired(request.concreteIndex())) { - throw new RoutingMissingException(request.concreteIndex(), request.request().id()); - } } @Override diff --git a/server/src/main/java/org/elasticsearch/action/get/TransportMultiGetAction.java b/server/src/main/java/org/elasticsearch/action/get/TransportMultiGetAction.java index 7c3df21c301ff..da5748367fbd2 100644 --- a/server/src/main/java/org/elasticsearch/action/get/TransportMultiGetAction.java +++ b/server/src/main/java/org/elasticsearch/action/get/TransportMultiGetAction.java @@ -58,27 +58,21 @@ protected void doExecute(Task task, final MultiGetRequest request, final ActionL for (int i = 0; i < request.items.size(); i++) { MultiGetRequest.Item item = request.items.get(i); - String concreteSingleIndex; + ShardId shardId; try { - concreteSingleIndex = indexNameExpressionResolver.concreteSingleIndex(clusterState, item).getName(); - + String concreteSingleIndex = indexNameExpressionResolver.concreteSingleIndex(clusterState, item).getName(); item.routing(clusterState.metadata().resolveIndexRouting(item.routing(), item.index())); - if ((item.routing() == null) && (clusterState.getMetadata().routingRequired(concreteSingleIndex))) { - responses.set( - i, - newItemFailure(concreteSingleIndex, item.id(), new RoutingMissingException(concreteSingleIndex, item.id())) - ); - continue; - } + shardId = clusterService.operationRouting() + .getShards(clusterState, concreteSingleIndex, item.id(), item.routing(), null) + .shardId(); + } catch (RoutingMissingException e) { + responses.set(i, newItemFailure(e.getIndex().getName(), e.getId(), e)); + continue; } catch (Exception e) { responses.set(i, newItemFailure(item.index(), item.id(), e)); continue; } - ShardId shardId = clusterService.operationRouting() - .getShards(clusterState, concreteSingleIndex, item.id(), item.routing(), null) - .shardId(); - MultiGetShardRequest shardRequest = shardRequests.get(shardId); if (shardRequest == null) { shardRequest = new MultiGetShardRequest(request, shardId.getIndexName(), shardId.getId()); diff --git a/server/src/main/java/org/elasticsearch/action/index/IndexRequest.java b/server/src/main/java/org/elasticsearch/action/index/IndexRequest.java index 7a67697072376..9605844ce32ec 100644 --- a/server/src/main/java/org/elasticsearch/action/index/IndexRequest.java +++ b/server/src/main/java/org/elasticsearch/action/index/IndexRequest.java @@ -14,12 +14,10 @@ import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.CompositeIndicesRequest; import org.elasticsearch.action.DocWriteRequest; -import org.elasticsearch.action.RoutingMissingException; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.action.support.replication.ReplicatedWriteRequest; import org.elasticsearch.action.support.replication.ReplicationRequest; import org.elasticsearch.client.Requests; -import org.elasticsearch.cluster.metadata.MappingMetadata; import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.bytes.BytesArray; @@ -589,14 +587,7 @@ public VersionType versionType() { return this.versionType; } - public void process(Version indexCreatedVersion, @Nullable MappingMetadata mappingMd, String concreteIndex) { - if (mappingMd != null) { - // might as well check for routing here - if (mappingMd.routingRequired() && routing == null) { - throw new RoutingMissingException(concreteIndex, id); - } - } - + public void process() { if ("".equals(id)) { throw new IllegalArgumentException("if _id is specified it must not be empty"); } diff --git a/server/src/main/java/org/elasticsearch/action/termvectors/TransportMultiTermVectorsAction.java b/server/src/main/java/org/elasticsearch/action/termvectors/TransportMultiTermVectorsAction.java index 410522475fde6..2df42818cc0cd 100644 --- a/server/src/main/java/org/elasticsearch/action/termvectors/TransportMultiTermVectorsAction.java +++ b/server/src/main/java/org/elasticsearch/action/termvectors/TransportMultiTermVectorsAction.java @@ -58,26 +58,20 @@ protected void doExecute(Task task, final MultiTermVectorsRequest request, final Map shardRequests = new HashMap<>(); for (int i = 0; i < request.requests.size(); i++) { TermVectorsRequest termVectorsRequest = request.requests.get(i); - String concreteSingleIndex; + ShardId shardId; try { termVectorsRequest.routing( clusterState.metadata().resolveIndexRouting(termVectorsRequest.routing(), termVectorsRequest.index()) ); - concreteSingleIndex = indexNameExpressionResolver.concreteSingleIndex(clusterState, termVectorsRequest).getName(); - if (termVectorsRequest.routing() == null && clusterState.getMetadata().routingRequired(concreteSingleIndex)) { - responses.set( - i, - new MultiTermVectorsItemResponse( - null, - new MultiTermVectorsResponse.Failure( - concreteSingleIndex, - termVectorsRequest.id(), - new RoutingMissingException(concreteSingleIndex, termVectorsRequest.id()) - ) - ) - ); - continue; - } + String concreteSingleIndex = indexNameExpressionResolver.concreteSingleIndex(clusterState, termVectorsRequest).getName(); + shardId = clusterService.operationRouting() + .shardId(clusterState, concreteSingleIndex, termVectorsRequest.id(), termVectorsRequest.routing()); + } catch (RoutingMissingException e) { + responses.set( + i, + new MultiTermVectorsItemResponse(null, new MultiTermVectorsResponse.Failure(e.getIndex().getName(), e.getId(), e)) + ); + continue; } catch (Exception e) { responses.set( i, @@ -89,8 +83,6 @@ protected void doExecute(Task task, final MultiTermVectorsRequest request, final continue; } - ShardId shardId = clusterService.operationRouting() - .shardId(clusterState, concreteSingleIndex, termVectorsRequest.id(), termVectorsRequest.routing()); MultiTermVectorsShardRequest shardRequest = shardRequests.get(shardId); if (shardRequest == null) { shardRequest = new MultiTermVectorsShardRequest(shardId.getIndexName(), shardId.id()); diff --git a/server/src/main/java/org/elasticsearch/action/termvectors/TransportTermVectorsAction.java b/server/src/main/java/org/elasticsearch/action/termvectors/TransportTermVectorsAction.java index 120b3825df40c..eb07ff914efe9 100644 --- a/server/src/main/java/org/elasticsearch/action/termvectors/TransportTermVectorsAction.java +++ b/server/src/main/java/org/elasticsearch/action/termvectors/TransportTermVectorsAction.java @@ -9,7 +9,6 @@ package org.elasticsearch.action.termvectors; import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.RoutingMissingException; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.single.shard.TransportSingleShardAction; import org.elasticsearch.cluster.ClusterState; @@ -81,10 +80,6 @@ protected boolean resolveIndex(TermVectorsRequest request) { protected void resolveRequest(ClusterState state, InternalRequest request) { // update the routing (request#index here is possibly an alias or a parent) request.request().routing(state.metadata().resolveIndexRouting(request.request().routing(), request.request().index())); - // Fail fast on the node that received the request. - if (request.request().routing() == null && state.getMetadata().routingRequired(request.concreteIndex())) { - throw new RoutingMissingException(request.concreteIndex(), request.request().id()); - } } @Override diff --git a/server/src/main/java/org/elasticsearch/action/update/TransportUpdateAction.java b/server/src/main/java/org/elasticsearch/action/update/TransportUpdateAction.java index 85ca9c1ab76d1..cce6ba78559e2 100644 --- a/server/src/main/java/org/elasticsearch/action/update/TransportUpdateAction.java +++ b/server/src/main/java/org/elasticsearch/action/update/TransportUpdateAction.java @@ -12,7 +12,6 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionRunnable; import org.elasticsearch.action.DocWriteRequest; -import org.elasticsearch.action.RoutingMissingException; import org.elasticsearch.action.admin.indices.create.CreateIndexRequest; import org.elasticsearch.action.admin.indices.create.CreateIndexResponse; import org.elasticsearch.action.delete.DeleteRequest; @@ -27,7 +26,6 @@ import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; -import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.cluster.routing.IndexRouting; import org.elasticsearch.cluster.routing.RoutingTable; import org.elasticsearch.cluster.routing.ShardIterator; @@ -110,16 +108,8 @@ protected boolean retryOnFailure(Exception e) { } @Override - protected void resolveRequest(ClusterState state, UpdateRequest request) { - resolveAndValidateRouting(state.metadata(), request.concreteIndex(), request); - } - - public static void resolveAndValidateRouting(Metadata metadata, String concreteIndex, UpdateRequest request) { - request.routing((metadata.resolveWriteIndexRouting(request.routing(), request.index()))); - // Fail fast on the node that received the request, rather than failing when translating on the index or delete request. - if (request.routing() == null && metadata.routingRequired(concreteIndex)) { - throw new RoutingMissingException(concreteIndex, request.id()); - } + protected void resolveRequest(ClusterState state, UpdateRequest docWriteRequest) { + docWriteRequest.routing(state.metadata().resolveWriteIndexRouting(docWriteRequest.routing(), docWriteRequest.index())); } @Override diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java b/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java index 731e7c1a33ea8..1e858d3120cfa 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java @@ -857,21 +857,6 @@ public int getTotalOpenIndexShards() { return this.totalOpenIndexShards; } - /** - * @param concreteIndex The concrete index to check if routing is required - * @return Whether routing is required according to the mapping for the specified index and type - */ - public boolean routingRequired(String concreteIndex) { - IndexMetadata indexMetadata = indices.get(concreteIndex); - if (indexMetadata != null) { - MappingMetadata mappingMetadata = indexMetadata.mapping(); - if (mappingMetadata != null) { - return mappingMetadata.routingRequired(); - } - } - return false; - } - @Override public Iterator iterator() { return indices.valuesIt(); diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java b/server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java index 0e07b5110e94e..27c7c69c1b554 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java @@ -8,7 +8,9 @@ package org.elasticsearch.cluster.routing; +import org.elasticsearch.action.RoutingMissingException; import org.elasticsearch.cluster.metadata.IndexMetadata; +import org.elasticsearch.cluster.metadata.MappingMetadata; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.core.Nullable; @@ -35,34 +37,24 @@ public abstract class IndexRouting { /** * Build the routing from {@link IndexMetadata}. */ - public static IndexRouting fromIndexMetadata(IndexMetadata indexMetadata) { - if (false == indexMetadata.getRoutingPaths().isEmpty()) { - if (indexMetadata.isRoutingPartitionedIndex()) { - throw new IllegalArgumentException("routing_partition_size is incompatible with routing_path"); - } - return new ExtractFromSource( - indexMetadata.getRoutingNumShards(), - indexMetadata.getRoutingFactor(), - indexMetadata.getIndex().getName(), - indexMetadata.getRoutingPaths() - ); + public static IndexRouting fromIndexMetadata(IndexMetadata metadata) { + if (false == metadata.getRoutingPaths().isEmpty()) { + return new ExtractFromSource(metadata); } - if (indexMetadata.isRoutingPartitionedIndex()) { - return new Partitioned( - indexMetadata.getRoutingNumShards(), - indexMetadata.getRoutingFactor(), - indexMetadata.getRoutingPartitionSize() - ); + if (metadata.isRoutingPartitionedIndex()) { + return new Partitioned(metadata); } - return new Unpartitioned(indexMetadata.getRoutingNumShards(), indexMetadata.getRoutingFactor()); + return new Unpartitioned(metadata); } + protected final String indexName; private final int routingNumShards; private final int routingFactor; - private IndexRouting(int routingNumShards, int routingFactor) { - this.routingNumShards = routingNumShards; - this.routingFactor = routingFactor; + private IndexRouting(IndexMetadata metadata) { + this.indexName = metadata.getIndex().getName(); + this.routingNumShards = metadata.getRoutingNumShards(); + this.routingFactor = metadata.getRoutingFactor(); } /** @@ -119,39 +111,53 @@ private static int effectiveRoutingToHash(String effectiveRouting) { } private abstract static class IdAndRoutingOnly extends IndexRouting { - IdAndRoutingOnly(int routingNumShards, int routingFactor) { - super(routingNumShards, routingFactor); + private final boolean routingRequired; + + IdAndRoutingOnly(IndexMetadata metadata) { + super(metadata); + MappingMetadata mapping = metadata.mapping(); + this.routingRequired = mapping == null ? false : mapping.routingRequired(); } protected abstract int shardId(String id, @Nullable String routing); @Override public int indexShard(String id, @Nullable String routing, XContentType sourceType, BytesReference source) { + checkRoutingRequired(id, routing); return shardId(id, routing); } @Override public int updateShard(String id, @Nullable String routing) { + checkRoutingRequired(id, routing); return shardId(id, routing); } @Override public int deleteShard(String id, @Nullable String routing) { + checkRoutingRequired(id, routing); return shardId(id, routing); } @Override public int getShard(String id, @Nullable String routing) { + checkRoutingRequired(id, routing); return shardId(id, routing); } + + private void checkRoutingRequired(String id, @Nullable String routing) { + if (routingRequired && routing == null) { + throw new RoutingMissingException(indexName, id); + } + } } /** * Strategy for indices that are not partitioned. */ private static class Unpartitioned extends IdAndRoutingOnly { - Unpartitioned(int routingNumShards, int routingFactor) { - super(routingNumShards, routingFactor); + Unpartitioned(IndexMetadata metadata) { + super(metadata); } @Override @@ -171,9 +177,9 @@ public void collectSearchShards(String routing, IntConsumer consumer) { private static class Partitioned extends IdAndRoutingOnly { private final int routingPartitionSize; - Partitioned(int routingNumShards, int routingFactor, int routingPartitionSize) { - super(routingNumShards, routingFactor); - this.routingPartitionSize = routingPartitionSize; + Partitioned(IndexMetadata metadata) { + super(metadata); + this.routingPartitionSize = metadata.getRoutingPartitionSize(); } @Override @@ -195,13 +201,14 @@ public void collectSearchShards(String routing, IntConsumer consumer) { } private static class ExtractFromSource extends IndexRouting { - private final String indexName; private final XContentParserConfiguration parserConfig; - ExtractFromSource(int routingNumShards, int routingFactor, String indexName, List routingPaths) { - super(routingNumShards, routingFactor); - this.indexName = indexName; - this.parserConfig = XContentParserConfiguration.EMPTY.withFiltering(Set.copyOf(routingPaths), null); + ExtractFromSource(IndexMetadata metadata) { + super(metadata); + if (metadata.isRoutingPartitionedIndex()) { + throw new IllegalArgumentException("routing_partition_size is incompatible with routing_path"); + } + this.parserConfig = XContentParserConfiguration.EMPTY.withFiltering(Set.copyOf(metadata.getRoutingPaths()), null); } @Override diff --git a/server/src/test/java/org/elasticsearch/action/index/IndexRequestTests.java b/server/src/test/java/org/elasticsearch/action/index/IndexRequestTests.java index e947b07b52c33..471c81936a771 100644 --- a/server/src/test/java/org/elasticsearch/action/index/IndexRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/index/IndexRequestTests.java @@ -109,10 +109,10 @@ public void testWaitForActiveShards() { public void testAutoGenIdTimestampIsSet() { IndexRequest request = new IndexRequest("index"); - request.process(Version.CURRENT, null, "index"); + request.process(); assertTrue("expected > 0 but got: " + request.getAutoGeneratedTimestamp(), request.getAutoGeneratedTimestamp() > 0); request = new IndexRequest("index").id("1"); - request.process(Version.CURRENT, null, "index"); + request.process(); assertEquals(IndexRequest.UNSET_AUTO_GENERATED_TIMESTAMP, request.getAutoGeneratedTimestamp()); } diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/IndexRoutingTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/IndexRoutingTests.java index e8bc169e1cd08..f452e54209075 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/IndexRoutingTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/IndexRoutingTests.java @@ -8,6 +8,7 @@ package org.elasticsearch.cluster.routing; import org.elasticsearch.Version; +import org.elasticsearch.action.RoutingMissingException; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.core.Nullable; @@ -401,21 +402,34 @@ public void testBWC() { } } + public void testRequiredRouting() { + IndexRouting indexRouting = IndexRouting.fromIndexMetadata( + IndexMetadata.builder("test") + .settings(settings(Version.CURRENT)) + .numberOfShards(2) + .numberOfReplicas(1) + .putMapping("{\"_routing\":{\"required\": true}}") + .build() + ); + Exception e = expectThrows(RoutingMissingException.class, () -> shardIdFromSimple(indexRouting, "id", null)); + assertThat(e.getMessage(), equalTo("routing is required for [test]/[id]")); + } + /** * Extract a shardId from a "simple" {@link IndexRouting} using a randomly * chosen method. All of the random methods should return * the same results. */ - private int shardIdFromSimple(IndexRouting indexRouting, String key, @Nullable String routing) { + private int shardIdFromSimple(IndexRouting indexRouting, String id, @Nullable String routing) { switch (between(0, 3)) { case 0: - return indexRouting.indexShard(key, routing, null, null); + return indexRouting.indexShard(id, routing, null, null); case 1: - return indexRouting.updateShard(key, routing); + return indexRouting.updateShard(id, routing); case 2: - return indexRouting.deleteShard(key, routing); + return indexRouting.deleteShard(id, routing); case 3: - return indexRouting.getShard(key, routing); + return indexRouting.getShard(id, routing); default: throw new AssertionError("invalid option"); } diff --git a/server/src/test/java/org/elasticsearch/index/replication/IndexLevelReplicationTests.java b/server/src/test/java/org/elasticsearch/index/replication/IndexLevelReplicationTests.java index 230e003b3dce5..789e743421374 100644 --- a/server/src/test/java/org/elasticsearch/index/replication/IndexLevelReplicationTests.java +++ b/server/src/test/java/org/elasticsearch/index/replication/IndexLevelReplicationTests.java @@ -12,7 +12,6 @@ import org.apache.lucene.index.Term; import org.apache.lucene.search.TermQuery; import org.apache.lucene.search.TopDocs; -import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.DocWriteResponse; import org.elasticsearch.action.bulk.BulkItemResponse; @@ -149,7 +148,7 @@ public void testRetryAppendOnlyAfterRecovering() throws Exception { try (ReplicationGroup shards = createGroup(0)) { shards.startAll(); final IndexRequest originalRequest = new IndexRequest(index.getName()).source("{}", XContentType.JSON); - originalRequest.process(Version.CURRENT, null, index.getName()); + originalRequest.process(); final IndexRequest retryRequest = copyIndexRequest(originalRequest); retryRequest.onRetry(); shards.index(retryRequest); diff --git a/server/src/test/java/org/elasticsearch/index/replication/RecoveryDuringReplicationTests.java b/server/src/test/java/org/elasticsearch/index/replication/RecoveryDuringReplicationTests.java index 57ed066a99dfb..8d73de01e0138 100644 --- a/server/src/test/java/org/elasticsearch/index/replication/RecoveryDuringReplicationTests.java +++ b/server/src/test/java/org/elasticsearch/index/replication/RecoveryDuringReplicationTests.java @@ -12,7 +12,6 @@ import org.apache.lucene.index.IndexWriter; import org.apache.lucene.index.IndexableField; import org.apache.lucene.store.AlreadyClosedException; -import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.indices.flush.FlushRequest; import org.elasticsearch.action.admin.indices.forcemerge.ForceMergeRequest; @@ -611,7 +610,7 @@ public void testTransferMaxSeenAutoIdTimestampOnResync() throws Exception { List replicationRequests = new ArrayList<>(); for (int numDocs = between(1, 10), i = 0; i < numDocs; i++) { final IndexRequest indexRequest = new IndexRequest(index.getName()).source("{}", XContentType.JSON); - indexRequest.process(Version.CURRENT, null, index.getName()); + indexRequest.process(); final IndexRequest copyRequest; if (randomBoolean()) { copyRequest = copyIndexRequest(indexRequest); diff --git a/test/framework/src/main/java/org/elasticsearch/index/replication/ESIndexLevelReplicationTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/replication/ESIndexLevelReplicationTestCase.java index 5587263a15c5b..41b2a2eed8244 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/replication/ESIndexLevelReplicationTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/replication/ESIndexLevelReplicationTestCase.java @@ -878,7 +878,7 @@ private void executeShardBulkOnPrimary( ) { for (BulkItemRequest itemRequest : request.items()) { if (itemRequest.request() instanceof IndexRequest) { - ((IndexRequest) itemRequest.request()).process(Version.CURRENT, null, index.getName()); + ((IndexRequest) itemRequest.request()).process(); } } final PlainActionFuture permitAcquiredFuture = new PlainActionFuture<>();