From 76f50c3a9763813e9246d627148063af45dd7b68 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 13 Sep 2021 18:07:14 -0400 Subject: [PATCH 01/31] Route documents to the correct shards in tsdb This causes elasticsearch to land documents from the same time series on the same shard. It does so by adding a new index setting `routing_path` which must be set when an index is in `mode: time_series` and may not be set outside of that mode. That setting contains a list of patterns to extract from the `_source` document that are hashed into the routing value. --- .../groovy/elasticsearch.formatting.gradle | 1 + .../20_tsdb_consistency.yml | 103 +++++++ .../rest-api-spec/test/tsdb/10_settings.yml | 32 +++ .../test/tsdb/20_unsupported_operations.yml | 239 ++++++++++++++++ .../action/bulk/TransportBulkAction.java | 26 +- .../action/update/TransportUpdateAction.java | 6 +- .../cluster/metadata/IndexMetadata.java | 24 +- .../cluster/routing/IndexRouting.java | 259 ++++++++++++++++- .../cluster/routing/OperationRouting.java | 19 +- .../common/settings/IndexScopedSettings.java | 1 + .../org/elasticsearch/index/IndexMode.java | 21 +- .../index/shard/ShardSplittingQuery.java | 7 +- .../cluster/routing/IndexRoutingTests.java | 261 +++++++++++++++++- .../index/shard/ShardSplittingQueryTests.java | 10 +- .../index/shard/StoreRecoveryTests.java | 4 +- .../test/InternalTestCluster.java | 6 +- 16 files changed, 956 insertions(+), 63 deletions(-) create mode 100644 qa/smoke-test-multinode/src/test/resources/rest-api-spec/test/smoke_test_multinode/20_tsdb_consistency.yml create mode 100644 rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/20_unsupported_operations.yml diff --git a/build-tools-internal/src/main/groovy/elasticsearch.formatting.gradle b/build-tools-internal/src/main/groovy/elasticsearch.formatting.gradle index 205e5e3229394..4c5195a7ac2ba 100644 --- a/build-tools-internal/src/main/groovy/elasticsearch.formatting.gradle +++ b/build-tools-internal/src/main/groovy/elasticsearch.formatting.gradle @@ -207,6 +207,7 @@ subprojects { 'src/*/java/org/elasticsearch/action/admin/cluster/snapshots/**/*.java', 'src/test/java/org/elasticsearch/common/xcontent/support/AbstractFilteringTestCase.java', 'src/test/java/org/elasticsearch/common/xcontent/support/XContentMapValuesTests.java', + 'src/*/java/org/elasticsearch/index/IndexRouting.java', 'src/*/java/org/elasticsearch/index/snapshots/**/*.java', 'src/*/java/org/elasticsearch/repositories/**/*.java', 'src/*/java/org/elasticsearch/search/aggregations/**/*.java', diff --git a/qa/smoke-test-multinode/src/test/resources/rest-api-spec/test/smoke_test_multinode/20_tsdb_consistency.yml b/qa/smoke-test-multinode/src/test/resources/rest-api-spec/test/smoke_test_multinode/20_tsdb_consistency.yml new file mode 100644 index 0000000000000..d05cb27544d77 --- /dev/null +++ b/qa/smoke-test-multinode/src/test/resources/rest-api-spec/test/smoke_test_multinode/20_tsdb_consistency.yml @@ -0,0 +1,103 @@ +# Test the mode:time_series properly groups by _tsid. If we could put this in +# rest-api-spec we would, but it requires painless. + +setup: + - do: + indices.create: + index: test + body: + settings: + index: + mode: time_series + routing_path: [metricset, k8s.pod.uid] + number_of_shards: 10 + number_of_replicas: 1 + mappings: + properties: + "@timestamp": + type: date + metricset: + type: keyword + dimension: true + k8s: + properties: + pod: + properties: + uid: + type: keyword + dimension: true + name: + type: keyword + ip: + type: ip + network: + properties: + tx: + type: long + rx: + type: long + - do: + bulk: + refresh: true + index: test + body: + - '{"index": {}}' + - '{"@timestamp": "2021-04-28T18:35:24.467Z", "metricset": "pod", "k8s": {"pod": {"name": "cat", "uid":"947e4ced-1786-4e53-9e0c-5c447e959507", "ip": "10.10.55.1", "network": {"tx": 2001818691, "rx": 802133794}}}}' + - '{"index": {}}' + - '{"@timestamp": "2021-04-28T19:50:04.467Z", "metricset": "pod", "k8s": {"pod": {"name": "cat", "uid":"947e4ced-1786-4e53-9e0c-5c447e959507", "ip": "10.10.55.1", "network": {"tx": 2005177954, "rx": 801479970}}}}' + - '{"index": {}}' + - '{"@timestamp": "2021-04-28T17:53:34.467Z", "metricset": "pod", "k8s": {"pod": {"name": "cat", "uid":"947e4ced-1786-4e53-9e0c-5c447e959507", "ip": "10.10.55.1", "network": {"tx": 2006223737, "rx": 802337279}}}}' + - '{"index": {}}' + - '{"@timestamp": "2021-04-28T18:03:24.467Z", "metricset": "pod", "k8s": {"pod": {"name": "cat", "uid":"947e4ced-1786-4e53-9e0c-5c447e959507", "ip": "10.10.55.2", "network": {"tx": 2012916202, "rx": 803685721}}}}' + - '{"index": {}}' + - '{"@timestamp": "2021-04-28T18:35:24.467Z", "metricset": "pod", "k8s": {"pod": {"name": "dog", "uid":"df3145b3-0563-4d3b-a0f7-897eb2876ea9", "ip": "10.10.55.3", "network": {"tx": 1434521831, "rx": 530575198}}}}' + - '{"index": {}}' + - '{"@timestamp": "2021-04-28T19:50:04.467Z", "metricset": "pod", "k8s": {"pod": {"name": "dog", "uid":"df3145b3-0563-4d3b-a0f7-897eb2876ea9", "ip": "10.10.55.3", "network": {"tx": 1434577921, "rx": 530600088}}}}' + - '{"index": {}}' + - '{"@timestamp": "2021-04-28T17:53:34.467Z", "metricset": "pod", "k8s": {"pod": {"name": "dog", "uid":"df3145b3-0563-4d3b-a0f7-897eb2876ea9", "ip": "10.10.55.3", "network": {"tx": 1434587694, "rx": 530604797}}}}' + - '{"index": {}}' + - '{"@timestamp": "2021-04-28T18:03:24.467Z", "metricset": "pod", "k8s": {"pod": {"name": "dog", "uid":"df3145b3-0563-4d3b-a0f7-897eb2876ea9", "ip": "10.10.55.3", "network": {"tx": 1434595272, "rx": 530605511}}}}' + +--- +"index with replicas and shards is green": + - skip: + version: " - 7.99.99" + reason: introduced in 8.0.0 to be backported to 7.16.0 + + - do: + cluster.health: + wait_for_status: green + - match: { status: green } + +--- +"each shard has unique _tsids": + - skip: + version: " - 7.99.99" + reason: introduced in 8.0.0 to be backported to 7.16.0 + + - do: + search: + index: test + body: + size: 0 + aggs: + check: + scripted_metric: + init_script: "state.timeSeries = new HashSet()" + map_script: "state.timeSeries.add(doc['metricset'].value + ':' + doc['k8s.pod.uid'].value)" + combine_script: "return state.timeSeries" + reduce_script: | + Set timeSeries = new TreeSet(); + for (s in states) { + for (ts in s) { + boolean newTs = timeSeries.add(ts); + if (false == newTs) { + throw new IllegalArgumentException(ts + " appeared in two shards"); + } + } + } + return timeSeries; + - match: {hits.total.value: 8} + - match: {aggregations.check.value: ['pod:947e4ced-1786-4e53-9e0c-5c447e959507', 'pod:df3145b3-0563-4d3b-a0f7-897eb2876ea9']} + +# todo clone and shrink diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/10_settings.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/10_settings.yml index a4987bf69d70b..c3a3547f0a103 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/10_settings.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/10_settings.yml @@ -10,6 +10,7 @@ enable: settings: index: mode: time_series + routing_path: [metricset, k8s.pod.uid] number_of_replicas: 0 number_of_shards: 2 mappings: @@ -117,3 +118,34 @@ no partitioning: mode: time_series shards: 5 routing_partition_size: 2 + +--- +routing_path required: + - skip: + version: " - 7.99.99" + reason: introduced in 8.0.0 to be backported to 7.16.0 + + - do: + catch: /\[index.mode=time_series\] requires \[index.routing_path\]/ + indices.create: + index: test_index + body: + settings: + index: + mode: time_series + shards: 5 + +--- +routing_path is not allowed in standard mode: + - skip: + version: " - 7.99.99" + reason: introduced in 8.0.0 to be backported to 7.16.0 + + - do: + catch: /\[index.routing_path\] requires \[index.mode=time_series\]/ + indices.create: + index: test_index + body: + settings: + index: + routing_path: foo diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/20_unsupported_operations.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/20_unsupported_operations.yml new file mode 100644 index 0000000000000..1610978f3dcf2 --- /dev/null +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/20_unsupported_operations.yml @@ -0,0 +1,239 @@ +setup: + - skip: + version: " - 7.99.99" + reason: introduced in 8.0.0 to be backported to 7.16.0 + + - do: + indices.create: + index: test + body: + settings: + index: + mode: time_series + routing_path: [metricset, k8s.pod.uid] + number_of_replicas: 0 + number_of_shards: 2 + mappings: + properties: + "@timestamp": + type: date + metricset: + type: keyword + dimension: true + k8s: + properties: + pod: + properties: + uid: + type: keyword + dimension: true + name: + type: keyword + ip: + type: ip + network: + properties: + tx: + type: long + rx: + type: long + + - do: + bulk: + refresh: true + index: test + body: + - '{"index": {}}' + - '{"@timestamp": "2021-04-28T18:50:04.467Z", "metricset": "pod", "k8s": {"pod": {"name": "cat", "uid":"947e4ced-1786-4e53-9e0c-5c447e959507", "ip": "10.10.55.1", "network": {"tx": 2001818691, "rx": 802133794}}}}' + - '{"index": {}}' + - '{"@timestamp": "2021-04-28T18:50:24.467Z", "metricset": "pod", "k8s": {"pod": {"name": "cat", "uid":"947e4ced-1786-4e53-9e0c-5c447e959507", "ip": "10.10.55.1", "network": {"tx": 2005177954, "rx": 801479970}}}}' + - '{"index": {}}' + - '{"@timestamp": "2021-04-28T18:50:44.467Z", "metricset": "pod", "k8s": {"pod": {"name": "cat", "uid":"947e4ced-1786-4e53-9e0c-5c447e959507", "ip": "10.10.55.1", "network": {"tx": 2006223737, "rx": 802337279}}}}' + - '{"index": {}}' + - '{"@timestamp": "2021-04-28T18:51:04.467Z", "metricset": "pod", "k8s": {"pod": {"name": "cat", "uid":"947e4ced-1786-4e53-9e0c-5c447e959507", "ip": "10.10.55.2", "network": {"tx": 2012916202, "rx": 803685721}}}}' + - '{"index": {}}' + - '{"@timestamp": "2021-04-28T18:50:03.142Z", "metricset": "pod", "k8s": {"pod": {"name": "dog", "uid":"df3145b3-0563-4d3b-a0f7-897eb2876ea9", "ip": "10.10.55.3", "network": {"tx": 1434521831, "rx": 530575198}}}}' + - '{"index": {}}' + - '{"@timestamp": "2021-04-28T18:50:23.142Z", "metricset": "pod", "k8s": {"pod": {"name": "dog", "uid":"df3145b3-0563-4d3b-a0f7-897eb2876ea9", "ip": "10.10.55.3", "network": {"tx": 1434577921, "rx": 530600088}}}}' + - '{"index": {}}' + - '{"@timestamp": "2021-04-28T18:50:53.142Z", "metricset": "pod", "k8s": {"pod": {"name": "dog", "uid":"df3145b3-0563-4d3b-a0f7-897eb2876ea9", "ip": "10.10.55.3", "network": {"tx": 1434587694, "rx": 530604797}}}}' + - '{"index": {}}' + - '{"@timestamp": "2021-04-28T18:51:03.142Z", "metricset": "pod", "k8s": {"pod": {"name": "dog", "uid":"df3145b3-0563-4d3b-a0f7-897eb2876ea9", "ip": "10.10.55.3", "network": {"tx": 1434595272, "rx": 530605511}}}}' + +--- +index with specified id: + - skip: + version: " - 7.99.99" + reason: introduced in 8.0.0 to be backported to 7.16.0 + + - do: + catch: /indexing with a specified id is not supported because the destination index \[test\] is in time series mode/ + index: + index: test + id: foo + body: + doc: + "@timestamp": "2021-04-28T18:35:24.467Z" + metricset: "pod" + k8s: + pod: + name: "cat" + uid: "947e4ced-1786-4e53-9e0c-5c447e959507" + ip: "10.10.55.1" + network: + tx: 2001818691 + rx: 802133794 + +--- +index with specified id over _bulk: + - skip: + version: " - 7.99.99" + reason: introduced in 8.0.0 to be backported to 7.16.0 + + - do: + bulk: + refresh: true + index: test + body: + - '{"index": {"_id": "foo"}}' + - '{"@timestamp": "2021-04-28T18:50:04.467Z", "metricset": "pod", "k8s": {"pod": {"name": "cat", "uid":"947e4ced-1786-4e53-9e0c-5c447e959507", "ip": "10.10.55.1", "network": {"tx": 2001818691, "rx": 802133794}}}}' + - match: {items.0.index.error.reason: "indexing with a specified id is not supported because the destination index [test] is in time series mode"} + +--- +index with specified routing: + - skip: + version: " - 7.99.99" + reason: introduced in 8.0.0 to be backported to 7.16.0 + + - do: + catch: /indexing with a specified routing is not supported because the destination index \[test\] is in time series mode/ + index: + index: test + routing: foo + body: + doc: + "@timestamp": "2021-04-28T18:35:24.467Z" + metricset: "pod" + k8s: + pod: + name: "cat" + uid: "947e4ced-1786-4e53-9e0c-5c447e959507" + ip: "10.10.55.1" + network: + tx: 2001818691 + rx: 802133794 + +--- +index with specified routing over _bulk: + - skip: + version: " - 7.99.99" + reason: introduced in 8.0.0 to be backported to 7.16.0 + + - do: + bulk: + refresh: true + index: test + body: + - '{"index": {"routing": "foo"}}' + - '{"@timestamp": "2021-04-28T18:50:04.467Z", "metricset": "pod", "k8s": {"pod": {"name": "cat", "uid":"947e4ced-1786-4e53-9e0c-5c447e959507", "ip": "10.10.55.1", "network": {"tx": 2001818691, "rx": 802133794}}}}' + - match: {items.0.index.error.reason: "indexing with a specified routing is not supported because the destination index [test] is in time series mode"} + +--- +delete: + - skip: + version: " - 7.99.99" + reason: introduced in 8.0.0 to be backported to 7.16.0 + + - do: + catch: /delete is not supported because the destination index \[test\] is in time series mode/ + delete: + index: test + id: 1 + +--- +delete over _bulk: + - skip: + version: " - 7.99.99" + reason: introduced in 8.0.0 to be backported to 7.16.0 + + - do: + bulk: + index: test + body: + - '{"delete": {"_id": 1}}' + - '{"delete": {"_id": 2}}' + - match: {items.0.delete.error.reason: "delete is not supported because the destination index [test] is in time series mode"} + +--- +noop update: + - skip: + version: " - 7.99.99" + reason: introduced in 8.0.0 to be backported to 7.16.0 + + - do: + search: + index: test + size: 1 + + - length: {hits.hits: 1} + + - do: + catch: /update is not supported because the destination index \[test\] is in time series mode/ + update: + index: test + id: $body.hits.hits.0._id + body: + doc: + $body.hits.hits.0._source + +--- +update: + - skip: + version: " - 7.99.99" + reason: introduced in 8.0.0 to be backported to 7.16.0 + + # We fail even though the document isn't found. + - do: + catch: /update is not supported because the destination index \[test\] is in time series mode/ + update: + index: test + id: 1 + body: + doc: + "@timestamp": "2021-04-28T18:35:24.467Z" + metricset: "pod" + k8s: + pod: + name: "cat" + uid: "947e4ced-1786-4e53-9e0c-5c447e959507" + ip: "10.10.55.1" + network: + tx: 2001818691 + rx: 802133794 + +--- +update over _bulk: + - skip: + version: " - 7.99.99" + reason: introduced in 8.0.0 to be backported to 7.16.0 + + - do: + bulk: + index: test + body: + - '{"update": {"_id": 1}}' + - '{"doc":{"@timestamp": "2021-04-28T18:03:24.467Z", "metricset": "pod", "k8s": {"pod": {"name": "dog", "uid":"df3145b3-0563-4d3b-a0f7-897eb2876ea9", "ip": "10.10.55.3", "network": {"tx": 1434595272, "rx": 530605511}}}}}' + - match: {items.0.update.error.reason: "update is not supported because the destination index [test] is in time series mode"} + +--- +search with routing: + - skip: + version: " - 7.99.99" + reason: introduced in 8.0.0 to be backported to 7.16.0 + + # We fail even though the document isn't found. + - do: + catch: /searching with a specified routing is not supported because the destination index \[test\] is in time series mode/ + search: + index: test + routing: rrrr 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 3c9cb0fa90017..49293c4185b07 100644 --- a/server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java +++ b/server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java @@ -43,6 +43,7 @@ import org.elasticsearch.cluster.metadata.MappingMetadata; import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.cluster.routing.IndexRouting; +import org.elasticsearch.cluster.routing.RoutingTable; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.unit.ByteSizeUnit; @@ -451,6 +452,11 @@ protected void doRun() { throw new IllegalArgumentException("only write ops with an op_type of create are allowed in data streams"); } + IndexRouting indexRouting = indexRoutings.computeIfAbsent( + concreteIndex, + idx -> IndexRouting.fromIndexMetadata(clusterState.metadata().getIndexSafe(idx)) + ); + int shardId; switch (docWriteRequest.opType()) { case CREATE: case INDEX: @@ -461,11 +467,20 @@ protected void doRun() { MappingMetadata mappingMd = indexMetadata.mapping(); Version indexCreated = indexMetadata.getCreationVersion(); indexRequest.resolveRouting(metadata); + boolean idIsAutoGenerated = indexRequest.id() == null; indexRequest.process(indexCreated, mappingMd, concreteIndex.getName()); + shardId = indexRouting.indexShard( + idIsAutoGenerated, + docWriteRequest.id(), + docWriteRequest.routing(), + indexRequest.getContentType(), + indexRequest.source() + ); break; case UPDATE: TransportUpdateAction.resolveAndValidateRouting(metadata, concreteIndex.getName(), (UpdateRequest) docWriteRequest); + shardId = indexRouting.updateShard(docWriteRequest.id(), docWriteRequest.routing()); break; case DELETE: docWriteRequest.routing(metadata.resolveWriteIndexRouting(docWriteRequest.routing(), docWriteRequest.index())); @@ -473,17 +488,14 @@ protected void doRun() { if (docWriteRequest.routing() == null && metadata.routingRequired(concreteIndex.getName())) { throw new RoutingMissingException(concreteIndex.getName(), docWriteRequest.id()); } + shardId = indexRouting.deleteShard(docWriteRequest.id(), docWriteRequest.routing()); break; default: throw new AssertionError("request type not supported: [" + docWriteRequest.opType() + "]"); } - IndexRouting indexRouting = indexRoutings.computeIfAbsent( - concreteIndex, - idx -> IndexRouting.fromIndexMetadata(clusterState.metadata().getIndexSafe(idx)) + List shardRequests = requestsByShard.computeIfAbsent( + RoutingTable.shardRoutingTable(clusterState.routingTable().index(concreteIndex), shardId).getShardId(), + shard -> new ArrayList<>() ); - ShardId shardId = clusterService.operationRouting() - .indexShards(clusterState, concreteIndex.getName(), indexRouting, docWriteRequest.id(), docWriteRequest.routing()) - .shardId(); - List shardRequests = requestsByShard.computeIfAbsent(shardId, shard -> new ArrayList<>()); shardRequests.add(new BulkItemRequest(i, docWriteRequest)); } catch (ElasticsearchParseException | IllegalArgumentException | RoutingMissingException e) { BulkItemResponse.Failure failure = new BulkItemResponse.Failure(concreteIndex.getName(), docWriteRequest.id(), e); 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 fdc668b881f3e..c1a1583a68464 100644 --- a/server/src/main/java/org/elasticsearch/action/update/TransportUpdateAction.java +++ b/server/src/main/java/org/elasticsearch/action/update/TransportUpdateAction.java @@ -30,6 +30,7 @@ import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.cluster.routing.IndexRouting; import org.elasticsearch.cluster.routing.PlainShardIterator; +import org.elasticsearch.cluster.routing.RoutingTable; import org.elasticsearch.cluster.routing.ShardIterator; import org.elasticsearch.cluster.routing.ShardRouting; import org.elasticsearch.cluster.service.ClusterService; @@ -164,8 +165,9 @@ protected ShardIterator shards(ClusterState clusterState, UpdateRequest request) throw new IndexNotFoundException(request.concreteIndex()); } IndexRouting indexRouting = IndexRouting.fromIndexMetadata(indexMetadata); - ShardIterator shardIterator = clusterService.operationRouting() - .indexShards(clusterState, request.concreteIndex(), indexRouting, request.id(), request.routing()); + int shardId = indexRouting.updateShard(request.id(), request.routing()); + ShardIterator shardIterator = RoutingTable.shardRoutingTable(clusterState.routingTable().index(request.concreteIndex()), shardId) + .shardsIt(); ShardRouting shard; while ((shard = shardIterator.nextOrNull()) != null) { if (shard.primary()) { 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 b06a0a7c27841..9de3784087f76 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetadata.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetadata.java @@ -328,6 +328,14 @@ public static APIBlock readFrom(StreamInput input) throws IOException { public static final Setting INDEX_FORMAT_SETTING = Setting.intSetting(INDEX_FORMAT, 0, Setting.Property.IndexScope, Setting.Property.Final); + public static final Setting> INDEX_ROUTING_PATH = Setting.listSetting( + "index.routing_path", + List.of(), + Function.identity(), + Setting.Property.IndexScope, + Setting.Property.Final + ); + 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"; @@ -350,6 +358,7 @@ public static APIBlock readFrom(StreamInput input) throws IOException { private final int routingNumShards; private final int routingFactor; private final int routingPartitionSize; + private final List routingPaths; private final int numberOfShards; private final int numberOfReplicas; @@ -414,6 +423,7 @@ private IndexMetadata( final Version indexCreatedVersion, final int routingNumShards, final int routingPartitionSize, + final List routingPaths, final ActiveShardCount waitForActiveShards, final ImmutableOpenMap rolloverInfos, final boolean isSystem, @@ -446,6 +456,7 @@ private IndexMetadata( this.routingNumShards = routingNumShards; this.routingFactor = routingNumShards / numberOfShards; this.routingPartitionSize = routingPartitionSize; + this.routingPaths = routingPaths; this.waitForActiveShards = waitForActiveShards; this.rolloverInfos = rolloverInfos; this.isSystem = isSystem; @@ -532,6 +543,10 @@ public boolean isRoutingPartitionedIndex() { return routingPartitionSize != 1; } + public List getRoutingPaths() { + return routingPaths; + } + public int getTotalNumberOfShards() { return totalNumberOfShards; } @@ -625,6 +640,8 @@ public IndexLongFieldRange getTimestampRange() { return timestampRange; } + + @Override public boolean equals(Object o) { if (this == o) { @@ -1282,6 +1299,8 @@ public IndexMetadata build() { "number of shard copies [" + (numberOfReplicas + 1) + "]"); } + final List routingPaths = INDEX_ROUTING_PATH.get(settings); + final String uuid = settings.get(SETTING_INDEX_UUID, INDEX_UUID_NA_VALUE); return new IndexMetadata( @@ -1306,6 +1325,7 @@ public IndexMetadata build() { indexCreatedVersion, getRoutingNumShards(), routingPartitionSize, + routingPaths, waitForActiveShards, rolloverInfos.build(), isSystem, @@ -1625,7 +1645,7 @@ public IndexMetadata fromXContent(XContentParser parser) throws IOException { /** * Returns the number of shards that should be used for routing. This basically defines the hash space we use in - * {@link IndexRouting#shardId(String, String)} to route documents + * {@link IndexRouting#indexShard} to route documents * to shards based on their ID or their specific routing value. The default value is {@link #getNumberOfShards()}. This value only * changes if and index is shrunk. */ @@ -1738,7 +1758,7 @@ public static Set selectShrinkShards(int shardId, IndexMetadata sourceI /** * Returns the routing factor for and shrunk index with the given number of target shards. * This factor is used in the hash function in - * {@link IndexRouting#shardId(String, String)} to guarantee consistent + * {@link IndexRouting#indexShard} to guarantee consistent * hashing / routing of documents even if the number of shards changed (ie. a shrunk index). * * @param sourceNumberOfShards the total number of shards in the source index 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 efd34c4079194..133aaa0f729c0 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java @@ -8,11 +8,26 @@ package org.elasticsearch.cluster.routing; +import org.apache.lucene.util.ArrayUtil; +import org.apache.lucene.util.IntroSorter; import org.elasticsearch.cluster.metadata.IndexMetadata; +import org.elasticsearch.common.ParsingException; +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.xcontent.DeprecationHandler; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.XContentParser.Token; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.common.xcontent.support.filtering.FilterPath; import org.elasticsearch.core.Nullable; +import java.io.IOException; +import java.util.List; +import java.util.Set; import java.util.function.IntConsumer; +import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken; + /** * Generates the shard id for {@code (id, routing)} pairs. */ @@ -21,6 +36,17 @@ 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() + ); + } if (indexMetadata.isRoutingPartitionedIndex()) { return new Partitioned( indexMetadata.getRoutingNumShards(), @@ -40,17 +66,46 @@ private IndexRouting(int routingNumShards, int routingFactor) { } /** - * Generate the single shard id that should contain a document with the - * provided {@code id} and {@code routing}. + * Called when indexing a document to generate the shard id that should contain + * a document with the provided parameters. + */ + public abstract int indexShard( + boolean idIsAutoGenerated, + String id, + @Nullable String routing, + XContentType sourceType, + BytesReference source + ); + + /** + * Called when updating a document to generate the shard id that should contain + * a document with the provided {@code _id} and (optional) {@code _routing}. */ - public abstract int shardId(String id, @Nullable String routing); + public abstract int updateShard(String id, @Nullable String routing); + + /** + * Called when deleting a document to generate the shard id that should contain + * a document with the provided {@code _id} and (optional) {@code _routing}. + */ + public abstract int deleteShard(String id, @Nullable String routing); + + /** + * Called when getting a document to generate the shard id that should contain + * a document with the provided {@code _id} and (optional) {@code _routing}. + */ + public abstract int getShard(String id, @Nullable String routing); /** * Collect all of the shard ids that *may* contain documents with the * provided {@code routing}. Indices with a {@code routing_partition} * will collect more than one shard. Indices without a partition * will collect the same shard id as would be returned - * by {@link #shardId}. + * by {@link #getShard}. + *

+ * Note: This is called for any search-like requests that have a + * routing specified but only if they have a routing + * specified. If they do not have a routing they just use all shards + * in the index. */ public abstract void collectSearchShards(String routing, IntConsumer consumer); @@ -69,16 +124,50 @@ private static int effectiveRoutingToHash(String effectiveRouting) { return Murmur3HashFunction.hash(effectiveRouting); } + private abstract static class IdAndRoutingOnly extends IndexRouting { + IdAndRoutingOnly(int routingNumShards, int routingFactor) { + super(routingNumShards, routingFactor); + } + + protected abstract int shardId(String id, @Nullable String routing); + + @Override + public int indexShard( + boolean idIsAutoGenerated, + String id, + @Nullable String routing, + XContentType sourceType, + BytesReference source + ) { + return shardId(id, routing); + } + + @Override + public int updateShard(String id, @Nullable String routing) { + return shardId(id, routing); + } + + @Override + public int deleteShard(String id, @Nullable String routing) { + return shardId(id, routing); + } + + @Override + public int getShard(String id, @Nullable String routing) { + return shardId(id, routing); + } + } + /** * Strategy for indices that are not partitioned. */ - private static class Unpartitioned extends IndexRouting { + private static class Unpartitioned extends IdAndRoutingOnly { Unpartitioned(int routingNumShards, int routingFactor) { super(routingNumShards, routingFactor); } @Override - public int shardId(String id, @Nullable String routing) { + protected int shardId(String id, @Nullable String routing) { return hashToShardId(effectiveRoutingToHash(routing == null ? id : routing)); } @@ -91,7 +180,7 @@ public void collectSearchShards(String routing, IntConsumer consumer) { /** * Strategy for partitioned indices. */ - private static class Partitioned extends IndexRouting { + private static class Partitioned extends IdAndRoutingOnly { private final int routingPartitionSize; Partitioned(int routingNumShards, int routingFactor, int routingPartitionSize) { @@ -100,7 +189,7 @@ private static class Partitioned extends IndexRouting { } @Override - public int shardId(String id, @Nullable String routing) { + protected int shardId(String id, @Nullable String routing) { if (routing == null) { throw new IllegalArgumentException("A routing value is required for gets from a partitioned index"); } @@ -116,4 +205,158 @@ public void collectSearchShards(String routing, IntConsumer consumer) { } } } + + private static class ExtractFromSource extends IndexRouting { + private final String indexName; + private final FilterPath[] include; + + ExtractFromSource(int routingNumShards, int routingFactor, String indexName, List routingPaths) { + super(routingNumShards, routingFactor); + this.indexName = indexName; + this.include = FilterPath.compile(Set.copyOf(routingPaths)); + } + + @Override + public int indexShard( + boolean idIsAutoGenerated, + String id, + @Nullable String routing, + XContentType sourceType, + BytesReference source + ) { + if (idIsAutoGenerated == false) { + throw new IllegalArgumentException(error("indexing with a specified id")); + } + if (routing != null) { + throw new IllegalArgumentException(error("indexing with a specified routing")); + } + try { + try ( + XContentParser parser = sourceType.xContent() + .createParser( + NamedXContentRegistry.EMPTY, + DeprecationHandler.THROW_UNSUPPORTED_OPERATION, + source.streamInput(), + include, + null + ) + ) { + parser.nextToken(); // Move to first token + if (parser.currentToken() == null) { + throw new IllegalArgumentException("Error extracting routing: source didn't contain any routing fields"); + } + int hash = extractObject(parser); + ensureExpectedToken(null, parser.nextToken(), parser); + return hashToShardId(hash); + } + } catch (IOException | ParsingException e) { + throw new IllegalArgumentException("Error extracting routing: " + e.getMessage(), e); + } + } + + private static int extractObject(XContentParser source) throws IOException { + ensureExpectedToken(Token.FIELD_NAME, source.nextToken(), source); + String firstFieldName = source.currentName(); + source.nextToken(); + int firstHash = extractItem(source); + if (source.currentToken() == Token.END_OBJECT) { + // Just one routing key in this object + // Use ^ like Map.Entery's hashcode + return Murmur3HashFunction.hash(firstFieldName) ^ firstHash; + } + String[] fields = new String[] { firstFieldName, null }; + int[] hashes = new int[] { firstHash, 0 }; + int length = 1; + do { + ensureExpectedToken(Token.FIELD_NAME, source.currentToken(), source); + String fieldName = source.currentName(); + source.nextToken(); + if (length >= fields.length) { + fields = ArrayUtil.grow(fields, length + 1); + hashes = ArrayUtil.grow(hashes, fields.length); + } + fields[length] = fieldName; + hashes[length] = extractItem(source); + length++; + } while (source.currentToken() != Token.END_OBJECT); + return convertManyHashesToOne(length, fields, hashes); + } + + private static int extractItem(XContentParser source) throws IOException { + if (source.currentToken() == Token.START_OBJECT) { + int hash = extractObject(source); + source.nextToken(); + return hash; + } + if (source.currentToken() == Token.VALUE_STRING) { + int hash = Murmur3HashFunction.hash(source.text()); + source.nextToken(); + return hash; + } + throw new ParsingException(source.getTokenLocation(), "Routing values must be strings but found [{}]", source.currentToken()); + } + + private static int convertManyHashesToOne(int length, String[] fields, int[] hashes) { + new IntroSorter() { + String pivot; + + @Override + protected void swap(int i, int j) { + String field = fields[i]; + fields[i] = fields[j]; + fields[j] = field; + int hash = hashes[i]; + hashes[i] = hashes[j]; + hashes[j] = hash; + } + + @Override + protected void setPivot(int i) { + pivot = fields[i]; + } + + @Override + protected int comparePivot(int j) { + return pivot.compareTo(fields[j]); + } + }.sort(0, length); + /* + * This is the same as Arrays.hash(Map.Entry) but we're + * writing it out so we don't have to allocate the entries and for extra + * paranoia. Changing this will change how documents are routed and we + * don't want a jdk update that modifies Arrays or Map.Entry to sneak up + * on us. + */ + int hash = 0; + for (int i = 0; i < length; i++) { + int thisHash = Murmur3HashFunction.hash(fields[i]) ^ hashes[i]; + hash = 31 * hash + thisHash; + } + return hash; + } + + @Override + public int updateShard(String id, @Nullable String routing) { + throw new IllegalArgumentException(error("update")); + } + + @Override + public int deleteShard(String id, @Nullable String routing) { + throw new IllegalArgumentException(error("delete")); + } + + @Override + public int getShard(String id, @Nullable String routing) { + throw new IllegalArgumentException(error("get")); + } + + @Override + public void collectSearchShards(String routing, IntConsumer consumer) { + throw new IllegalArgumentException(error("searching with a specified routing")); + } + + private String error(String operation) { + return operation + " is not supported because the destination index [" + indexName + "] is in time series mode"; + } + } } diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/OperationRouting.java b/server/src/main/java/org/elasticsearch/cluster/routing/OperationRouting.java index 322db24e8b85b..a775dbad9ad62 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/OperationRouting.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/OperationRouting.java @@ -45,24 +45,15 @@ void setUseAdaptiveReplicaSelection(boolean useAdaptiveReplicaSelection) { this.useAdaptiveReplicaSelection = useAdaptiveReplicaSelection; } - public ShardIterator indexShards( - ClusterState clusterState, - String index, - IndexRouting indexRouting, - String id, - @Nullable String routing - ) { - return shards(clusterState, index, indexRouting, id, routing).shardsIt(); - } - /** * Shards to use for a {@code GET} operation. */ public ShardIterator getShards(ClusterState clusterState, String index, String id, @Nullable String routing, @Nullable String preference) { IndexRouting indexRouting = IndexRouting.fromIndexMetadata(indexMetadata(clusterState, index)); + IndexShardRoutingTable shards = clusterState.getRoutingTable().shardRoutingTable(index, indexRouting.getShard(id, routing)); return preferenceActiveShardIterator( - shards(clusterState, index, indexRouting, id, routing), + shards, clusterState.nodes().getLocalNodeId(), clusterState.nodes(), preference, @@ -220,12 +211,8 @@ private IndexMetadata indexMetadata(ClusterState clusterState, String index) { return indexMetadata; } - private IndexShardRoutingTable shards(ClusterState clusterState, String index, IndexRouting indexRouting, String id, String routing) { - return clusterState.getRoutingTable().shardRoutingTable(index, indexRouting.shardId(id, routing)); - } - public ShardId shardId(ClusterState clusterState, String index, String id, @Nullable String routing) { IndexMetadata indexMetadata = indexMetadata(clusterState, index); - return new ShardId(indexMetadata.getIndex(), IndexRouting.fromIndexMetadata(indexMetadata).shardId(id, routing)); + return new ShardId(indexMetadata.getIndex(), IndexRouting.fromIndexMetadata(indexMetadata).getShard(id, routing)); } } diff --git a/server/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java b/server/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java index 75cd526920222..5f3c31206c3d2 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java @@ -62,6 +62,7 @@ public final class IndexScopedSettings extends AbstractScopedSettings { IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING, IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING, IndexMetadata.INDEX_ROUTING_PARTITION_SIZE_SETTING, + IndexMetadata.INDEX_ROUTING_PATH, IndexMetadata.INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING, IndexMetadata.INDEX_READ_ONLY_SETTING, IndexMetadata.INDEX_BLOCKS_READ_SETTING, diff --git a/server/src/main/java/org/elasticsearch/index/IndexMode.java b/server/src/main/java/org/elasticsearch/index/IndexMode.java index c92a5d742ec09..10cfac6edddb5 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexMode.java +++ b/server/src/main/java/org/elasticsearch/index/IndexMode.java @@ -25,7 +25,16 @@ public enum IndexMode { STANDARD { @Override - void validateWithOtherSettings(Map, Object> settings) {} + void validateWithOtherSettings(Map, Object> settings) { + if (false == Objects.equals( + IndexMetadata.INDEX_ROUTING_PATH.getDefault(Settings.EMPTY), + settings.get(IndexMetadata.INDEX_ROUTING_PATH) + )) { + throw new IllegalArgumentException( + "[" + IndexMetadata.INDEX_ROUTING_PATH.getKey() + "] requires [" + IndexSettings.MODE.getKey() + "=time_series]" + ); + } + } }, TIME_SERIES { @Override @@ -38,6 +47,11 @@ void validateWithOtherSettings(Map, Object> settings) { throw new IllegalArgumentException(error(unsupported)); } } + if (IndexMetadata.INDEX_ROUTING_PATH.getDefault(Settings.EMPTY).equals(settings.get(IndexMetadata.INDEX_ROUTING_PATH))) { + throw new IllegalArgumentException( + "[" + IndexSettings.MODE.getKey() + "=time_series] requires [" + IndexMetadata.INDEX_ROUTING_PATH.getKey() + "]" + ); + } } private String error(Setting unsupported) { @@ -53,7 +67,10 @@ private String error(Setting unsupported) { ); static final List> VALIDATE_WITH_SETTINGS = List.copyOf( - Stream.concat(Stream.of(IndexMetadata.INDEX_ROUTING_PARTITION_SIZE_SETTING), TIME_SERIES_UNSUPPORTED.stream()).collect(toSet()) + Stream.concat( + Stream.of(IndexMetadata.INDEX_ROUTING_PARTITION_SIZE_SETTING, IndexMetadata.INDEX_ROUTING_PATH), + TIME_SERIES_UNSUPPORTED.stream() + ).collect(toSet()) ); abstract void validateWithOtherSettings(Map, Object> settings); diff --git a/server/src/main/java/org/elasticsearch/index/shard/ShardSplittingQuery.java b/server/src/main/java/org/elasticsearch/index/shard/ShardSplittingQuery.java index d59a755993e9e..a65b5cf113d49 100644 --- a/server/src/main/java/org/elasticsearch/index/shard/ShardSplittingQuery.java +++ b/server/src/main/java/org/elasticsearch/index/shard/ShardSplittingQuery.java @@ -72,7 +72,8 @@ public Scorer scorer(LeafReaderContext context) throws IOException { FixedBitSet bitSet = new FixedBitSet(leafReader.maxDoc()); Terms terms = leafReader.terms(RoutingFieldMapper.NAME); Predicate includeInShard = ref -> { - int targetShardId = indexRouting.shardId(Uid.decodeId(ref.bytes, ref.offset, ref.length), null); + // TODO IndexRouting should build the query somehow + int targetShardId = indexRouting.getShard(Uid.decodeId(ref.bytes, ref.offset, ref.length), null); return shardId == targetShardId; }; if (terms == null) { @@ -115,7 +116,7 @@ public Scorer scorer(LeafReaderContext context) throws IOException { }; // in the _routing case we first go and find all docs that have a routing value and mark the ones we have to delete findSplitDocs(RoutingFieldMapper.NAME, ref -> { - int targetShardId = indexRouting.shardId(null, ref.utf8ToString()); + int targetShardId = indexRouting.getShard(null, ref.utf8ToString()); return shardId == targetShardId; }, leafReader, maybeWrapConsumer.apply(bitSet::set)); @@ -259,7 +260,7 @@ boolean matches(int doc) throws IOException { leftToVisit = 2; leafReader.document(doc, this); assert id != null : "docID must not be null - we might have hit a nested document"; - int targetShardId = indexRouting.shardId(id, routing); + int targetShardId = indexRouting.getShard(id, routing); return targetShardId != shardId; } } 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 0c5e050887951..bde7ed51df163 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/IndexRoutingTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/IndexRoutingTests.java @@ -9,16 +9,28 @@ import org.elasticsearch.Version; import org.elasticsearch.cluster.metadata.IndexMetadata; +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.xcontent.DeprecationHandler; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.common.xcontent.support.MapXContentParser; +import org.elasticsearch.core.Nullable; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.VersionUtils; +import java.io.IOException; import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.TreeMap; +import java.util.concurrent.atomic.AtomicInteger; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; public class IndexRoutingTests extends ESTestCase{ @@ -33,18 +45,18 @@ public void testGenerateShardId() { IndexMetadata metadata = IndexMetadata.builder("test").settings(settings(Version.CURRENT)).numberOfShards(shardSplits[0]) .numberOfReplicas(1).build(); String term = randomAlphaOfLength(10); - final int shard = IndexRouting.fromIndexMetadata(metadata).shardId(term, null); + final int shard = shardIdFromSimple(IndexRouting.fromIndexMetadata(metadata), term, null); IndexMetadata shrunk = IndexMetadata.builder("test").settings(settings(Version.CURRENT)).numberOfShards(shardSplits[1]) .numberOfReplicas(1) .setRoutingNumShards(shardSplits[0]).build(); - int shrunkShard = IndexRouting.fromIndexMetadata(shrunk).shardId(term, null); + int shrunkShard = shardIdFromSimple(IndexRouting.fromIndexMetadata(shrunk), term, null); Set shardIds = IndexMetadata.selectShrinkShards(shrunkShard, metadata, shrunk.getNumberOfShards()); assertEquals(1, shardIds.stream().filter((sid) -> sid.id() == shard).count()); shrunk = IndexMetadata.builder("test").settings(settings(Version.CURRENT)).numberOfShards(shardSplits[2]).numberOfReplicas(1) .setRoutingNumShards(shardSplits[0]).build(); - shrunkShard = IndexRouting.fromIndexMetadata(shrunk).shardId(term, null); + shrunkShard = shardIdFromSimple(IndexRouting.fromIndexMetadata(shrunk), term, null); shardIds = IndexMetadata.selectShrinkShards(shrunkShard, metadata, shrunk.getNumberOfShards()); assertEquals(Arrays.toString(shardSplits), 1, shardIds.stream().filter((sid) -> sid.id() == shard).count()); } @@ -61,11 +73,11 @@ public void testGenerateShardIdSplit() { IndexMetadata metadata = IndexMetadata.builder("test").settings(settings(Version.CURRENT)).numberOfShards(shardSplits[0]) .numberOfReplicas(1).setRoutingNumShards(shardSplits[2]).build(); String term = randomAlphaOfLength(10); - final int shard = IndexRouting.fromIndexMetadata(metadata).shardId(term, null); + final int shard = shardIdFromSimple(IndexRouting.fromIndexMetadata(metadata), term, null); IndexMetadata split = IndexMetadata.builder("test").settings(settings(Version.CURRENT)).numberOfShards(shardSplits[1]) .numberOfReplicas(1) .setRoutingNumShards(shardSplits[2]).build(); - int shrunkShard = IndexRouting.fromIndexMetadata(split).shardId(term, null); + int shrunkShard = shardIdFromSimple(IndexRouting.fromIndexMetadata(split), term, null); ShardId shardId = IndexMetadata.selectSplitShard(shrunkShard, metadata, split.getNumberOfShards()); assertNotNull(shardId); @@ -73,7 +85,7 @@ public void testGenerateShardIdSplit() { split = IndexMetadata.builder("test").settings(settings(Version.CURRENT)).numberOfShards(shardSplits[2]).numberOfReplicas(1) .setRoutingNumShards(shardSplits[2]).build(); - shrunkShard = IndexRouting.fromIndexMetadata(split).shardId(term, null); + shrunkShard = shardIdFromSimple(IndexRouting.fromIndexMetadata(split), term, null); shardId = IndexMetadata.selectSplitShard(shrunkShard, metadata, split.getNumberOfShards()); assertNotNull(shardId); assertEquals(shard, shardId.getId()); @@ -115,7 +127,7 @@ public void testPartitionedIndex() { Set shardSet = new HashSet<>(); for (int k = 0; k < 150; k++) { String id = randomUnicodeOfLengthBetween(1, 50); - shardSet.add(indexRouting.shardId(id, routing)); + shardSet.add(shardIdFromSimple(indexRouting, id, routing)); } assertThat(shardSet, hasSize(partitionSize)); @@ -183,7 +195,7 @@ public void testPartitionedIndexShrunk() { String id = idEntry.getKey(); int shard = idEntry.getValue(); - assertEquals(shard, indexRouting.shardId(id, routing)); + assertEquals(shard, shardIdFromSimple(indexRouting, id, routing)); } } } @@ -235,7 +247,7 @@ public void testPartitionedIndexBWC() { String id = idEntry.getKey(); int shard = idEntry.getValue(); - assertEquals(shard, indexRouting.shardId(id, routing)); + assertEquals(shard, shardIdFromSimple(indexRouting, id, routing)); } } } @@ -349,8 +361,235 @@ public void testBWC() { ); for (Map.Entry entry : termToShard.entrySet()) { String key = entry.getKey(); - int shard = randomBoolean() ? indexRouting.shardId(key, null) : indexRouting.shardId(randomAlphaOfLength(5), key); - assertEquals(shard, entry.getValue().intValue()); + int shardId; + switch (between(0, 2)) { + case 0: + shardId = shardIdFromSimple(indexRouting, key, null); + break; + case 1: + shardId = shardIdFromSimple(indexRouting, randomAlphaOfLength(5), key); + break; + case 2: + AtomicInteger s = new AtomicInteger(-1); + indexRouting.collectSearchShards(key, r -> { + int old = s.getAndSet(r); + assertThat("only called once", old, equalTo(-1)); + }); + shardId = s.get(); + break; + default: + throw new AssertionError("invalid option"); + } + assertEquals(shardId, entry.getValue().intValue()); + } + } + + /** + * 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) { + switch (between(0, 3)) { + case 0: + return indexRouting.indexShard(randomBoolean(), key, routing, null, null); + case 1: + return indexRouting.updateShard(key, routing); + case 2: + return indexRouting.deleteShard(key, routing); + case 3: + return indexRouting.getShard(key, routing); + default: + throw new AssertionError("invalid option"); + } + } + + public void testRoutingPathSpecifiedId() throws IOException { + IndexRouting routing = indexRoutingForPath(between(1, 5), randomAlphaOfLength(5)); + Exception e = expectThrows( + IllegalArgumentException.class, + () -> routing.indexShard(false, randomAlphaOfLength(5), null, XContentType.JSON, source(Map.of())) + ); + assertThat( + e.getMessage(), + equalTo("indexing with a specified id is not supported because the destination index [test] is in time series mode") + ); + } + + public void testRoutingPathSpecifiedRouting() throws IOException { + IndexRouting routing = indexRoutingForPath(between(1, 5), randomAlphaOfLength(5)); + Exception e = expectThrows( + IllegalArgumentException.class, + () -> routing.indexShard(true, null, randomAlphaOfLength(5), XContentType.JSON, source(Map.of())) + ); + assertThat( + e.getMessage(), + equalTo("indexing with a specified routing is not supported because the destination index [test] is in time series mode") + ); + } + + public void testRoutingPathEmptySource() throws IOException { + IndexRouting routing = indexRoutingForPath(between(1, 5), randomAlphaOfLength(5)); + Exception e = expectThrows( + IllegalArgumentException.class, + () -> routing.indexShard(true, randomAlphaOfLength(5), null, XContentType.JSON, source(Map.of())) + ); + assertThat(e.getMessage(), equalTo("Error extracting routing: source didn't contain any routing fields")); + } + + public void testRoutingPathMismatchSource() throws IOException { + IndexRouting routing = indexRoutingForPath(between(1, 5), "foo"); + Exception e = expectThrows( + IllegalArgumentException.class, + () -> routing.indexShard(true, randomAlphaOfLength(5), null, XContentType.JSON, source(Map.of("bar", "dog"))) + ); + assertThat(e.getMessage(), equalTo("Error extracting routing: source didn't contain any routing fields")); + } + + public void testRoutingPathUpdate() throws IOException { + IndexRouting routing = indexRoutingForPath(between(1, 5), "foo"); + Exception e = expectThrows( + IllegalArgumentException.class, + () -> routing.updateShard(randomAlphaOfLength(5), randomBoolean() ? null : randomAlphaOfLength(5)) + ); + assertThat(e.getMessage(), equalTo("update is not supported because the destination index [test] is in time series mode")); + } + + public void testRoutingPathDelete() throws IOException { + IndexRouting routing = indexRoutingForPath(between(1, 5), "foo"); + Exception e = expectThrows( + IllegalArgumentException.class, + () -> routing.deleteShard(randomAlphaOfLength(5), randomBoolean() ? null : randomAlphaOfLength(5)) + ); + assertThat(e.getMessage(), equalTo("delete is not supported because the destination index [test] is in time series mode")); + } + + public void testRoutingPathGet() throws IOException { + IndexRouting routing = indexRoutingForPath(between(1, 5), "foo"); + Exception e = expectThrows( + IllegalArgumentException.class, + () -> routing.getShard(randomAlphaOfLength(5), randomBoolean() ? null : randomAlphaOfLength(5)) + ); + assertThat(e.getMessage(), equalTo("get is not supported because the destination index [test] is in time series mode")); + } + + public void testRoutingPathCollectSearchWithRouting() throws IOException { + IndexRouting routing = indexRoutingForPath(between(1, 5), "foo"); + Exception e = expectThrows(IllegalArgumentException.class, () -> routing.collectSearchShards(randomAlphaOfLength(5), null)); + assertThat( + e.getMessage(), + equalTo("searching with a specified routing is not supported because the destination index [test] is in time series mode") + ); + } + + public void testRoutingPathOneTopLevel() throws IOException { + int shards = between (2, 1000); + IndexRouting routing = indexRoutingForPath(shards, "foo"); + assertIndexShard(routing, Map.of("foo", "cat", "bar", "dog"), Math.floorMod(hash(List.of("foo", "cat")), shards)); + } + + public void testRoutingPathManyTopLevel() throws IOException { + int shards = between (2, 1000); + IndexRouting routing = indexRoutingForPath(shards, "f*"); + assertIndexShard( + routing, + Map.of("foo", "cat", "bar", "dog", "foa", "a", "fob", "b"), + Math.floorMod(hash(List.of("foa", "a", "fob", "b", "foo", "cat")), shards) // Note that the fields are sorted + ); + } + + public void testRoutingPathOneSub() throws IOException { + int shards = between (2, 1000); + IndexRouting routing = indexRoutingForPath(shards, "foo.*"); + assertIndexShard( + routing, + Map.of("foo", Map.of("bar", "cat"), "baz", "dog"), + Math.floorMod(hash(List.of("foo", List.of("bar", "cat"))), shards) + ); + } + + public void testRoutingPathManySubs() throws IOException { + int shards = between (2, 1000); + IndexRouting routing = indexRoutingForPath(shards, "foo.*,bar.*,baz.*"); + assertIndexShard( + routing, + Map.of("foo", Map.of("a", "cat"), "bar", Map.of("thing", "yay", "this", "too")), + Math.floorMod(hash(List.of("bar", List.of("thing", "yay", "this", "too"), "foo", List.of("a", "cat"))), shards) + ); + } + + public void testRoutingPathBwc() throws IOException { + Version version = VersionUtils.randomIndexCompatibleVersion(random()); + IndexRouting routing = indexRoutingForPath(version, 8, "dim.*,other.*,top"); + /* + * These when we first added routing_path. If these values change + * time series will be routed to unexpected shards. You may modify + * them with a new index created version, but when you do you must + * copy this test and patch the versions at the top. Because newer + * versions of Elasticsearch must continue to route based on the + * version on the index. + */ + assertIndexShard(routing, Map.of("dim", Map.of("a", "a")), 0); + assertIndexShard(routing, Map.of("dim", Map.of("a", "b")), 5); + assertIndexShard(routing, Map.of("dim", Map.of("c", "d")), 4); + assertIndexShard(routing, Map.of("other", Map.of("a", "a")), 5); + assertIndexShard(routing, Map.of("top", "a"), 3); + assertIndexShard(routing, Map.of("dim", Map.of("c", "d"), "top", "b"), 2); + } + + private IndexRouting indexRoutingForPath(int shards, String path) { + return indexRoutingForPath(Version.CURRENT, shards, path); + } + + private IndexRouting indexRoutingForPath(Version createdVersion, int shards, String path) { + return IndexRouting.fromIndexMetadata( + IndexMetadata.builder("test") + .settings(settings(createdVersion).put(IndexMetadata.INDEX_ROUTING_PATH.getKey(), path)) + .numberOfShards(shards) + .numberOfReplicas(1) + .build() + ); + } + + private void assertIndexShard(IndexRouting routing, Map source, int id) throws IOException { + assertThat(routing.indexShard(true, randomAlphaOfLength(5), null, XContentType.JSON, source(source)), equalTo(id)); + } + + private BytesReference source(Map doc) throws IOException { + return BytesReference.bytes( + JsonXContent.contentBuilder() + .copyCurrentStructure( + new MapXContentParser( + NamedXContentRegistry.EMPTY, + DeprecationHandler.IGNORE_DEPRECATIONS, + doc, + randomFrom(XContentType.values()) + ) + ) + ); + } + + /** + * Build the hash we expect from the extracter. + */ + private int hash(List keysAndValues) { + assertThat(keysAndValues.size() % 2, equalTo(0)); + int hash = 0; + for (int i = 0; i < keysAndValues.size(); i += 2) { + int thisHash = Murmur3HashFunction.hash(keysAndValues.get(i).toString()) ^ expectedValueHash(keysAndValues.get(i + 1)); + hash = hash * 31 + thisHash; + } + return hash; + } + + private int expectedValueHash(Object value) { + if (value instanceof List) { + return hash((List) value); } + if (value instanceof String) { + return Murmur3HashFunction.hash((String) value); + } + throw new IllegalArgumentException("Unsupported value: " + value); } + } diff --git a/server/src/test/java/org/elasticsearch/index/shard/ShardSplittingQueryTests.java b/server/src/test/java/org/elasticsearch/index/shard/ShardSplittingQueryTests.java index 11fdbbe7dccd7..768694da96a4d 100644 --- a/server/src/test/java/org/elasticsearch/index/shard/ShardSplittingQueryTests.java +++ b/server/src/test/java/org/elasticsearch/index/shard/ShardSplittingQueryTests.java @@ -55,7 +55,7 @@ public void testSplitOnID() throws IOException { int targetShardId = randomIntBetween(0, numShards-1); boolean hasNested = randomBoolean(); for (int j = 0; j < numDocs; j++) { - int shardId = IndexRouting.fromIndexMetadata(metadata).shardId(Integer.toString(j), null); + int shardId = IndexRouting.fromIndexMetadata(metadata).getShard(Integer.toString(j), null); if (hasNested) { List> docs = new ArrayList<>(); int numNested = randomIntBetween(0, 10); @@ -103,7 +103,7 @@ public void testSplitOnRouting() throws IOException { int targetShardId = randomIntBetween(0, numShards-1); for (int j = 0; j < numDocs; j++) { String routing = randomRealisticUnicodeOfCodepointLengthBetween(1, 5); - final int shardId = IndexRouting.fromIndexMetadata(metadata).shardId(null, routing); + final int shardId = IndexRouting.fromIndexMetadata(metadata).getShard(null, routing); if (hasNested) { List> docs = new ArrayList<>(); int numNested = randomIntBetween(0, 10); @@ -154,7 +154,7 @@ public void testSplitOnIdOrRouting() throws IOException { final int shardId; if (randomBoolean()) { String routing = randomRealisticUnicodeOfCodepointLengthBetween(1, 5); - shardId = IndexRouting.fromIndexMetadata(metadata).shardId(null, routing); + shardId = IndexRouting.fromIndexMetadata(metadata).getShard(null, routing); rootDoc = Arrays.asList( new StringField(IdFieldMapper.NAME, Uid.encodeId(Integer.toString(j)), Field.Store.YES), new StringField(RoutingFieldMapper.NAME, routing, Field.Store.YES), @@ -162,7 +162,7 @@ public void testSplitOnIdOrRouting() throws IOException { sequenceIDFields.primaryTerm ); } else { - shardId = IndexRouting.fromIndexMetadata(metadata).shardId(Integer.toString(j), null); + shardId = IndexRouting.fromIndexMetadata(metadata).getShard(Integer.toString(j), null); rootDoc = Arrays.asList( new StringField(IdFieldMapper.NAME, Uid.encodeId(Integer.toString(j)), Field.Store.YES), new SortedNumericDocValuesField("shard_id", shardId), @@ -209,7 +209,7 @@ public void testSplitOnRoutingPartitioned() throws IOException { int targetShardId = randomIntBetween(0, numShards-1); for (int j = 0; j < numDocs; j++) { String routing = randomRealisticUnicodeOfCodepointLengthBetween(1, 5); - final int shardId = IndexRouting.fromIndexMetadata(metadata).shardId(Integer.toString(j), routing); + final int shardId = IndexRouting.fromIndexMetadata(metadata).getShard(Integer.toString(j), routing); if (hasNested) { List> docs = new ArrayList<>(); diff --git a/server/src/test/java/org/elasticsearch/index/shard/StoreRecoveryTests.java b/server/src/test/java/org/elasticsearch/index/shard/StoreRecoveryTests.java index 3e4642df368df..da8a8339d4f61 100644 --- a/server/src/test/java/org/elasticsearch/index/shard/StoreRecoveryTests.java +++ b/server/src/test/java/org/elasticsearch/index/shard/StoreRecoveryTests.java @@ -198,12 +198,12 @@ public void testSplitShard() throws IOException { assertEquals( "value has wrong shards: " + value, targetShardId, - IndexRouting.fromIndexMetadata(metadata).shardId(value, null) + IndexRouting.fromIndexMetadata(metadata).getShard(value, null) ); } for (int i = 0; i < numDocs; i++) { ref = new BytesRef(Integer.toString(i)); - int shardId = IndexRouting.fromIndexMetadata(metadata).shardId(ref.utf8ToString(), null); + int shardId = IndexRouting.fromIndexMetadata(metadata).getShard(ref.utf8ToString(), null); if (shardId == targetShardId) { assertTrue(ref.utf8ToString() + " is missing", terms.iterator().seekExact(ref)); } else { diff --git a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java index fe87536438061..eba39106f8405 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java @@ -2174,14 +2174,10 @@ synchronized String routingKeyForShard(Index index, int shard, Random random) { assertThat(indexService.getIndexSettings().getSettings().getAsInt(IndexMetadata.SETTING_NUMBER_OF_SHARDS, -1), greaterThan(shard)); ClusterState clusterState = clusterService.state(); - OperationRouting operationRouting = clusterService.operationRouting(); IndexRouting indexRouting = IndexRouting.fromIndexMetadata(clusterState.metadata().getIndexSafe(index)); while (true) { String routing = RandomStrings.randomAsciiLettersOfLength(random, 10); - final int targetShard = operationRouting - .indexShards(clusterState, index.getName(), indexRouting, null, routing) - .shardId().getId(); - if (shard == targetShard) { + if (shard == indexRouting.indexShard(false, null, routing, null, null)) { return routing; } } From 9828ea0591ced2c73886e57946e81a5315b4ba7b Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Wed, 15 Sep 2021 16:33:14 -0400 Subject: [PATCH 02/31] Moar skip --- .../test/smoke_test_multinode/20_tsdb_consistency.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/qa/smoke-test-multinode/src/test/resources/rest-api-spec/test/smoke_test_multinode/20_tsdb_consistency.yml b/qa/smoke-test-multinode/src/test/resources/rest-api-spec/test/smoke_test_multinode/20_tsdb_consistency.yml index d05cb27544d77..363f3fd463292 100644 --- a/qa/smoke-test-multinode/src/test/resources/rest-api-spec/test/smoke_test_multinode/20_tsdb_consistency.yml +++ b/qa/smoke-test-multinode/src/test/resources/rest-api-spec/test/smoke_test_multinode/20_tsdb_consistency.yml @@ -2,6 +2,10 @@ # rest-api-spec we would, but it requires painless. setup: + - skip: + version: " - 7.99.99" + reason: introduced in 8.0.0 to be backported to 7.16.0 + - do: indices.create: index: test From 653c7f0ebf7e8001cc21a7b0fbf09d7b5b05bf42 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Wed, 15 Sep 2021 17:10:47 -0400 Subject: [PATCH 03/31] tsdb survives full cluster restart --- .../upgrades/FullClusterRestartIT.java | 185 ++++++++++++++++-- .../cluster/routing/IndexRouting.java | 2 +- 2 files changed, 169 insertions(+), 18 deletions(-) diff --git a/qa/full-cluster-restart/src/test/java/org/elasticsearch/upgrades/FullClusterRestartIT.java b/qa/full-cluster-restart/src/test/java/org/elasticsearch/upgrades/FullClusterRestartIT.java index d6dcd01a3e053..0ed4ed95b9371 100644 --- a/qa/full-cluster-restart/src/test/java/org/elasticsearch/upgrades/FullClusterRestartIT.java +++ b/qa/full-cluster-restart/src/test/java/org/elasticsearch/upgrades/FullClusterRestartIT.java @@ -26,6 +26,7 @@ import org.elasticsearch.core.Booleans; import org.elasticsearch.core.CheckedFunction; import org.elasticsearch.index.IndexSettings; +import org.elasticsearch.index.mapper.DateFieldMapper; import org.elasticsearch.rest.action.admin.indices.RestPutIndexTemplateAction; import org.elasticsearch.test.NotEqualMessageBuilder; import org.elasticsearch.test.XContentTestUtils; @@ -45,12 +46,17 @@ import java.util.Locale; import java.util.Map; import java.util.Set; +import java.util.concurrent.TimeUnit; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.IntStream; +import static io.github.nik9000.mapmatcher.MapMatcher.assertMap; +import static io.github.nik9000.mapmatcher.MapMatcher.matchesMap; import static java.util.Collections.emptyMap; import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; +import static java.util.stream.Collectors.toList; import static org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.SYSTEM_INDEX_ENFORCEMENT_VERSION; import static org.elasticsearch.cluster.routing.UnassignedInfo.INDEX_DELAYED_NODE_LEFT_TIMEOUT_SETTING; import static org.elasticsearch.cluster.routing.allocation.decider.MaxRetryAllocationDecider.SETTING_ALLOCATION_MAX_RETRY; @@ -128,6 +134,7 @@ public void testSearch() throws Exception { count, true, true, + randomBoolean(), i -> JsonXContent.contentBuilder().startObject() .field("string", randomAlphaOfLength(10)) .field("int", randomInt(100)) @@ -151,7 +158,7 @@ public void testSearch() throws Exception { assertStoredBinaryFields(count); } - public void testNewReplicasWork() throws Exception { + public void testNewReplicas() throws Exception { if (isRunningAgainstOldCluster()) { XContentBuilder mappingsAndSettings = jsonBuilder(); mappingsAndSettings.startObject(); @@ -180,7 +187,12 @@ public void testNewReplicasWork() throws Exception { int numDocs = randomIntBetween(2000, 3000); indexRandomDocuments( - numDocs, true, false, i -> JsonXContent.contentBuilder().startObject().field("field", "value").endObject()); + numDocs, + true, + false, + randomBoolean(), + i -> JsonXContent.contentBuilder().startObject().field("field", "value").endObject() + ); logger.info("Refreshing [{}]", index); client().performRequest(new Request("POST", "/" + index + "/_refresh")); } else { @@ -189,7 +201,7 @@ public void testNewReplicasWork() throws Exception { logger.debug("--> creating [{}] replicas for index [{}]", numReplicas, index); Request setNumberOfReplicas = new Request("PUT", "/" + index + "/_settings"); setNumberOfReplicas.setJsonEntity("{ \"index\": { \"number_of_replicas\" : " + numReplicas + " }}"); - Response response = client().performRequest(setNumberOfReplicas); + client().performRequest(setNumberOfReplicas); ensureGreenLongWait(index); @@ -210,6 +222,124 @@ public void testNewReplicasWork() throws Exception { } } + public void testSearchTimeSeriesMode() throws Exception { + assumeTrue("time series mode introduced in 8.0.0 to be backported to 7.15.0", getOldClusterVersion().onOrAfter(Version.V_8_0_0)); + int numDocs; + if (isRunningAgainstOldCluster()) { + numDocs = createTimeSeriesModeIndex(1); + } else { + numDocs = countOfIndexedRandomDocuments(); + } + assertCountAll(numDocs); + Request request = new Request("GET", "/" + index + "/_search"); + XContentBuilder body = jsonBuilder().startObject(); + body.field("size", 0); + body.startObject("aggs").startObject("check").startObject("scripted_metric"); + { + body.field("init_script", "state.timeSeries = new HashSet()"); + body.field("map_script", "state.timeSeries.add(doc['dim'].value)"); + body.field("combine_script", "return state.timeSeries"); + StringBuilder reduceScript = new StringBuilder(); + reduceScript.append("Set timeSeries = new TreeSet();"); + reduceScript.append("for (s in states) {"); + reduceScript.append(" for (ts in s) {"); + reduceScript.append(" boolean newTs = timeSeries.add(ts);"); + reduceScript.append(" if (false == newTs) {"); + reduceScript.append(" throw new IllegalArgumentException(ts + ' appeared in two shards');"); + reduceScript.append(" }"); + reduceScript.append(" }"); + reduceScript.append("}"); + reduceScript.append("return timeSeries;"); + body.field("reduce_script", reduceScript.toString()); + } + body.endObject().endObject().endObject(); + body.endObject(); + request.setJsonEntity(Strings.toString(body)); + Map response = entityAsMap(client().performRequest(request)); + assertMap(response, matchesMap().extraOk() + .entry( + "hits", + matchesMap().extraOk().entry("total", Map.of("value", numDocs, "relation", "eq"))) + .entry("aggregations", Map.of("check", Map.of("value", IntStream.range(0, 10).mapToObj(i -> "dim" + i).collect(toList())))) + ); + } + + public void testNewReplicasTimeSeriesMode() throws Exception { + assumeTrue("time series mode introduced in 8.0.0 to be backported to 7.15.0", getOldClusterVersion().onOrAfter(Version.V_8_0_0)); + if (isRunningAgainstOldCluster()) { + createTimeSeriesModeIndex(0); + } else { + final int numReplicas = 1; + final long startTime = System.currentTimeMillis(); + logger.debug("--> creating [{}] replicas for index [{}]", numReplicas, index); + Request setNumberOfReplicas = new Request("PUT", "/" + index + "/_settings"); + setNumberOfReplicas.setJsonEntity("{ \"index\": { \"number_of_replicas\" : " + numReplicas + " }}"); + client().performRequest(setNumberOfReplicas); + + ensureGreenLongWait(index); + + logger.debug("--> index [{}] is green, took [{}] ms", index, (System.currentTimeMillis() - startTime)); + Map recoverRsp = entityAsMap(client().performRequest(new Request("GET", "/" + index + "/_recovery"))); + logger.debug("--> recovery status:\n{}", recoverRsp); + + Set counts = new HashSet<>(); + for (String node : dataNodes(index, client())) { + Request search = new Request("GET", "/" + index + "/_search"); + search.addParameter("preference", "_only_nodes:" + node); + Map responseBody = entityAsMap(client().performRequest(search)); + assertNoFailures(responseBody); + int hits = extractTotalHits(responseBody); + counts.add(hits); + } + assertEquals("All nodes should have a consistent number of documents", 1, counts.size()); + } + } + + private int createTimeSeriesModeIndex(int replicas) throws IOException { + XContentBuilder mappingsAndSettings = jsonBuilder(); + mappingsAndSettings.startObject(); + { + mappingsAndSettings.startObject("settings"); + mappingsAndSettings.field("number_of_shards", 1); + mappingsAndSettings.field("number_of_replicas", replicas); + mappingsAndSettings.field("mode", "time_series"); + mappingsAndSettings.field("routing_path", "dim"); + mappingsAndSettings.endObject(); + } + { + mappingsAndSettings.startObject("mappings"); + mappingsAndSettings.startObject("properties"); + { + mappingsAndSettings.startObject("@timestamp").field("type", "date").endObject(); + mappingsAndSettings.startObject("dim").field("type", "keyword").field("dimension", true).endObject(); + } + mappingsAndSettings.endObject(); + mappingsAndSettings.endObject(); + } + mappingsAndSettings.endObject(); + + Request createIndex = new Request("PUT", "/" + index); + createIndex.setJsonEntity(Strings.toString(mappingsAndSettings)); + client().performRequest(createIndex); + + int numDocs = randomIntBetween(2000, 3000); + long basetime = DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER.parseMillis("2021-01-01T00:00:00Z"); + indexRandomDocuments( + numDocs, + true, + true, + false, + i -> JsonXContent.contentBuilder() + .startObject() + .field("@timestamp", basetime + TimeUnit.MINUTES.toMillis(i)) + .field("dim", "dim" + (i % 10)) + .endObject() + ); + logger.info("Refreshing [{}]", index); + client().performRequest(new Request("POST", "/" + index + "/_refresh")); + return numDocs; + } + public void testClusterState() throws Exception { if (isRunningAgainstOldCluster()) { XContentBuilder mappingsAndSettings = jsonBuilder(); @@ -289,7 +419,12 @@ public void testShrink() throws IOException { numDocs = randomIntBetween(512, 1024); indexRandomDocuments( - numDocs, true, true, i -> JsonXContent.contentBuilder().startObject().field("field", "value").endObject()); + numDocs, + true, + true, + randomBoolean(), + i -> JsonXContent.contentBuilder().startObject().field("field", "value").endObject() + ); ensureGreen(index); // wait for source index to be available on both nodes before starting shrink @@ -362,6 +497,7 @@ public void testShrinkAfterUpgrade() throws IOException { numDocs, true, true, + randomBoolean(), i -> JsonXContent.contentBuilder().startObject().field("field", "value").endObject() ); } else { @@ -459,14 +595,18 @@ public void testRollover() throws IOException { assertEquals(expectedCount, extractTotalHits(count)); } + void assertCountAll(int count) throws IOException { + Map response = entityAsMap(client().performRequest(new Request("GET", "/" + index + "/_search"))); + assertNoFailures(response); + int numDocs = extractTotalHits(response); + logger.info("Found {} in old index", numDocs); + assertEquals(count, numDocs); + } + void assertBasicSearchWorks(int count) throws IOException { logger.info("--> testing basic search"); { - Map response = entityAsMap(client().performRequest(new Request("GET", "/" + index + "/_search"))); - assertNoFailures(response); - int numDocs = extractTotalHits(response); - logger.info("Found {} in old index", numDocs); - assertEquals(count, numDocs); + assertCountAll(count); } logger.info("--> testing basic search with sort"); @@ -672,7 +812,7 @@ public void testRecovery() throws Exception { } final String mappings = randomBoolean() ? "\"_source\": { \"enabled\": false}" : null; createIndex(index, settings.build(), mappings); - indexRandomDocuments(count, true, true, i -> jsonBuilder().startObject().field("field", "value").endObject()); + indexRandomDocuments(count, true, true, true, i -> jsonBuilder().startObject().field("field", "value").endObject()); // make sure all recoveries are done ensureGreen(index); @@ -689,6 +829,7 @@ public void testRecovery() throws Exception { count / 10, false, // flushing here would invalidate the whole thing false, + true, i -> jsonBuilder().startObject().field("field", "value").endObject() ); } @@ -782,7 +923,7 @@ public void testSnapshotRestore() throws IOException { settings.put(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), randomBoolean()); } createIndex(index, settings.build()); - indexRandomDocuments(count, true, true, i -> jsonBuilder().startObject().field("field", "value").endObject()); + indexRandomDocuments(count, true, true, randomBoolean(), i -> jsonBuilder().startObject().field("field", "value").endObject()); } else { count = countOfIndexedRandomDocuments(); } @@ -1136,18 +1277,17 @@ private void checkSnapshot(final String snapshotName, final int count, final Ver } } - // TODO tests for upgrades after shrink. We've had trouble with shrink in the past. - private void indexRandomDocuments( final int count, final boolean flushAllowed, final boolean saveInfo, + final boolean specifyId, final CheckedFunction docSupplier) throws IOException { logger.info("Indexing {} random documents", count); for (int i = 0; i < count; i++) { logger.debug("Indexing document [{}]", i); - Request createDocument = new Request("POST", "/" + index + "/_doc/" + i); + Request createDocument = new Request("POST", "/" + index + "/_doc/" + (specifyId ? i : "")); createDocument.setJsonEntity(Strings.toString(docSupplier.apply(i))); client().performRequest(createDocument); if (rarely()) { @@ -1489,7 +1629,13 @@ public void testEnableSoftDeletesOnRestore() throws Exception { createIndex(index, settings.build()); ensureGreen(index); int numDocs = randomIntBetween(0, 100); - indexRandomDocuments(numDocs, true, true, i -> jsonBuilder().startObject().field("field", "value").endObject()); + indexRandomDocuments( + numDocs, + true, + true, + randomBoolean(), + i -> jsonBuilder().startObject().field("field", "value").endObject() + ); // create repo XContentBuilder repoConfig = JsonXContent.contentBuilder().startObject(); { @@ -1543,7 +1689,13 @@ public void testForbidDisableSoftDeletesOnRestore() throws Exception { createIndex(index, settings.build()); ensureGreen(index); int numDocs = randomIntBetween(0, 100); - indexRandomDocuments(numDocs, true, true, i -> jsonBuilder().startObject().field("field", "value").endObject()); + indexRandomDocuments( + numDocs, + true, + true, + randomBoolean(), + i -> jsonBuilder().startObject().field("field", "value").endObject() + ); // create repo XContentBuilder repoConfig = JsonXContent.contentBuilder().startObject(); { @@ -1629,5 +1781,4 @@ public static void assertNumHits(String index, int numHits, int totalShards) thr assertThat(XContentMapValues.extractValue("_shards.successful", resp), equalTo(totalShards)); assertThat(extractTotalHits(resp), equalTo(numHits)); } - } 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 133aaa0f729c0..6ecdc97ab1858 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java @@ -261,7 +261,7 @@ private static int extractObject(XContentParser source) throws IOException { int firstHash = extractItem(source); if (source.currentToken() == Token.END_OBJECT) { // Just one routing key in this object - // Use ^ like Map.Entery's hashcode + // Use ^ like Map.Entry's hashcode return Murmur3HashFunction.hash(firstFieldName) ^ firstHash; } String[] fields = new String[] { firstFieldName, null }; From d762a0361b7c773dd05ed28eb8ec9fbe0f5826a3 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 16 Sep 2021 10:35:17 -0400 Subject: [PATCH 04/31] Remove auto generated id rejection We do want to reject these documents but let's sae that for a follow up change. --- .../test/tsdb/20_unsupported_operations.yml | 39 ------------------- .../action/bulk/TransportBulkAction.java | 2 - .../cluster/routing/IndexRouting.java | 27 ++----------- .../cluster/routing/IndexRoutingTests.java | 22 +++-------- .../test/InternalTestCluster.java | 2 +- 5 files changed, 9 insertions(+), 83 deletions(-) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/20_unsupported_operations.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/20_unsupported_operations.yml index 1610978f3dcf2..cc22eb22cb819 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/20_unsupported_operations.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/20_unsupported_operations.yml @@ -84,45 +84,6 @@ index with specified id: tx: 2001818691 rx: 802133794 ---- -index with specified id over _bulk: - - skip: - version: " - 7.99.99" - reason: introduced in 8.0.0 to be backported to 7.16.0 - - - do: - bulk: - refresh: true - index: test - body: - - '{"index": {"_id": "foo"}}' - - '{"@timestamp": "2021-04-28T18:50:04.467Z", "metricset": "pod", "k8s": {"pod": {"name": "cat", "uid":"947e4ced-1786-4e53-9e0c-5c447e959507", "ip": "10.10.55.1", "network": {"tx": 2001818691, "rx": 802133794}}}}' - - match: {items.0.index.error.reason: "indexing with a specified id is not supported because the destination index [test] is in time series mode"} - ---- -index with specified routing: - - skip: - version: " - 7.99.99" - reason: introduced in 8.0.0 to be backported to 7.16.0 - - - do: - catch: /indexing with a specified routing is not supported because the destination index \[test\] is in time series mode/ - index: - index: test - routing: foo - body: - doc: - "@timestamp": "2021-04-28T18:35:24.467Z" - metricset: "pod" - k8s: - pod: - name: "cat" - uid: "947e4ced-1786-4e53-9e0c-5c447e959507" - ip: "10.10.55.1" - network: - tx: 2001818691 - rx: 802133794 - --- index with specified routing over _bulk: - skip: 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 49293c4185b07..43f46a3d50dde 100644 --- a/server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java +++ b/server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java @@ -467,10 +467,8 @@ protected void doRun() { MappingMetadata mappingMd = indexMetadata.mapping(); Version indexCreated = indexMetadata.getCreationVersion(); indexRequest.resolveRouting(metadata); - boolean idIsAutoGenerated = indexRequest.id() == null; indexRequest.process(indexCreated, mappingMd, concreteIndex.getName()); shardId = indexRouting.indexShard( - idIsAutoGenerated, docWriteRequest.id(), docWriteRequest.routing(), indexRequest.getContentType(), 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 6ecdc97ab1858..cb95ce6739ef3 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java @@ -69,13 +69,7 @@ private IndexRouting(int routingNumShards, int routingFactor) { * Called when indexing a document to generate the shard id that should contain * a document with the provided parameters. */ - public abstract int indexShard( - boolean idIsAutoGenerated, - String id, - @Nullable String routing, - XContentType sourceType, - BytesReference source - ); + public abstract int indexShard(String id, @Nullable String routing, XContentType sourceType, BytesReference source); /** * Called when updating a document to generate the shard id that should contain @@ -132,13 +126,7 @@ private abstract static class IdAndRoutingOnly extends IndexRouting { protected abstract int shardId(String id, @Nullable String routing); @Override - public int indexShard( - boolean idIsAutoGenerated, - String id, - @Nullable String routing, - XContentType sourceType, - BytesReference source - ) { + public int indexShard(String id, @Nullable String routing, XContentType sourceType, BytesReference source) { return shardId(id, routing); } @@ -217,16 +205,7 @@ private static class ExtractFromSource extends IndexRouting { } @Override - public int indexShard( - boolean idIsAutoGenerated, - String id, - @Nullable String routing, - XContentType sourceType, - BytesReference source - ) { - if (idIsAutoGenerated == false) { - throw new IllegalArgumentException(error("indexing with a specified id")); - } + public int indexShard(String id, @Nullable String routing, XContentType sourceType, BytesReference source) { if (routing != null) { throw new IllegalArgumentException(error("indexing with a specified routing")); } 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 bde7ed51df163..4c19700c69769 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/IndexRoutingTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/IndexRoutingTests.java @@ -392,7 +392,7 @@ public void testBWC() { private int shardIdFromSimple(IndexRouting indexRouting, String key, @Nullable String routing) { switch (between(0, 3)) { case 0: - return indexRouting.indexShard(randomBoolean(), key, routing, null, null); + return indexRouting.indexShard(key, routing, null, null); case 1: return indexRouting.updateShard(key, routing); case 2: @@ -404,23 +404,11 @@ private int shardIdFromSimple(IndexRouting indexRouting, String key, @Nullable S } } - public void testRoutingPathSpecifiedId() throws IOException { - IndexRouting routing = indexRoutingForPath(between(1, 5), randomAlphaOfLength(5)); - Exception e = expectThrows( - IllegalArgumentException.class, - () -> routing.indexShard(false, randomAlphaOfLength(5), null, XContentType.JSON, source(Map.of())) - ); - assertThat( - e.getMessage(), - equalTo("indexing with a specified id is not supported because the destination index [test] is in time series mode") - ); - } - public void testRoutingPathSpecifiedRouting() throws IOException { IndexRouting routing = indexRoutingForPath(between(1, 5), randomAlphaOfLength(5)); Exception e = expectThrows( IllegalArgumentException.class, - () -> routing.indexShard(true, null, randomAlphaOfLength(5), XContentType.JSON, source(Map.of())) + () -> routing.indexShard(null, randomAlphaOfLength(5), XContentType.JSON, source(Map.of())) ); assertThat( e.getMessage(), @@ -432,7 +420,7 @@ public void testRoutingPathEmptySource() throws IOException { IndexRouting routing = indexRoutingForPath(between(1, 5), randomAlphaOfLength(5)); Exception e = expectThrows( IllegalArgumentException.class, - () -> routing.indexShard(true, randomAlphaOfLength(5), null, XContentType.JSON, source(Map.of())) + () -> routing.indexShard(randomAlphaOfLength(5), null, XContentType.JSON, source(Map.of())) ); assertThat(e.getMessage(), equalTo("Error extracting routing: source didn't contain any routing fields")); } @@ -441,7 +429,7 @@ public void testRoutingPathMismatchSource() throws IOException { IndexRouting routing = indexRoutingForPath(between(1, 5), "foo"); Exception e = expectThrows( IllegalArgumentException.class, - () -> routing.indexShard(true, randomAlphaOfLength(5), null, XContentType.JSON, source(Map.of("bar", "dog"))) + () -> routing.indexShard(randomAlphaOfLength(5), null, XContentType.JSON, source(Map.of("bar", "dog"))) ); assertThat(e.getMessage(), equalTo("Error extracting routing: source didn't contain any routing fields")); } @@ -552,7 +540,7 @@ private IndexRouting indexRoutingForPath(Version createdVersion, int shards, Str } private void assertIndexShard(IndexRouting routing, Map source, int id) throws IOException { - assertThat(routing.indexShard(true, randomAlphaOfLength(5), null, XContentType.JSON, source(source)), equalTo(id)); + assertThat(routing.indexShard(randomAlphaOfLength(5), null, XContentType.JSON, source(source)), equalTo(id)); } private BytesReference source(Map doc) throws IOException { diff --git a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java index eba39106f8405..6cb1a379edc97 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java @@ -2177,7 +2177,7 @@ synchronized String routingKeyForShard(Index index, int shard, Random random) { IndexRouting indexRouting = IndexRouting.fromIndexMetadata(clusterState.metadata().getIndexSafe(index)); while (true) { String routing = RandomStrings.randomAsciiLettersOfLength(random, 10); - if (shard == indexRouting.indexShard(false, null, routing, null, null)) { + if (shard == indexRouting.indexShard(null, routing, null, null)) { return routing; } } From 85faa613db5f7180b83d278e8dc054846c67dfee Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 16 Sep 2021 11:22:37 -0400 Subject: [PATCH 05/31] Simplify --- .../cluster/routing/IndexRouting.java | 81 +++++++------------ 1 file changed, 29 insertions(+), 52 deletions(-) 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 cb95ce6739ef3..84bdd0cf579e3 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java @@ -8,8 +8,6 @@ package org.elasticsearch.cluster.routing; -import org.apache.lucene.util.ArrayUtil; -import org.apache.lucene.util.IntroSorter; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.bytes.BytesReference; @@ -22,6 +20,9 @@ import org.elasticsearch.core.Nullable; import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; import java.util.List; import java.util.Set; import java.util.function.IntConsumer; @@ -243,22 +244,27 @@ private static int extractObject(XContentParser source) throws IOException { // Use ^ like Map.Entry's hashcode return Murmur3HashFunction.hash(firstFieldName) ^ firstHash; } - String[] fields = new String[] { firstFieldName, null }; - int[] hashes = new int[] { firstHash, 0 }; - int length = 1; + List hashes = new ArrayList<>(); + hashes.add(new NameAndHash(firstFieldName, firstHash)); do { ensureExpectedToken(Token.FIELD_NAME, source.currentToken(), source); String fieldName = source.currentName(); source.nextToken(); - if (length >= fields.length) { - fields = ArrayUtil.grow(fields, length + 1); - hashes = ArrayUtil.grow(hashes, fields.length); - } - fields[length] = fieldName; - hashes[length] = extractItem(source); - length++; + hashes.add(new NameAndHash(fieldName, extractItem(source))); } while (source.currentToken() != Token.END_OBJECT); - return convertManyHashesToOne(length, fields, hashes); + Collections.sort(hashes, Comparator.comparing(nameAndHash -> nameAndHash.name)); + /* + * This is the same as Arrays.hash(Map.Entry) but we're + * writing it out so for extra paranoia. Changing this will change how + * documents are routed and we don't want a jdk update that modifies Arrays + * or Map.Entry to sneak up on us. + */ + int hash = 0; + for (NameAndHash nameAndHash : hashes) { + int thisHash = Murmur3HashFunction.hash(nameAndHash.name) ^ nameAndHash.hash; + hash = 31 * hash + thisHash; + } + return hash; } private static int extractItem(XContentParser source) throws IOException { @@ -275,45 +281,6 @@ private static int extractItem(XContentParser source) throws IOException { throw new ParsingException(source.getTokenLocation(), "Routing values must be strings but found [{}]", source.currentToken()); } - private static int convertManyHashesToOne(int length, String[] fields, int[] hashes) { - new IntroSorter() { - String pivot; - - @Override - protected void swap(int i, int j) { - String field = fields[i]; - fields[i] = fields[j]; - fields[j] = field; - int hash = hashes[i]; - hashes[i] = hashes[j]; - hashes[j] = hash; - } - - @Override - protected void setPivot(int i) { - pivot = fields[i]; - } - - @Override - protected int comparePivot(int j) { - return pivot.compareTo(fields[j]); - } - }.sort(0, length); - /* - * This is the same as Arrays.hash(Map.Entry) but we're - * writing it out so we don't have to allocate the entries and for extra - * paranoia. Changing this will change how documents are routed and we - * don't want a jdk update that modifies Arrays or Map.Entry to sneak up - * on us. - */ - int hash = 0; - for (int i = 0; i < length; i++) { - int thisHash = Murmur3HashFunction.hash(fields[i]) ^ hashes[i]; - hash = 31 * hash + thisHash; - } - return hash; - } - @Override public int updateShard(String id, @Nullable String routing) { throw new IllegalArgumentException(error("update")); @@ -338,4 +305,14 @@ private String error(String operation) { return operation + " is not supported because the destination index [" + indexName + "] is in time series mode"; } } + + private static class NameAndHash { + private final String name; + private final int hash; + + NameAndHash(String name, int hash) { + this.name = name; + this.hash = hash; + } + } } From 1b398e535214ab82a58b4de5a836b9973151aac0 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 16 Sep 2021 12:26:45 -0400 Subject: [PATCH 06/31] Forbid routing_required --- .../groovy/elasticsearch.formatting.gradle | 2 + rest-api-spec/build.gradle | 2 + .../rest-api-spec/test/tsdb/10_settings.yml | 19 +++++++ .../test/tsdb/20_unsupported_operations.yml | 35 +++++++++++- .../cluster/metadata/AliasAction.java | 12 +++- .../metadata/MetadataIndexAliasesService.java | 7 ++- .../org/elasticsearch/index/IndexMode.java | 41 ++++++++++++- .../index/mapper/DocumentMapper.java | 1 + .../index/mapper/RoutingFieldMapper.java | 6 ++ .../index/TimeSeriesModeTests.java | 57 ++++++++++++++++++- 10 files changed, 171 insertions(+), 11 deletions(-) diff --git a/build-tools-internal/src/main/groovy/elasticsearch.formatting.gradle b/build-tools-internal/src/main/groovy/elasticsearch.formatting.gradle index 4c5195a7ac2ba..7e5be89bed6c9 100644 --- a/build-tools-internal/src/main/groovy/elasticsearch.formatting.gradle +++ b/build-tools-internal/src/main/groovy/elasticsearch.formatting.gradle @@ -207,7 +207,9 @@ subprojects { 'src/*/java/org/elasticsearch/action/admin/cluster/snapshots/**/*.java', 'src/test/java/org/elasticsearch/common/xcontent/support/AbstractFilteringTestCase.java', 'src/test/java/org/elasticsearch/common/xcontent/support/XContentMapValuesTests.java', + 'src/*/java/org/elasticsearch/index/IndexMode.java', 'src/*/java/org/elasticsearch/index/IndexRouting.java', + 'src/*/java/org/elasticsearch/index/TimeSeriesModeTests.java', 'src/*/java/org/elasticsearch/index/snapshots/**/*.java', 'src/*/java/org/elasticsearch/repositories/**/*.java', 'src/*/java/org/elasticsearch/search/aggregations/**/*.java', diff --git a/rest-api-spec/build.gradle b/rest-api-spec/build.gradle index a208a103b0726..606402ab1f799 100644 --- a/rest-api-spec/build.gradle +++ b/rest-api-spec/build.gradle @@ -101,6 +101,8 @@ tasks.named("yamlRestTestV7CompatTest").configure { 'search/340_type_query/type query', //#47207 type query throws exception in compatible mode 'search.aggregation/200_top_hits_metric/top_hits aggregation with sequence numbers', // #42809 the use nested path and filter sort throws an exception 'search/310_match_bool_prefix/multi_match multiple fields with cutoff_frequency throws exception', //#42654 cutoff_frequency, common terms are not supported. Throwing an exception + + 'tsdb/10_settings/enable', // Remove once we've backported #77731 ].join(',') } diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/10_settings.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/10_settings.yml index c3a3547f0a103..4cf2cf069a414 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/10_settings.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/10_settings.yml @@ -149,3 +149,22 @@ routing_path is not allowed in standard mode: settings: index: routing_path: foo + +--- +routing required: + - skip: + version: " - 7.99.99" + reason: introduced in 8.0.0 to be backported to 7.16.0 + + - do: + catch: /routing is forbidden on CRUD operations that target indices in \[index.mode=time_series\]/ + indices.create: + index: test_index + body: + settings: + index: + mode: time_series + routing_path: foo + mappings: + _routing: + required: true diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/20_unsupported_operations.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/20_unsupported_operations.yml index cc22eb22cb819..2629bb87b279f 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/20_unsupported_operations.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/20_unsupported_operations.yml @@ -61,16 +61,16 @@ setup: - '{"@timestamp": "2021-04-28T18:51:03.142Z", "metricset": "pod", "k8s": {"pod": {"name": "dog", "uid":"df3145b3-0563-4d3b-a0f7-897eb2876ea9", "ip": "10.10.55.3", "network": {"tx": 1434595272, "rx": 530605511}}}}' --- -index with specified id: +index with specified routing: - skip: version: " - 7.99.99" reason: introduced in 8.0.0 to be backported to 7.16.0 - do: - catch: /indexing with a specified id is not supported because the destination index \[test\] is in time series mode/ + catch: /indexing with a specified routing is not supported because the destination index \[test\] is in time series mode/ index: index: test - id: foo + routing: foo body: doc: "@timestamp": "2021-04-28T18:35:24.467Z" @@ -198,3 +198,32 @@ search with routing: search: index: test routing: rrrr + +--- +alias with routing: + - skip: + version: " - 7.99.99" + reason: introduced in 8.0.0 to be backported to 7.16.0 + + - do: + catch: /routing is forbidden on CRUD operations that target indices in \[index.mode=time_series\]/ + indices.put_alias: + index: test + name: alias + body: + routing: foo + +--- +alias with search_routing: + - skip: + version: " - 7.99.99" + reason: introduced in 8.0.0 to be backported to 7.16.0 + + - do: + catch: /routing is forbidden on CRUD operations that target indices in \[index.mode=time_series\]/ + indices.put_alias: + index: test + name: alias + body: + search_routing: foo + diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/AliasAction.java b/server/src/main/java/org/elasticsearch/cluster/metadata/AliasAction.java index 9f289ef65ce48..e1f7a79c24a70 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/AliasAction.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/AliasAction.java @@ -54,7 +54,13 @@ public String getIndex() { */ @FunctionalInterface public interface NewAliasValidator { - void validate(String alias, @Nullable String indexRouting, @Nullable String filter, @Nullable Boolean writeIndex); + void validate( + String alias, + @Nullable String indexRouting, + @Nullable String searchRouting, + @Nullable String filter, + @Nullable Boolean writeIndex + ); } /** @@ -117,7 +123,7 @@ boolean removeIndex() { @Override boolean apply(NewAliasValidator aliasValidator, Metadata.Builder metadata, IndexMetadata index) { - aliasValidator.validate(alias, indexRouting, filter, writeIndex); + aliasValidator.validate(alias, indexRouting, searchRouting, filter, writeIndex); AliasMetadata newAliasMd = AliasMetadata.newAliasMetadataBuilder(alias).filter(filter).indexRouting(indexRouting) .searchRouting(searchRouting).writeIndex(writeIndex).isHidden(isHidden).build(); @@ -233,7 +239,7 @@ boolean removeIndex() { @Override boolean apply(NewAliasValidator aliasValidator, Metadata.Builder metadata, IndexMetadata index) { - aliasValidator.validate(aliasName, null, filter, isWriteDataStream); + aliasValidator.validate(aliasName, null, null, filter, isWriteDataStream); return metadata.put(aliasName, dataStreamName, isWriteDataStream, filter); } } diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesService.java index 7c95387e6b4c2..c9f6bd555e67d 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesService.java @@ -23,6 +23,7 @@ import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.index.IndexService; +import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.indices.IndicesService; @@ -128,7 +129,7 @@ public ClusterState applyAliasActions(ClusterState currentState, Iterable { + NewAliasValidator newAliasValidator = (alias, indexRouting, searchRouting, filter, writeIndex) -> { aliasValidator.validateAlias(alias, action.getIndex(), indexRouting, lookup); if (Strings.hasLength(filter)) { for (Index index : dataStream.getIndices()) { @@ -136,6 +137,7 @@ public ClusterState applyAliasActions(ClusterState currentState, Iterable { + NewAliasValidator newAliasValidator = (alias, indexRouting, searchRouting, filter, writeIndex) -> { aliasValidator.validateAlias(alias, action.getIndex(), indexRouting, lookup); + IndexSettings.MODE.get(index.getSettings()).validateAlias(indexRouting, searchRouting); if (Strings.hasLength(filter)) { validateFilter(indicesToClose, indices, action, index, alias, filter); } diff --git a/server/src/main/java/org/elasticsearch/index/IndexMode.java b/server/src/main/java/org/elasticsearch/index/IndexMode.java index 10cfac6edddb5..a5680f52e76c0 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexMode.java +++ b/server/src/main/java/org/elasticsearch/index/IndexMode.java @@ -11,6 +11,9 @@ import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.core.Nullable; +import org.elasticsearch.index.mapper.MappingLookup; +import org.elasticsearch.index.mapper.RoutingFieldMapper; import java.util.List; import java.util.Map; @@ -35,6 +38,11 @@ void validateWithOtherSettings(Map, Object> settings) { ); } } + + public void validateMapping(MappingLookup lookup) {}; + + @Override + public void validateAlias(@Nullable String indexRouting, @Nullable String searchRouting) {} }, TIME_SERIES { @Override @@ -55,7 +63,28 @@ void validateWithOtherSettings(Map, Object> settings) { } private String error(Setting unsupported) { - return "[" + IndexSettings.MODE.getKey() + "=time_series] is incompatible with [" + unsupported.getKey() + "]"; + return tsdbMode() + " is incompatible with [" + unsupported.getKey() + "]"; + } + + public void validateMapping(MappingLookup lookup) { + if (((RoutingFieldMapper) lookup.getMapper(RoutingFieldMapper.NAME)).required()) { + throw new IllegalArgumentException(routingRequiredBad()); + } + } + + @Override + public void validateAlias(@Nullable String indexRouting, @Nullable String searchRouting) { + if (indexRouting != null || searchRouting != null) { + throw new IllegalArgumentException(routingRequiredBad()); + } + } + + private String routingRequiredBad() { + return "routing is forbidden on CRUD operations that target indices in " + tsdbMode(); + } + + private String tsdbMode() { + return "[" + IndexSettings.MODE.getKey() + "=time_series]"; } }; @@ -74,4 +103,14 @@ private String error(Setting unsupported) { ); abstract void validateWithOtherSettings(Map, Object> settings); + + /** + * Validate the mapping for this index. + */ + public abstract void validateMapping(MappingLookup lookup); + + /** + * Validate aliases targeting this index. + */ + public abstract void validateAlias(@Nullable String indexRouting, @Nullable String searchRouting); } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java index 0f52c02eeef27..7a919c21e5dfd 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java @@ -90,5 +90,6 @@ public void validate(IndexSettings settings, boolean checkLimits) { if (checkLimits) { this.mappingLookup.checkLimits(settings); } + settings.getMode().validateMapping(mappingLookup); } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/RoutingFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/RoutingFieldMapper.java index e7a676098d9e4..93fff23198b37 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/RoutingFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/RoutingFieldMapper.java @@ -88,6 +88,9 @@ public ValueFetcher valueFetcher(SearchExecutionContext context, String format) } } + /** + * Should we require {@code routing} on CRUD operations? + */ private final boolean required; private static final RoutingFieldMapper REQUIRED = new RoutingFieldMapper(true); @@ -102,6 +105,9 @@ private RoutingFieldMapper(boolean required) { this.required = required; } + /** + * Should we require {@code routing} on CRUD operations? + */ public boolean required() { return this.required; } diff --git a/server/src/test/java/org/elasticsearch/index/TimeSeriesModeTests.java b/server/src/test/java/org/elasticsearch/index/TimeSeriesModeTests.java index c0e73bc833b42..00d4bc38dfb25 100644 --- a/server/src/test/java/org/elasticsearch/index/TimeSeriesModeTests.java +++ b/server/src/test/java/org/elasticsearch/index/TimeSeriesModeTests.java @@ -10,11 +10,19 @@ import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.index.mapper.MapperServiceTestCase; import static org.hamcrest.Matchers.equalTo; -public class TimeSeriesModeTests extends ESTestCase { +public class TimeSeriesModeTests extends MapperServiceTestCase { + public void testConfigureIndex() { + Settings s = Settings.builder() + .put(IndexSettings.MODE.getKey(), "time_series") + .put(IndexMetadata.INDEX_ROUTING_PATH.getKey(), "foo") + .build(); + assertSame(IndexMode.TIME_SERIES, IndexSettings.MODE.get(s)); + } + public void testPartitioned() { Settings s = Settings.builder() .put(IndexMetadata.INDEX_ROUTING_PARTITION_SIZE_SETTING.getKey(), 2) @@ -50,4 +58,49 @@ public void testSortOrder() { Exception e = expectThrows(IllegalArgumentException.class, () -> IndexSettings.MODE.get(s)); assertThat(e.getMessage(), equalTo("[index.mode=time_series] is incompatible with [index.sort.order]")); } + + public void testWithoutRoutingPath() { + Settings s = Settings.builder().put(IndexSettings.MODE.getKey(), "time_series").build(); + Exception e = expectThrows(IllegalArgumentException.class, () -> IndexSettings.MODE.get(s)); + assertThat(e.getMessage(), equalTo("[index.mode=time_series] requires [index.routing_path]")); + } + + public void testRequiredRouting() { + Settings s = Settings.builder() + .put(IndexSettings.MODE.getKey(), "time_series") + .put(IndexMetadata.INDEX_ROUTING_PATH.getKey(), "foo") + .build(); + Exception e = expectThrows( + IllegalArgumentException.class, + () -> createMapperService(s, topMapping(b -> b.startObject("_routing").field("required", true).endObject())) + ); + assertThat(e.getMessage(), equalTo("routing is forbidden on CRUD operations that target indices in [index.mode=time_series]")); + } + + public void testValidateAlias() { + Settings s = Settings.builder() + .put(IndexSettings.MODE.getKey(), "time_series") + .put(IndexMetadata.INDEX_ROUTING_PATH.getKey(), "foo") + .build(); + IndexSettings.MODE.get(s).validateAlias(null, null); // Doesn't throw exception + } + + public void testValidateAliasWithIndexRouting() { + Settings s = Settings.builder() + .put(IndexSettings.MODE.getKey(), "time_series") + .put(IndexMetadata.INDEX_ROUTING_PATH.getKey(), "foo") + .build(); + Exception e = expectThrows(IllegalArgumentException.class, () -> IndexSettings.MODE.get(s).validateAlias("r", null)); + assertThat(e.getMessage(), equalTo("routing is forbidden on CRUD operations that target indices in [index.mode=time_series]")); + } + + public void testValidateAliasWithSearchRouting() { + Settings s = Settings.builder() + .put(IndexSettings.MODE.getKey(), "time_series") + .put(IndexMetadata.INDEX_ROUTING_PATH.getKey(), "foo") + .build(); + Exception e = expectThrows(IllegalArgumentException.class, () -> IndexSettings.MODE.get(s).validateAlias(null, "r")); + assertThat(e.getMessage(), equalTo("routing is forbidden on CRUD operations that target indices in [index.mode=time_series]")); + } + } From 317cbe62a1cc7b347acfa96e19a9f20c0210e835 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 23 Sep 2021 11:53:56 -0400 Subject: [PATCH 07/31] Small --- .../org/elasticsearch/upgrades/FullClusterRestartIT.java | 6 ++++-- .../org/elasticsearch/action/bulk/TransportBulkAction.java | 3 +-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/qa/full-cluster-restart/src/test/java/org/elasticsearch/upgrades/FullClusterRestartIT.java b/qa/full-cluster-restart/src/test/java/org/elasticsearch/upgrades/FullClusterRestartIT.java index 0ed4ed95b9371..94f68e479ec6c 100644 --- a/qa/full-cluster-restart/src/test/java/org/elasticsearch/upgrades/FullClusterRestartIT.java +++ b/qa/full-cluster-restart/src/test/java/org/elasticsearch/upgrades/FullClusterRestartIT.java @@ -196,6 +196,7 @@ public void testNewReplicas() throws Exception { logger.info("Refreshing [{}]", index); client().performRequest(new Request("POST", "/" + index + "/_refresh")); } else { + // The test runs with two nodes so this should still go green. final int numReplicas = 1; final long startTime = System.currentTimeMillis(); logger.debug("--> creating [{}] replicas for index [{}]", numReplicas, index); @@ -223,7 +224,7 @@ public void testNewReplicas() throws Exception { } public void testSearchTimeSeriesMode() throws Exception { - assumeTrue("time series mode introduced in 8.0.0 to be backported to 7.15.0", getOldClusterVersion().onOrAfter(Version.V_8_0_0)); + assumeTrue("time series mode introduced in 8.0.0 to be backported to 7.16.0", getOldClusterVersion().onOrAfter(Version.V_8_0_0)); int numDocs; if (isRunningAgainstOldCluster()) { numDocs = createTimeSeriesModeIndex(1); @@ -265,10 +266,11 @@ public void testSearchTimeSeriesMode() throws Exception { } public void testNewReplicasTimeSeriesMode() throws Exception { - assumeTrue("time series mode introduced in 8.0.0 to be backported to 7.15.0", getOldClusterVersion().onOrAfter(Version.V_8_0_0)); + assumeTrue("time series mode introduced in 8.0.0 to be backported to 7.16.0", getOldClusterVersion().onOrAfter(Version.V_8_0_0)); if (isRunningAgainstOldCluster()) { createTimeSeriesModeIndex(0); } else { + // The test runs with two nodes so this should still go green. final int numReplicas = 1; final long startTime = System.currentTimeMillis(); logger.debug("--> creating [{}] replicas for index [{}]", numReplicas, index); 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 43f46a3d50dde..04bc45beaf8f3 100644 --- a/server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java +++ b/server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java @@ -43,7 +43,6 @@ import org.elasticsearch.cluster.metadata.MappingMetadata; import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.cluster.routing.IndexRouting; -import org.elasticsearch.cluster.routing.RoutingTable; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.unit.ByteSizeUnit; @@ -491,7 +490,7 @@ protected void doRun() { default: throw new AssertionError("request type not supported: [" + docWriteRequest.opType() + "]"); } List shardRequests = requestsByShard.computeIfAbsent( - RoutingTable.shardRoutingTable(clusterState.routingTable().index(concreteIndex), shardId).getShardId(), + new ShardId(concreteIndex, shardId), shard -> new ArrayList<>() ); shardRequests.add(new BulkItemRequest(i, docWriteRequest)); From 80548de5d4e922bb0d2dcc2993270aa6289827d7 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 23 Sep 2021 13:31:25 -0400 Subject: [PATCH 08/31] Fork fork knife --- .../20_tsdb_consistency.yml | 6 +- .../rest-api-spec/test/tsdb/10_settings.yml | 4 +- .../rest-api-spec/test/tsdb/20_mappings.yml | 1 + ...ions.yml => 30_unsupported_operations.yml} | 27 ++-- .../action/bulk/TransportBulkAction.java | 151 +++++++++++++----- .../cluster/routing/IndexRouting.java | 18 +++ ...ActionIndicesThatCannotBeCreatedTests.java | 12 +- .../bulk/TransportBulkActionIngestTests.java | 12 +- .../action/bulk/TransportBulkActionTests.java | 12 ++ .../bulk/TransportBulkActionTookTests.java | 52 ++++-- 10 files changed, 222 insertions(+), 73 deletions(-) rename rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/{20_unsupported_operations.yml => 30_unsupported_operations.yml} (90%) diff --git a/qa/smoke-test-multinode/src/test/resources/rest-api-spec/test/smoke_test_multinode/20_tsdb_consistency.yml b/qa/smoke-test-multinode/src/test/resources/rest-api-spec/test/smoke_test_multinode/20_tsdb_consistency.yml index 363f3fd463292..4dbfc8b9a3f31 100644 --- a/qa/smoke-test-multinode/src/test/resources/rest-api-spec/test/smoke_test_multinode/20_tsdb_consistency.yml +++ b/qa/smoke-test-multinode/src/test/resources/rest-api-spec/test/smoke_test_multinode/20_tsdb_consistency.yml @@ -4,7 +4,7 @@ setup: - skip: version: " - 7.99.99" - reason: introduced in 8.0.0 to be backported to 7.16.0 + reason: introduced in 8.0.0 - do: indices.create: @@ -22,14 +22,14 @@ setup: type: date metricset: type: keyword - dimension: true + time_series_dimension: true k8s: properties: pod: properties: uid: type: keyword - dimension: true + time_series_dimension: true name: type: keyword ip: diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/10_settings.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/10_settings.yml index 0f02f332f1019..34fd190d5ed4c 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/10_settings.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/10_settings.yml @@ -1,7 +1,7 @@ enable: - skip: version: " - 7.99.99" - reason: introduced in 8.0.0 to be backported to 7.16.0 + reason: introduced in 8.0.0 - do: indices.create: @@ -42,7 +42,7 @@ enable: no sort field: - skip: version: " - 7.99.99" - reason: introduced in 8.0.0 to be backported to 7.16.0 + reason: introduced in 8.0.0 - do: catch: /\[index.mode=time_series\] is incompatible with \[index.sort.field\]/ diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/20_mappings.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/20_mappings.yml index e747f068f6f67..d556490a97b74 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/20_mappings.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/20_mappings.yml @@ -10,6 +10,7 @@ add time series mappings: settings: index: mode: time_series + routing_path: metricset number_of_replicas: 0 number_of_shards: 2 mappings: diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/20_unsupported_operations.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/30_unsupported_operations.yml similarity index 90% rename from rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/20_unsupported_operations.yml rename to rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/30_unsupported_operations.yml index 2629bb87b279f..c2a7db55ab46e 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/20_unsupported_operations.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/30_unsupported_operations.yml @@ -1,7 +1,7 @@ setup: - skip: version: " - 7.99.99" - reason: introduced in 8.0.0 to be backported to 7.16.0 + reason: introduced in 8.0.0 - do: indices.create: @@ -19,14 +19,14 @@ setup: type: date metricset: type: keyword - dimension: true + time_series_dimension: true k8s: properties: pod: properties: uid: type: keyword - dimension: true + time_series_dimension: true name: type: keyword ip: @@ -64,7 +64,7 @@ setup: index with specified routing: - skip: version: " - 7.99.99" - reason: introduced in 8.0.0 to be backported to 7.16.0 + reason: introduced in 8.0.0 - do: catch: /indexing with a specified routing is not supported because the destination index \[test\] is in time series mode/ @@ -88,8 +88,7 @@ index with specified routing: index with specified routing over _bulk: - skip: version: " - 7.99.99" - reason: introduced in 8.0.0 to be backported to 7.16.0 - + reason: introduced in 8.0.0 - do: bulk: refresh: true @@ -103,7 +102,7 @@ index with specified routing over _bulk: delete: - skip: version: " - 7.99.99" - reason: introduced in 8.0.0 to be backported to 7.16.0 + reason: introduced in 8.0.0 - do: catch: /delete is not supported because the destination index \[test\] is in time series mode/ @@ -115,7 +114,7 @@ delete: delete over _bulk: - skip: version: " - 7.99.99" - reason: introduced in 8.0.0 to be backported to 7.16.0 + reason: introduced in 8.0.0 - do: bulk: @@ -129,7 +128,7 @@ delete over _bulk: noop update: - skip: version: " - 7.99.99" - reason: introduced in 8.0.0 to be backported to 7.16.0 + reason: introduced in 8.0.0 - do: search: @@ -151,7 +150,7 @@ noop update: update: - skip: version: " - 7.99.99" - reason: introduced in 8.0.0 to be backported to 7.16.0 + reason: introduced in 8.0.0 # We fail even though the document isn't found. - do: @@ -176,7 +175,7 @@ update: update over _bulk: - skip: version: " - 7.99.99" - reason: introduced in 8.0.0 to be backported to 7.16.0 + reason: introduced in 8.0.0 - do: bulk: @@ -190,7 +189,7 @@ update over _bulk: search with routing: - skip: version: " - 7.99.99" - reason: introduced in 8.0.0 to be backported to 7.16.0 + reason: introduced in 8.0.0 # We fail even though the document isn't found. - do: @@ -203,7 +202,7 @@ search with routing: alias with routing: - skip: version: " - 7.99.99" - reason: introduced in 8.0.0 to be backported to 7.16.0 + reason: introduced in 8.0.0 - do: catch: /routing is forbidden on CRUD operations that target indices in \[index.mode=time_series\]/ @@ -217,7 +216,7 @@ alias with routing: alias with search_routing: - skip: version: " - 7.99.99" - reason: introduced in 8.0.0 to be backported to 7.16.0 + reason: introduced in 8.0.0 - do: catch: /routing is forbidden on CRUD operations that target indices in \[index.mode=time_series\]/ 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 04bc45beaf8f3..a2fb848b8875a 100644 --- a/server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java +++ b/server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java @@ -46,6 +46,7 @@ import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.unit.ByteSizeUnit; +import org.elasticsearch.common.util.concurrent.AbstractRunnable; import org.elasticsearch.common.util.concurrent.AtomicArray; import org.elasticsearch.core.Releasable; import org.elasticsearch.core.TimeValue; @@ -157,22 +158,31 @@ protected void doExecute(Task task, BulkRequest bulkRequest, ActionListener releasingListener = ActionListener.runBefore(listener, releasable::close); final String executorName = isOnlySystem ? Names.SYSTEM_WRITE : Names.WRITE; - ActionRunnable internalExecute = new ActionRunnable<>(releasingListener) { - @Override - protected void doRun() { - doInternalExecute(task, bulkRequest, executorName, releasingListener); - } - }; // We dispatch large bulk requests as coordinating these bytes can occur on the transport thread and might involve // compression. if (indexingBytes >= ONE_MEGABYTE) { - threadPool.executor(executorName).execute(internalExecute); + threadPool.executor(executorName).execute(new ActionRunnable<>(releasingListener) { + @Override + protected void doRun() { + doInternalExecute(task, bulkRequest, false, executorName, releasingListener); + } + }); } else { - internalExecute.run(); + try { + doInternalExecute(task, bulkRequest, true, executorName, releasingListener); + } catch(Exception e) { + releasingListener.onFailure(e); + } } } - protected void doInternalExecute(Task task, BulkRequest bulkRequest, String executorName, ActionListener listener) { + protected void doInternalExecute( + Task task, + BulkRequest bulkRequest, + boolean onTransportThread, + String executorName, + ActionListener listener + ) { final long startTime = relativeTime(); final AtomicArray responses = new AtomicArray<>(bulkRequest.requests.size()); @@ -245,7 +255,7 @@ protected void doInternalExecute(Task task, BulkRequest bulkRequest, String exec // Step 3: create all the indices that are missing, if there are any missing. start the bulk after all the creates come back. if (autoCreateIndices.isEmpty()) { - executeBulk(task, bulkRequest, startTime, listener, responses, indicesThatCannotBeCreated); + executeBulk(task, bulkRequest, startTime, listener, executorName, onTransportThread, responses, indicesThatCannotBeCreated); } else { final AtomicInteger counter = new AtomicInteger(autoCreateIndices.size()); for (String index : autoCreateIndices) { @@ -254,10 +264,18 @@ protected void doInternalExecute(Task task, BulkRequest bulkRequest, String exec public void onResponse(CreateIndexResponse result) { if (counter.decrementAndGet() == 0) { threadPool.executor(executorName).execute(new ActionRunnable<>(listener) { - @Override protected void doRun() { - executeBulk(task, bulkRequest, startTime, listener, responses, indicesThatCannotBeCreated); + executeBulk( + task, + bulkRequest, + startTime, + listener, + executorName, + false, + responses, + indicesThatCannotBeCreated + ); } }); } @@ -286,7 +304,16 @@ else if ((cause instanceof ResourceAlreadyExistsException) == false) { threadPool.executor(executorName).execute(new ActionRunnable<>(wrappedListener) { @Override protected void doRun() { - executeBulk(task, bulkRequest, startTime, wrappedListener, responses, indicesThatCannotBeCreated); + executeBulk( + task, + bulkRequest, + startTime, + wrappedListener, + executorName, + false, + responses, + indicesThatCannotBeCreated + ); } @Override @@ -394,37 +421,60 @@ private long buildTookInMillis(long startTimeNanos) { * retries on retryable cluster blocks, resolves item requests, * constructs shard bulk requests and delegates execution to shard bulk action * */ - private final class BulkOperation extends ActionRunnable { + private final class BulkOperation { private final Task task; private BulkRequest bulkRequest; // set to null once all requests are sent out + private final ActionListener listener; private final AtomicArray responses; private final long startTimeNanos; private final ClusterStateObserver observer; private final Map indicesThatCannotBeCreated; - - BulkOperation(Task task, BulkRequest bulkRequest, ActionListener listener, AtomicArray responses, - long startTimeNanos, Map indicesThatCannotBeCreated) { - super(listener); + private final String executorName; + + BulkOperation( + Task task, + BulkRequest bulkRequest, + ActionListener listener, + String executorName, + AtomicArray responses, + long startTimeNanos, + Map indicesThatCannotBeCreated + ) { this.task = task; this.bulkRequest = bulkRequest; + this.listener = listener; this.responses = responses; this.startTimeNanos = startTimeNanos; this.indicesThatCannotBeCreated = indicesThatCannotBeCreated; + this.executorName = executorName; this.observer = new ClusterStateObserver(clusterService, bulkRequest.timeout(), logger, threadPool.getThreadContext()); } - @Override - protected void doRun() { + private void performBulkRequests(boolean onTransportThread) { assert bulkRequest != null; final ClusterState clusterState = observer.setAndGetObservedState(); if (handleBlockExceptions(clusterState)) { return; } - final ConcreteIndices concreteIndices = new ConcreteIndices(clusterState, indexNameExpressionResolver); + ConcreteIndices concreteIndices = new ConcreteIndices(clusterState, indexNameExpressionResolver); + + if (onTransportThread && routingMustForkFromTransportThread(bulkRequest.requests(), concreteIndices)) { + threadPool.executor(executorName).execute(new ActionRunnable<>(listener) { + @Override + protected void doRun() throws Exception { + performBulkRequests(clusterState, concreteIndices); + } + }); + } else { + performBulkRequests(clusterState, concreteIndices); + } + } + + private void performBulkRequests(ClusterState clusterState, ConcreteIndices concreteIndices) { Metadata metadata = clusterState.metadata(); // Group the requests by ShardId -> Operations mapping Map> requestsByShard = new HashMap<>(); - Map indexRoutings = new HashMap<>(); + for (int i = 0; i < bulkRequest.requests.size(); i++) { DocWriteRequest docWriteRequest = bulkRequest.requests.get(i); //the request can only be null because we set it to null in the previous step, so it gets ignored @@ -451,10 +501,8 @@ protected void doRun() { throw new IllegalArgumentException("only write ops with an op_type of create are allowed in data streams"); } - IndexRouting indexRouting = indexRoutings.computeIfAbsent( - concreteIndex, - idx -> IndexRouting.fromIndexMetadata(clusterState.metadata().getIndexSafe(idx)) - ); + IndexRouting indexRouting = concreteIndices.routing(concreteIndex); + int shardId; switch (docWriteRequest.opType()) { case CREATE: @@ -567,7 +615,7 @@ private boolean handleBlockExceptions(ClusterState state) { logger.trace("cluster is blocked, scheduling a retry", blockException); retry(blockException); } else { - onFailure(blockException); + listener.onFailure(blockException); } return true; } @@ -578,24 +626,33 @@ void retry(Exception failure) { assert failure != null; if (observer.isTimedOut()) { // we running as a last attempt after a timeout has happened. don't retry - onFailure(failure); + listener.onFailure(failure); return; } observer.waitForNextChange(new ClusterStateObserver.Listener() { @Override public void onNewClusterState(ClusterState state) { - run(); + dispatchRetry(); } @Override public void onClusterServiceClose() { - onFailure(new NodeClosedException(clusterService.localNode())); + listener.onFailure(new NodeClosedException(clusterService.localNode())); } @Override public void onTimeout(TimeValue timeout) { // Try one more time... - run(); + dispatchRetry(); + } + + private void dispatchRetry() { + threadPool.executor(executorName).submit(new ActionRunnable<>(listener) { + @Override + protected void doRun() throws Exception { + performBulkRequests(false); + } + }); } }); } @@ -648,15 +705,25 @@ private void addFailure(DocWriteRequest request, int idx, Exception unavailab } } - void executeBulk(Task task, final BulkRequest bulkRequest, final long startTimeNanos, final ActionListener listener, - final AtomicArray responses, Map indicesThatCannotBeCreated) { - new BulkOperation(task, bulkRequest, listener, responses, startTimeNanos, indicesThatCannotBeCreated).run(); + void executeBulk( + Task task, + BulkRequest bulkRequest, + long startTimeNanos, + ActionListener listener, + String executorName, + boolean onTransportThread, + AtomicArray responses, + Map indicesThatCannotBeCreated + ) { + new BulkOperation(task, bulkRequest, listener, executorName, responses, startTimeNanos, indicesThatCannotBeCreated) + .performBulkRequests(onTransportThread); } private static class ConcreteIndices { private final ClusterState state; private final IndexNameExpressionResolver indexNameExpressionResolver; private final Map indices = new HashMap<>(); + private final Map routings = new HashMap<>(); ConcreteIndices(ClusterState state, IndexNameExpressionResolver indexNameExpressionResolver) { this.state = state; @@ -685,6 +752,10 @@ Index resolveIfAbsent(DocWriteRequest request) { } return concreteIndex; } + + IndexRouting routing(Index index) { + return routings.computeIfAbsent(index, idx -> IndexRouting.fromIndexMetadata(state.metadata().getIndexSafe(idx))); + } } private long relativeTime() { @@ -718,12 +789,12 @@ private void processBulkIndexIngestRequest(Task task, BulkRequest original, Stri // before we continue the bulk request we should fork back on a write thread: if (originalThread == Thread.currentThread()) { assert Thread.currentThread().getName().contains(executorName); - doInternalExecute(task, bulkRequest, executorName, actionListener); + doInternalExecute(task, bulkRequest, false, executorName, actionListener); } else { threadPool.executor(executorName).execute(new ActionRunnable<>(actionListener) { @Override protected void doRun() { - doInternalExecute(task, bulkRequest, executorName, actionListener); + doInternalExecute(task, bulkRequest, false, executorName, actionListener); } @Override @@ -833,6 +904,14 @@ synchronized void markItemAsFailed(int slot, Exception e) { BulkItemResponse.Failure failure = new BulkItemResponse.Failure(indexRequest.index(), indexRequest.id(), e); itemResponses.add(BulkItemResponse.failure(slot, indexRequest.opType(), failure)); } + } + boolean routingMustForkFromTransportThread(Iterable> requests, ConcreteIndices concreteIndices) { + for (DocWriteRequest request : requests) { + if (concreteIndices.routing(concreteIndices.resolveIfAbsent(request)).mustForkFromTransportThread()) { + return true; + } + } + return false; } } 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 84bdd0cf579e3..28691137e420d 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java @@ -18,6 +18,7 @@ import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.common.xcontent.support.filtering.FilterPath; import org.elasticsearch.core.Nullable; +import org.elasticsearch.transport.Transports; import java.io.IOException; import java.util.ArrayList; @@ -66,6 +67,11 @@ private IndexRouting(int routingNumShards, int routingFactor) { this.routingFactor = routingFactor; } + /** + * Should the routing generation calls be forked from the transport thread? + */ + public abstract boolean mustForkFromTransportThread(); + /** * Called when indexing a document to generate the shard id that should contain * a document with the provided parameters. @@ -126,6 +132,11 @@ private abstract static class IdAndRoutingOnly extends IndexRouting { protected abstract int shardId(String id, @Nullable String routing); + @Override + public boolean mustForkFromTransportThread() { + return false; + } + @Override public int indexShard(String id, @Nullable String routing, XContentType sourceType, BytesReference source) { return shardId(id, routing); @@ -205,11 +216,18 @@ private static class ExtractFromSource extends IndexRouting { this.include = FilterPath.compile(Set.copyOf(routingPaths)); } + @Override + public boolean mustForkFromTransportThread() { + return true; + } + @Override public int indexShard(String id, @Nullable String routing, XContentType sourceType, BytesReference source) { if (routing != null) { throw new IllegalArgumentException(error("indexing with a specified routing")); } + assert Transports.assertNotTransportThread("parsing the _source can get slow"); + try { try ( XContentParser parser = sourceType.xContent() diff --git a/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionIndicesThatCannotBeCreatedTests.java b/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionIndicesThatCannotBeCreatedTests.java index 9067e2a1e02ba..d426920b1d0dc 100644 --- a/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionIndicesThatCannotBeCreatedTests.java +++ b/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionIndicesThatCannotBeCreatedTests.java @@ -118,8 +118,16 @@ public boolean hasIndexAbstraction(String indexAbstraction, ClusterState state) null, null, mock(ActionFilters.class), indexNameExpressionResolver, new IndexingPressure(Settings.EMPTY), EmptySystemIndices.INSTANCE) { @Override - void executeBulk(Task task, BulkRequest bulkRequest, long startTimeNanos, ActionListener listener, - AtomicArray responses, Map indicesThatCannotBeCreated) { + void executeBulk( + Task task, + BulkRequest bulkRequest, + long startTimeNanos, + ActionListener listener, + String executorName, + boolean onTransportThread, + AtomicArray responses, + Map indicesThatCannotBeCreated + ) { assertEquals(expected, indicesThatCannotBeCreated.keySet()); } diff --git a/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionIngestTests.java b/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionIngestTests.java index 14d8031cdc3d6..3715cdb35dcb1 100644 --- a/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionIngestTests.java +++ b/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionIngestTests.java @@ -137,8 +137,16 @@ null, new ActionFilters(Collections.emptySet()), } @Override - void executeBulk(Task task, final BulkRequest bulkRequest, final long startTimeNanos, final ActionListener listener, - final AtomicArray responses, Map indicesThatCannotBeCreated) { + void executeBulk( + Task task, + BulkRequest bulkRequest, + long startTimeNanos, + ActionListener listener, + String executorName, + boolean onTransportThread, + AtomicArray responses, + Map indicesThatCannotBeCreated + ) { assertTrue(indexCreated); isExecuted = true; } diff --git a/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTests.java b/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTests.java index 8e8b318475188..16a0cd2bd9794 100644 --- a/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTests.java @@ -259,6 +259,18 @@ public void testOnlySystem() { assertFalse(bulkAction.isOnlySystem(buildBulkRequest(mixed), indicesLookup, systemIndices)); } +// public void testAllRoutingOkOnTransportThread() { +// SortedMap indicesLookup = new TreeMap<>(); +// Settings settings = Settings.builder().put("index.version.created", Version.CURRENT).build(); +// indicesLookup.put(".foo", +// new Index(IndexMetadata.builder(".foo").settings(settings).system(true).numberOfShards(1).numberOfReplicas(0).build())); +// indicesLookup.put(".bar", +// new Index(IndexMetadata.builder(".bar").settings(settings).system(true).numberOfShards(1).numberOfReplicas(0).build())); +// List onlySystem = List.of(".foo", ".bar"); +// assertTrue(bulkAction.allRoutingOkOnTransportThread(buildBulkRequest(onlySystem), indicesLookup)); +// +// } + public void testRejectionAfterCreateIndexIsPropagated() throws Exception { BulkRequest bulkRequest = new BulkRequest().add(new IndexRequest("index").id("id").source(Collections.emptyMap())); bulkAction.failIndexCreation = randomBoolean(); diff --git a/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTookTests.java b/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTookTests.java index e6a8c5647c085..de868ebc2d7b7 100644 --- a/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTookTests.java +++ b/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTookTests.java @@ -119,14 +119,26 @@ void doExecute(ActionType action, Request request, ActionListener listener, - AtomicArray responses, - Map indicesThatCannotBeCreated) { + Task task, + BulkRequest bulkRequest, + long startTimeNanos, + ActionListener listener, + String executorName, + boolean onTransportThread, + AtomicArray responses, + Map indicesThatCannotBeCreated + ) { expected.set(1000000); - super.executeBulk(task, bulkRequest, startTimeNanos, listener, responses, indicesThatCannotBeCreated); + super.executeBulk( + task, + bulkRequest, + startTimeNanos, + listener, + executorName, + onTransportThread, + responses, + indicesThatCannotBeCreated + ); } }; } else { @@ -141,15 +153,27 @@ void executeBulk( @Override void executeBulk( - Task task, - BulkRequest bulkRequest, - long startTimeNanos, - ActionListener listener, - AtomicArray responses, - Map indicesThatCannotBeCreated) { + Task task, + BulkRequest bulkRequest, + long startTimeNanos, + ActionListener listener, + String executorName, + boolean onTransportThread, + AtomicArray responses, + Map indicesThatCannotBeCreated + ) { long elapsed = spinForAtLeastOneMillisecond(); expected.set(elapsed); - super.executeBulk(task, bulkRequest, startTimeNanos, listener, responses, indicesThatCannotBeCreated); + super.executeBulk( + task, + bulkRequest, + startTimeNanos, + listener, + executorName, + onTransportThread, + responses, + indicesThatCannotBeCreated + ); } }; } From 517b2cf86036fff205d63daac928a445bc16ccaf Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 23 Sep 2021 18:06:08 -0400 Subject: [PATCH 09/31] Let failures flow --- .../action/bulk/TransportBulkAction.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) 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 a2fb848b8875a..806e6684d9366 100644 --- a/server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java +++ b/server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java @@ -908,7 +908,16 @@ synchronized void markItemAsFailed(int slot, Exception e) { boolean routingMustForkFromTransportThread(Iterable> requests, ConcreteIndices concreteIndices) { for (DocWriteRequest request : requests) { - if (concreteIndices.routing(concreteIndices.resolveIfAbsent(request)).mustForkFromTransportThread()) { + if (request == null) { + continue; + } + Index index; + try { + index = concreteIndices.resolveIfAbsent(request); + } catch (IndexClosedException | IndexNotFoundException | IllegalArgumentException ex) { + continue; + } + if (concreteIndices.routing(index).mustForkFromTransportThread()) { return true; } } From 09cb170e05ab3d9071a8be9bcd00cfde27af1796 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 27 Sep 2021 10:42:53 -0400 Subject: [PATCH 10/31] Fix full cluster --- .../org/elasticsearch/upgrades/FullClusterRestartIT.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/qa/full-cluster-restart/src/test/java/org/elasticsearch/upgrades/FullClusterRestartIT.java b/qa/full-cluster-restart/src/test/java/org/elasticsearch/upgrades/FullClusterRestartIT.java index 94f68e479ec6c..f1a592704ca91 100644 --- a/qa/full-cluster-restart/src/test/java/org/elasticsearch/upgrades/FullClusterRestartIT.java +++ b/qa/full-cluster-restart/src/test/java/org/elasticsearch/upgrades/FullClusterRestartIT.java @@ -224,7 +224,7 @@ public void testNewReplicas() throws Exception { } public void testSearchTimeSeriesMode() throws Exception { - assumeTrue("time series mode introduced in 8.0.0 to be backported to 7.16.0", getOldClusterVersion().onOrAfter(Version.V_8_0_0)); + assumeTrue("time series mode introduced in 8.0.0", getOldClusterVersion().onOrAfter(Version.V_8_0_0)); int numDocs; if (isRunningAgainstOldCluster()) { numDocs = createTimeSeriesModeIndex(1); @@ -266,7 +266,7 @@ public void testSearchTimeSeriesMode() throws Exception { } public void testNewReplicasTimeSeriesMode() throws Exception { - assumeTrue("time series mode introduced in 8.0.0 to be backported to 7.16.0", getOldClusterVersion().onOrAfter(Version.V_8_0_0)); + assumeTrue("time series mode introduced in 8.0.0", getOldClusterVersion().onOrAfter(Version.V_8_0_0)); if (isRunningAgainstOldCluster()) { createTimeSeriesModeIndex(0); } else { @@ -313,7 +313,7 @@ private int createTimeSeriesModeIndex(int replicas) throws IOException { mappingsAndSettings.startObject("properties"); { mappingsAndSettings.startObject("@timestamp").field("type", "date").endObject(); - mappingsAndSettings.startObject("dim").field("type", "keyword").field("dimension", true).endObject(); + mappingsAndSettings.startObject("dim").field("type", "keyword").field("time_series_dimension", true).endObject(); } mappingsAndSettings.endObject(); mappingsAndSettings.endObject(); From a3c592a3d8d3404d88237f839df96ef7d3de0e49 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Fri, 1 Oct 2021 13:07:35 -0400 Subject: [PATCH 11/31] Always fork --- .../action/bulk/TransportBulkAction.java | 76 +++++++------------ ...ActionIndicesThatCannotBeCreatedTests.java | 1 - .../bulk/TransportBulkActionIngestTests.java | 1 - .../bulk/TransportBulkActionTookTests.java | 4 - 4 files changed, 28 insertions(+), 54 deletions(-) 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 806e6684d9366..ee1755fb368da 100644 --- a/server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java +++ b/server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java @@ -45,8 +45,6 @@ import org.elasticsearch.cluster.routing.IndexRouting; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.common.unit.ByteSizeUnit; -import org.elasticsearch.common.util.concurrent.AbstractRunnable; import org.elasticsearch.common.util.concurrent.AtomicArray; import org.elasticsearch.core.Releasable; import org.elasticsearch.core.TimeValue; @@ -91,7 +89,6 @@ public class TransportBulkAction extends HandledTransportAction { private static final Logger logger = LogManager.getLogger(TransportBulkAction.class); - private static final long ONE_MEGABYTE = ByteSizeUnit.MB.toBytes(1); private final ThreadPool threadPool; private final ClusterService clusterService; @@ -152,37 +149,27 @@ public static IndexRequest getIndexWriteRequest(DocWriteRequest docWriteReque @Override protected void doExecute(Task task, BulkRequest bulkRequest, ActionListener listener) { + /* + * This is called on the Transport tread so we can check the indexing + * memory pressure *quickly* but we don't want to keep the transport + * thread busy. So as son as we have the indexing pressure in we fork + * to one of the write thread pools. + */ final int indexingOps = bulkRequest.numberOfActions(); final long indexingBytes = bulkRequest.ramBytesUsed(); final boolean isOnlySystem = isOnlySystem(bulkRequest, clusterService.state().metadata().getIndicesLookup(), systemIndices); final Releasable releasable = indexingPressure.markCoordinatingOperationStarted(indexingOps, indexingBytes, isOnlySystem); final ActionListener releasingListener = ActionListener.runBefore(listener, releasable::close); final String executorName = isOnlySystem ? Names.SYSTEM_WRITE : Names.WRITE; - // We dispatch large bulk requests as coordinating these bytes can occur on the transport thread and might involve - // compression. - if (indexingBytes >= ONE_MEGABYTE) { - threadPool.executor(executorName).execute(new ActionRunnable<>(releasingListener) { - @Override - protected void doRun() { - doInternalExecute(task, bulkRequest, false, executorName, releasingListener); - } - }); - } else { - try { - doInternalExecute(task, bulkRequest, true, executorName, releasingListener); - } catch(Exception e) { - releasingListener.onFailure(e); + threadPool.executor(executorName).execute(new ActionRunnable<>(releasingListener) { + @Override + protected void doRun() { + doInternalExecute(task, bulkRequest, executorName, releasingListener); } - } + }); } - protected void doInternalExecute( - Task task, - BulkRequest bulkRequest, - boolean onTransportThread, - String executorName, - ActionListener listener - ) { + protected void doInternalExecute(Task task, BulkRequest bulkRequest, String executorName, ActionListener listener) { final long startTime = relativeTime(); final AtomicArray responses = new AtomicArray<>(bulkRequest.requests.size()); @@ -255,7 +242,7 @@ protected void doInternalExecute( // Step 3: create all the indices that are missing, if there are any missing. start the bulk after all the creates come back. if (autoCreateIndices.isEmpty()) { - executeBulk(task, bulkRequest, startTime, listener, executorName, onTransportThread, responses, indicesThatCannotBeCreated); + executeBulk(task, bulkRequest, startTime, listener, executorName, responses, indicesThatCannotBeCreated); } else { final AtomicInteger counter = new AtomicInteger(autoCreateIndices.size()); for (String index : autoCreateIndices) { @@ -272,7 +259,6 @@ protected void doRun() { startTime, listener, executorName, - false, responses, indicesThatCannotBeCreated ); @@ -310,7 +296,6 @@ protected void doRun() { startTime, wrappedListener, executorName, - false, responses, indicesThatCannotBeCreated ); @@ -450,27 +435,13 @@ private final class BulkOperation { this.observer = new ClusterStateObserver(clusterService, bulkRequest.timeout(), logger, threadPool.getThreadContext()); } - private void performBulkRequests(boolean onTransportThread) { + private void performBulkRequests() { assert bulkRequest != null; final ClusterState clusterState = observer.setAndGetObservedState(); if (handleBlockExceptions(clusterState)) { return; } ConcreteIndices concreteIndices = new ConcreteIndices(clusterState, indexNameExpressionResolver); - - if (onTransportThread && routingMustForkFromTransportThread(bulkRequest.requests(), concreteIndices)) { - threadPool.executor(executorName).execute(new ActionRunnable<>(listener) { - @Override - protected void doRun() throws Exception { - performBulkRequests(clusterState, concreteIndices); - } - }); - } else { - performBulkRequests(clusterState, concreteIndices); - } - } - - private void performBulkRequests(ClusterState clusterState, ConcreteIndices concreteIndices) { Metadata metadata = clusterState.metadata(); // Group the requests by ShardId -> Operations mapping Map> requestsByShard = new HashMap<>(); @@ -647,10 +618,20 @@ public void onTimeout(TimeValue timeout) { } private void dispatchRetry() { + /* + * This is called on the cluster state update and timer + * thread pools to dispatch the bulk request onto its + * appropriate thread pool. + */ threadPool.executor(executorName).submit(new ActionRunnable<>(listener) { @Override protected void doRun() throws Exception { - performBulkRequests(false); + threadPool.executor(executorName).execute(new ActionRunnable<>(listener) { + @Override + protected void doRun() throws Exception { + performBulkRequests(); + } + }); } }); } @@ -711,12 +692,11 @@ void executeBulk( long startTimeNanos, ActionListener listener, String executorName, - boolean onTransportThread, AtomicArray responses, Map indicesThatCannotBeCreated ) { new BulkOperation(task, bulkRequest, listener, executorName, responses, startTimeNanos, indicesThatCannotBeCreated) - .performBulkRequests(onTransportThread); + .performBulkRequests(); } private static class ConcreteIndices { @@ -789,12 +769,12 @@ private void processBulkIndexIngestRequest(Task task, BulkRequest original, Stri // before we continue the bulk request we should fork back on a write thread: if (originalThread == Thread.currentThread()) { assert Thread.currentThread().getName().contains(executorName); - doInternalExecute(task, bulkRequest, false, executorName, actionListener); + doInternalExecute(task, bulkRequest, executorName, actionListener); } else { threadPool.executor(executorName).execute(new ActionRunnable<>(actionListener) { @Override protected void doRun() { - doInternalExecute(task, bulkRequest, false, executorName, actionListener); + doInternalExecute(task, bulkRequest, executorName, actionListener); } @Override diff --git a/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionIndicesThatCannotBeCreatedTests.java b/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionIndicesThatCannotBeCreatedTests.java index d426920b1d0dc..9a6d1eced5876 100644 --- a/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionIndicesThatCannotBeCreatedTests.java +++ b/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionIndicesThatCannotBeCreatedTests.java @@ -124,7 +124,6 @@ void executeBulk( long startTimeNanos, ActionListener listener, String executorName, - boolean onTransportThread, AtomicArray responses, Map indicesThatCannotBeCreated ) { diff --git a/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionIngestTests.java b/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionIngestTests.java index 3715cdb35dcb1..794006c18feb9 100644 --- a/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionIngestTests.java +++ b/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionIngestTests.java @@ -143,7 +143,6 @@ void executeBulk( long startTimeNanos, ActionListener listener, String executorName, - boolean onTransportThread, AtomicArray responses, Map indicesThatCannotBeCreated ) { diff --git a/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTookTests.java b/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTookTests.java index de868ebc2d7b7..99808fc820853 100644 --- a/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTookTests.java +++ b/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTookTests.java @@ -124,7 +124,6 @@ void executeBulk( long startTimeNanos, ActionListener listener, String executorName, - boolean onTransportThread, AtomicArray responses, Map indicesThatCannotBeCreated ) { @@ -135,7 +134,6 @@ void executeBulk( startTimeNanos, listener, executorName, - onTransportThread, responses, indicesThatCannotBeCreated ); @@ -158,7 +156,6 @@ void executeBulk( long startTimeNanos, ActionListener listener, String executorName, - boolean onTransportThread, AtomicArray responses, Map indicesThatCannotBeCreated ) { @@ -170,7 +167,6 @@ void executeBulk( startTimeNanos, listener, executorName, - onTransportThread, responses, indicesThatCannotBeCreated ); From b3eae865f578b79baa8ef2557fa2687f758ce051 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Fri, 1 Oct 2021 13:56:59 -0400 Subject: [PATCH 12/31] Retry? --- .../org/elasticsearch/action/bulk/BulkProcessorRetryIT.java | 5 ++--- .../src/main/java/org/elasticsearch/action/bulk/Retry.java | 6 +++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/action/bulk/BulkProcessorRetryIT.java b/server/src/internalClusterTest/java/org/elasticsearch/action/bulk/BulkProcessorRetryIT.java index 23141f0ec3e42..4e00a57ad647d 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/action/bulk/BulkProcessorRetryIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/action/bulk/BulkProcessorRetryIT.java @@ -7,6 +7,7 @@ */ package org.elasticsearch.action.bulk; +import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.action.admin.indices.refresh.RefreshRequest; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.common.settings.Settings; @@ -14,7 +15,6 @@ import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.test.ESIntegTestCase; -import org.elasticsearch.transport.RemoteTransportException; import java.util.Collections; import java.util.Iterator; @@ -115,8 +115,7 @@ public void afterBulk(long executionId, BulkRequest request, Throwable failure) } } } else { - if (response instanceof RemoteTransportException - && ((RemoteTransportException) response).status() == RestStatus.TOO_MANY_REQUESTS) { + if (ExceptionsHelper.status((Throwable) response) == RestStatus.TOO_MANY_REQUESTS) { if (rejectedExecutionExpected == false) { assertRetriedCorrectly(internalPolicy, response, ((Throwable) response).getCause()); rejectedAfterAllRetries = true; diff --git a/server/src/main/java/org/elasticsearch/action/bulk/Retry.java b/server/src/main/java/org/elasticsearch/action/bulk/Retry.java index 628504f178c1b..0067968bd975a 100644 --- a/server/src/main/java/org/elasticsearch/action/bulk/Retry.java +++ b/server/src/main/java/org/elasticsearch/action/bulk/Retry.java @@ -7,15 +7,15 @@ */ package org.elasticsearch.action.bulk; -import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.core.TimeValue; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.threadpool.Scheduler; import org.elasticsearch.threadpool.ThreadPool; -import org.elasticsearch.transport.RemoteTransportException; import java.util.ArrayList; import java.util.Iterator; @@ -107,7 +107,7 @@ public void onResponse(BulkResponse bulkItemResponses) { @Override public void onFailure(Exception e) { - if (e instanceof RemoteTransportException && ((RemoteTransportException) e).status() == RETRY_STATUS && backoff.hasNext()) { + if (ExceptionsHelper.status(e) == RETRY_STATUS && backoff.hasNext()) { retry(currentBulkRequest); } else { try { From 7147bb711ac5d6e7f71bc92ba2e6b2e54beeae96 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 4 Oct 2021 15:01:36 -0400 Subject: [PATCH 13/31] Remove pressure test arm --- .../index/IndexingPressureIT.java | 51 ++++++++----------- 1 file changed, 20 insertions(+), 31 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/index/IndexingPressureIT.java b/server/src/internalClusterTest/java/org/elasticsearch/index/IndexingPressureIT.java index 4067f619b150f..4efd0b1df0f08 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/index/IndexingPressureIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/index/IndexingPressureIT.java @@ -161,45 +161,34 @@ public void testWriteIndexingPressureMetricsAreIncremented() throws Exception { final BulkRequest secondBulkRequest = new BulkRequest(); secondBulkRequest.add(request); - // Use the primary or the replica data node as the coordinating node this time - boolean usePrimaryAsCoordinatingNode = randomBoolean(); - final ActionFuture secondFuture; - if (usePrimaryAsCoordinatingNode) { - secondFuture = client(primaryName).bulk(secondBulkRequest); - } else { - secondFuture = client(replicaName).bulk(secondBulkRequest); - } + /* + * Use the primary as the coordinating node this time. + * We never use the replica as the coordinating node because + * we try to go async immediately and the replica's thread + * pool is stuffed. + */ + final ActionFuture secondFuture = client(primaryName).bulk(secondBulkRequest); final long secondBulkRequestSize = secondBulkRequest.ramBytesUsed(); final long secondBulkShardRequestSize = request.ramBytesUsed(); final long secondBulkOps = secondBulkRequest.numberOfActions(); - if (usePrimaryAsCoordinatingNode) { - assertBusy(() -> { - assertThat(primaryWriteLimits.stats().getCurrentCombinedCoordinatingAndPrimaryBytes(), - greaterThan(bulkShardRequestSize + secondBulkRequestSize)); - assertEquals(secondBulkRequestSize, primaryWriteLimits.stats().getCurrentCoordinatingBytes()); - assertEquals(secondBulkOps, primaryWriteLimits.stats().getCurrentCoordinatingOps()); - assertThat(primaryWriteLimits.stats().getCurrentPrimaryBytes(), - greaterThan(bulkShardRequestSize + secondBulkRequestSize)); - assertThat(primaryWriteLimits.stats().getCurrentPrimaryOps(), - equalTo(bulkOps + secondBulkOps)); - - assertEquals(0, replicaWriteLimits.stats().getCurrentCombinedCoordinatingAndPrimaryBytes()); - assertEquals(0, replicaWriteLimits.stats().getCurrentCoordinatingBytes()); - assertEquals(0, replicaWriteLimits.stats().getCurrentCoordinatingOps()); - assertEquals(0, replicaWriteLimits.stats().getCurrentPrimaryBytes()); - assertEquals(0, replicaWriteLimits.stats().getCurrentPrimaryOps()); - }); - } else { - assertThat(primaryWriteLimits.stats().getCurrentCombinedCoordinatingAndPrimaryBytes(), greaterThan(bulkShardRequestSize)); + assertBusy(() -> { + assertThat(primaryWriteLimits.stats().getCurrentCombinedCoordinatingAndPrimaryBytes(), + greaterThan(bulkShardRequestSize + secondBulkRequestSize)); + assertEquals(secondBulkRequestSize, primaryWriteLimits.stats().getCurrentCoordinatingBytes()); + assertEquals(secondBulkOps, primaryWriteLimits.stats().getCurrentCoordinatingOps()); + assertThat(primaryWriteLimits.stats().getCurrentPrimaryBytes(), + greaterThan(bulkShardRequestSize + secondBulkRequestSize)); + assertThat(primaryWriteLimits.stats().getCurrentPrimaryOps(), + equalTo(bulkOps + secondBulkOps)); - assertEquals(secondBulkRequestSize, replicaWriteLimits.stats().getCurrentCombinedCoordinatingAndPrimaryBytes()); - assertEquals(secondBulkRequestSize, replicaWriteLimits.stats().getCurrentCoordinatingBytes()); - assertEquals(secondBulkOps, replicaWriteLimits.stats().getCurrentCoordinatingOps()); + assertEquals(0, replicaWriteLimits.stats().getCurrentCombinedCoordinatingAndPrimaryBytes()); + assertEquals(0, replicaWriteLimits.stats().getCurrentCoordinatingBytes()); + assertEquals(0, replicaWriteLimits.stats().getCurrentCoordinatingOps()); assertEquals(0, replicaWriteLimits.stats().getCurrentPrimaryBytes()); assertEquals(0, replicaWriteLimits.stats().getCurrentPrimaryOps()); - } + }); assertEquals(bulkRequestSize, coordinatingWriteLimits.stats().getCurrentCombinedCoordinatingAndPrimaryBytes()); assertBusy(() -> assertThat(replicaWriteLimits.stats().getCurrentReplicaBytes(), greaterThan(bulkShardRequestSize + secondBulkShardRequestSize))); From e86650f1bda14ce8bbf67c48f3b4537625e503df Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 4 Oct 2021 16:37:07 -0400 Subject: [PATCH 14/31] Add missing settings --- .../test/multi_cluster/100_tsdb.yml | 1 + .../test/remote_cluster/10_basic.yml | 1 + .../elasticsearch/upgrades/IndexingIT.java | 5 ++- ...dimension_and_metric_in_non_tsdb_index.yml | 2 - .../rest-api-spec/test/tsdb/20_mapping.yml | 1 + .../rest-api-spec/test/tsdb/30_snapshot.yml | 1 + .../rest-api-spec/test/tsdb/40_search.yml | 1 + .../rest-api-spec/test/tsdb/50_alias.yml | 1 + .../test/tsdb/60_add_dimensions.yml | 5 +++ .../test/tsdb/70_dimension_types.yml | 41 +++++++++++-------- .../test/tsdb/80_index_resize.yml | 1 + ...ions.yml => 90_unsupported_operations.yml} | 0 12 files changed, 41 insertions(+), 19 deletions(-) rename rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/{30_unsupported_operations.yml => 90_unsupported_operations.yml} (100%) diff --git a/qa/multi-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/100_tsdb.yml b/qa/multi-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/100_tsdb.yml index 1c7d4aeee8a82..a3c73d7f434d6 100644 --- a/qa/multi-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/100_tsdb.yml +++ b/qa/multi-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/100_tsdb.yml @@ -12,6 +12,7 @@ setup: settings: index: mode: time_series + routing_path: [metricset, k8s.pod.uid] number_of_replicas: 0 number_of_shards: 2 mappings: diff --git a/qa/multi-cluster-search/src/test/resources/rest-api-spec/test/remote_cluster/10_basic.yml b/qa/multi-cluster-search/src/test/resources/rest-api-spec/test/remote_cluster/10_basic.yml index 5aa5bd659a1c2..4f46d103c238f 100644 --- a/qa/multi-cluster-search/src/test/resources/rest-api-spec/test/remote_cluster/10_basic.yml +++ b/qa/multi-cluster-search/src/test/resources/rest-api-spec/test/remote_cluster/10_basic.yml @@ -166,6 +166,7 @@ tsdb: settings: index: mode: time_series + routing_path: [metricset, k8s.pod.uid] number_of_replicas: 0 number_of_shards: 2 mappings: diff --git a/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/IndexingIT.java b/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/IndexingIT.java index 065e3d02c9034..fe1e6ea8e192b 100644 --- a/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/IndexingIT.java +++ b/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/IndexingIT.java @@ -318,7 +318,10 @@ private void createTsdbIndex() throws IOException { indexSpec.startObject("dim").field("type", "keyword").field("time_series_dimension", true).endObject(); } indexSpec.endObject().endObject(); - indexSpec.startObject("settings").field("mode", "time_series").endObject(); + indexSpec.startObject("settings").startObject("index"); + indexSpec.field("mode", "time_series"); + indexSpec.array("routing_path", new String[] {"metricset", "k8s.pod.uid"}); + indexSpec.endObject().endObject(); createIndex.setJsonEntity(Strings.toString(indexSpec.endObject())); client().performRequest(createIndex); } diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/05_dimension_and_metric_in_non_tsdb_index.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/05_dimension_and_metric_in_non_tsdb_index.yml index 3ca3545a51b21..f32dfb34bfee1 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/05_dimension_and_metric_in_non_tsdb_index.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/05_dimension_and_metric_in_non_tsdb_index.yml @@ -9,8 +9,6 @@ add time series mappings: body: settings: index: - mode: time_series - routing_path: metricset number_of_replicas: 0 number_of_shards: 2 mappings: diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/20_mapping.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/20_mapping.yml index ae26ec917f031..597c6488e6827 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/20_mapping.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/20_mapping.yml @@ -10,6 +10,7 @@ add time series mappings: settings: index: mode: time_series + routing_path: [metricset, k8s.pod.uid] number_of_replicas: 0 number_of_shards: 2 mappings: diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/30_snapshot.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/30_snapshot.yml index 6213311944e5e..e606e4dd82ca2 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/30_snapshot.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/30_snapshot.yml @@ -29,6 +29,7 @@ teardown: settings: index: mode: time_series + routing_path: [metricset, k8s.pod.uid] number_of_replicas: 0 number_of_shards: 2 mappings: diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/40_search.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/40_search.yml index 5227377249e66..223d87ab96a09 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/40_search.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/40_search.yml @@ -10,6 +10,7 @@ setup: settings: index: mode: time_series + routing_path: [metricset, k8s.pod.uid] mappings: properties: "@timestamp": diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/50_alias.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/50_alias.yml index 21ddeb55b13c3..5a187ce0b6430 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/50_alias.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/50_alias.yml @@ -10,6 +10,7 @@ setup: settings: index: mode: time_series + routing_path: [metricset, k8s.pod.uid] mappings: properties: "@timestamp": diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/60_add_dimensions.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/60_add_dimensions.yml index f593f554824cb..ca4aa52e15a13 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/60_add_dimensions.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/60_add_dimensions.yml @@ -11,6 +11,7 @@ add dimensions with put_mapping: settings: index: mode: time_series + routing_path: [metricset, k8s.pod.uid] mappings: properties: "@timestamp": @@ -60,6 +61,7 @@ add dimensions to no dims with dynamic_template over index: settings: index: mode: time_series + routing_path: [metricset] mappings: dynamic_templates: - keywords: @@ -104,6 +106,7 @@ add dimensions to no dims with dynamic_template over bulk: settings: index: mode: time_series + routing_path: [metricset] mappings: dynamic_templates: - keywords: @@ -148,6 +151,7 @@ add dimensions to some dims with dynamic_template over index: settings: index: mode: time_series + routing_path: [metricset] mappings: dynamic_templates: - keywords: @@ -196,6 +200,7 @@ add dimensions to some dims with dynamic_template over bulk: settings: index: mode: time_series + routing_path: [metricset] mappings: dynamic_templates: - keywords: diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/70_dimension_types.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/70_dimension_types.yml index 80d049a7e43d2..06eb087567238 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/70_dimension_types.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/70_dimension_types.yml @@ -11,6 +11,7 @@ keyword dimension: settings: index: mode: time_series + routing_path: [uid] mappings: properties: "@timestamp": @@ -58,10 +59,14 @@ long dimension: settings: index: mode: time_series + routing_path: [metricset] mappings: properties: "@timestamp": type: date + metricset: + type: keyword + time_series_dimension: true id: type: long time_series_dimension: true @@ -72,21 +77,21 @@ long dimension: index: test body: - '{"index": {}}' - - '{"@timestamp": "2021-04-28T18:35:24.467Z", "id": 1, "voltage": 7.2}' + - '{"@timestamp": "2021-04-28T18:35:24.467Z", "metricset": "aa", "id": 1, "voltage": 7.2}' - '{"index": {}}' - - '{"@timestamp": "2021-04-28T18:35:34.467Z", "id": "1", "voltage": 7.6}' + - '{"@timestamp": "2021-04-28T18:35:34.467Z", "metricset": "aa", "id": "1", "voltage": 7.6}' - '{"index": {}}' - - '{"@timestamp": "2021-04-28T18:35:44.467Z", "id": 1.0, "voltage": 7.1}' + - '{"@timestamp": "2021-04-28T18:35:44.467Z", "metricset": "aa", "id": 1.0, "voltage": 7.1}' - '{"index": {}}' - - '{"@timestamp": "2021-04-28T18:35:54.467Z", "id": "001", "voltage": 7.3}' + - '{"@timestamp": "2021-04-28T18:35:54.467Z", "metricset": "aa", "id": "001", "voltage": 7.3}' - '{"index": {}}' - - '{"@timestamp": "2021-04-28T18:35:24.467Z", "id": 2, "voltage": 3.2}' + - '{"@timestamp": "2021-04-28T18:35:24.467Z", "metricset": "aa", "id": 2, "voltage": 3.2}' - '{"index": {}}' - - '{"@timestamp": "2021-04-28T18:35:34.467Z", "id": 2, "voltage": 3.6}' + - '{"@timestamp": "2021-04-28T18:35:34.467Z", "metricset": "aa", "id": 2, "voltage": 3.6}' - '{"index": {}}' - - '{"@timestamp": "2021-04-28T18:35:44.467Z", "id": 2, "voltage": 3.1}' + - '{"@timestamp": "2021-04-28T18:35:44.467Z", "metricset": "aa", "id": 2, "voltage": 3.1}' - '{"index": {}}' - - '{"@timestamp": "2021-04-28T18:35:54.467Z", "id": 2, "voltage": 3.3}' + - '{"@timestamp": "2021-04-28T18:35:54.467Z", "metricset": "aa", "id": 2, "voltage": 3.3}' # TODO aggregate on tsid @@ -104,10 +109,14 @@ ip dimension: settings: index: mode: time_series + routing_path: [metricset] mappings: properties: "@timestamp": type: date + metricset: + type: keyword + time_series_dimension: true ip: type: ip time_series_dimension: true @@ -118,20 +127,20 @@ ip dimension: index: test body: - '{"index": {}}' - - '{"@timestamp": "2021-04-28T18:35:24.467Z", "ip": "10.10.1.1", "voltage": 7.2}' + - '{"@timestamp": "2021-04-28T18:35:24.467Z", "metricset": "aa", "ip": "10.10.1.1", "voltage": 7.2}' - '{"index": {}}' - - '{"@timestamp": "2021-04-28T18:35:34.467Z", "ip": "10.10.1.1", "voltage": 7.6}' + - '{"@timestamp": "2021-04-28T18:35:34.467Z", "metricset": "aa", "ip": "10.10.1.1", "voltage": 7.6}' - '{"index": {}}' - - '{"@timestamp": "2021-04-28T18:35:44.467Z", "ip": "10.10.1.1", "voltage": 7.1}' + - '{"@timestamp": "2021-04-28T18:35:44.467Z", "metricset": "aa", "ip": "10.10.1.1", "voltage": 7.1}' - '{"index": {}}' - - '{"@timestamp": "2021-04-28T18:35:54.467Z", "ip": "::ffff:10.10.1.1", "voltage": 7.3}' + - '{"@timestamp": "2021-04-28T18:35:54.467Z", "metricset": "aa", "ip": "::ffff:10.10.1.1", "voltage": 7.3}' - '{"index": {}}' - - '{"@timestamp": "2021-04-28T18:35:24.467Z", "ip": "2001:0db8:85a3:0000:0000:8a2e:0370:7334", "voltage": 3.2}' + - '{"@timestamp": "2021-04-28T18:35:24.467Z", "metricset": "aa", "ip": "2001:0db8:85a3:0000:0000:8a2e:0370:7334", "voltage": 3.2}' - '{"index": {}}' - - '{"@timestamp": "2021-04-28T18:35:34.467Z", "ip": "2001:0db8:85a3:0:0:8a2e:0370:7334", "voltage": 3.6}' + - '{"@timestamp": "2021-04-28T18:35:34.467Z", "metricset": "aa", "ip": "2001:0db8:85a3:0:0:8a2e:0370:7334", "voltage": 3.6}' - '{"index": {}}' - - '{"@timestamp": "2021-04-28T18:35:44.467Z", "ip": "2001:0db8:85a3::8a2e:0370:7334", "voltage": 3.1}' + - '{"@timestamp": "2021-04-28T18:35:44.467Z", "metricset": "aa", "ip": "2001:0db8:85a3::8a2e:0370:7334", "voltage": 3.1}' - '{"index": {}}' - - '{"@timestamp": "2021-04-28T18:35:54.467Z", "ip": "2001:0db8:85a3::8a2e:0370:7334", "voltage": 3.3}' + - '{"@timestamp": "2021-04-28T18:35:54.467Z", "metricset": "aa", "ip": "2001:0db8:85a3::8a2e:0370:7334", "voltage": 3.3}' # TODO aggregate on tsid diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/80_index_resize.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/80_index_resize.yml index b91b5e55150c5..8356d08c2ba5b 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/80_index_resize.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/80_index_resize.yml @@ -10,6 +10,7 @@ setup: settings: index: mode: time_series + routing_path: [metricset, k8s.pod.uid] number_of_shards: 3 number_of_replicas: 0 mappings: diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/30_unsupported_operations.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/90_unsupported_operations.yml similarity index 100% rename from rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/30_unsupported_operations.yml rename to rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/90_unsupported_operations.yml From f021e724304c0b7c3ef164e06da88707901cc1c8 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 4 Oct 2021 17:13:11 -0400 Subject: [PATCH 15/31] WIP --- .../src/test/java/org/elasticsearch/upgrades/IndexingIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/IndexingIT.java b/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/IndexingIT.java index fe1e6ea8e192b..ad74d0f88b2de 100644 --- a/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/IndexingIT.java +++ b/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/IndexingIT.java @@ -323,7 +323,7 @@ private void createTsdbIndex() throws IOException { indexSpec.array("routing_path", new String[] {"metricset", "k8s.pod.uid"}); indexSpec.endObject().endObject(); createIndex.setJsonEntity(Strings.toString(indexSpec.endObject())); - client().performRequest(createIndex); + assertEquals(client().performRequest(createIndex).getEntity().toString(), "ADSFADSF"); } private void tsdbBulk(StringBuilder bulk, String dim, long timeStart, long timeEnd, double rate) throws IOException { From 0761120d5f3ed6e231a88c181d174b3127885813 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Wed, 6 Oct 2021 16:58:17 -0400 Subject: [PATCH 16/31] Remove leftover --- .../cluster/routing/IndexRouting.java | 15 --------------- 1 file changed, 15 deletions(-) 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 28691137e420d..b8d00abb4b8d9 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java @@ -67,11 +67,6 @@ private IndexRouting(int routingNumShards, int routingFactor) { this.routingFactor = routingFactor; } - /** - * Should the routing generation calls be forked from the transport thread? - */ - public abstract boolean mustForkFromTransportThread(); - /** * Called when indexing a document to generate the shard id that should contain * a document with the provided parameters. @@ -132,11 +127,6 @@ private abstract static class IdAndRoutingOnly extends IndexRouting { protected abstract int shardId(String id, @Nullable String routing); - @Override - public boolean mustForkFromTransportThread() { - return false; - } - @Override public int indexShard(String id, @Nullable String routing, XContentType sourceType, BytesReference source) { return shardId(id, routing); @@ -216,11 +206,6 @@ private static class ExtractFromSource extends IndexRouting { this.include = FilterPath.compile(Set.copyOf(routingPaths)); } - @Override - public boolean mustForkFromTransportThread() { - return true; - } - @Override public int indexShard(String id, @Nullable String routing, XContentType sourceType, BytesReference source) { if (routing != null) { From d50181159130d046910816797159ce27a730720f Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Wed, 6 Oct 2021 17:41:16 -0400 Subject: [PATCH 17/31] More cleaning --- .../action/bulk/TransportBulkAction.java | 18 ------------------ .../test/data_stream/150_tsdb.yml | 1 + 2 files changed, 1 insertion(+), 18 deletions(-) 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 ee1755fb368da..bdbf88bc52364 100644 --- a/server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java +++ b/server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java @@ -885,22 +885,4 @@ synchronized void markItemAsFailed(int slot, Exception e) { itemResponses.add(BulkItemResponse.failure(slot, indexRequest.opType(), failure)); } } - - boolean routingMustForkFromTransportThread(Iterable> requests, ConcreteIndices concreteIndices) { - for (DocWriteRequest request : requests) { - if (request == null) { - continue; - } - Index index; - try { - index = concreteIndices.resolveIfAbsent(request); - } catch (IndexClosedException | IndexNotFoundException | IllegalArgumentException ex) { - continue; - } - if (concreteIndices.routing(index).mustForkFromTransportThread()) { - return true; - } - } - return false; - } } diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/data_stream/150_tsdb.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/data_stream/150_tsdb.yml index 3a4b18dfe1200..a4db59eb5c1c0 100644 --- a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/data_stream/150_tsdb.yml +++ b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/data_stream/150_tsdb.yml @@ -18,6 +18,7 @@ setup: number_of_replicas: 0 number_of_shards: 2 mode: time_series + routing_path: [metricset, time_series_dimension] mappings: properties: "@timestamp": From 07b2435b954fa649f6b3d5143622269092d699c1 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 7 Oct 2021 10:26:31 -0400 Subject: [PATCH 18/31] Fixup more tests --- .../test/java/org/elasticsearch/upgrades/IndexingIT.java | 6 +++--- .../java/org/elasticsearch/xpack/ccr/FollowIndexIT.java | 6 +++++- .../resources/rest-api-spec/test/analytics/histogram.yml | 1 + 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/IndexingIT.java b/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/IndexingIT.java index ad74d0f88b2de..a0624feeb30ce 100644 --- a/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/IndexingIT.java +++ b/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/IndexingIT.java @@ -306,7 +306,7 @@ private void bulk(String index, String entity) throws IOException { Request bulk = new Request("POST", "/_bulk"); bulk.addParameter("refresh", "true"); bulk.setJsonEntity(entity.toString()); - client().performRequest(bulk); + assertThat(EntityUtils.toString(client().performRequest(bulk).getEntity()), containsString("\"errors\":false")); } private void createTsdbIndex() throws IOException { @@ -320,10 +320,10 @@ private void createTsdbIndex() throws IOException { indexSpec.endObject().endObject(); indexSpec.startObject("settings").startObject("index"); indexSpec.field("mode", "time_series"); - indexSpec.array("routing_path", new String[] {"metricset", "k8s.pod.uid"}); + indexSpec.array("routing_path", new String[] {"dim"}); indexSpec.endObject().endObject(); createIndex.setJsonEntity(Strings.toString(indexSpec.endObject())); - assertEquals(client().performRequest(createIndex).getEntity().toString(), "ADSFADSF"); + client().performRequest(createIndex); } private void tsdbBulk(StringBuilder bulk, String dim, long timeStart, long timeEnd, double rate) throws IOException { diff --git a/x-pack/plugin/ccr/qa/multi-cluster/src/test/java/org/elasticsearch/xpack/ccr/FollowIndexIT.java b/x-pack/plugin/ccr/qa/multi-cluster/src/test/java/org/elasticsearch/xpack/ccr/FollowIndexIT.java index 0fee6e7c82dc5..6b8516b3597ed 100644 --- a/x-pack/plugin/ccr/qa/multi-cluster/src/test/java/org/elasticsearch/xpack/ccr/FollowIndexIT.java +++ b/x-pack/plugin/ccr/qa/multi-cluster/src/test/java/org/elasticsearch/xpack/ccr/FollowIndexIT.java @@ -12,6 +12,7 @@ import org.elasticsearch.client.ResponseException; import org.elasticsearch.client.RestClient; import org.elasticsearch.cluster.metadata.DataStream; +import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; @@ -247,7 +248,10 @@ public void testFollowTsdbIndex() throws Exception { logger.info("Running against leader cluster"); createIndex( leaderIndexName, - Settings.builder().put(IndexSettings.MODE.getKey(), "time_series").build(), + Settings.builder() + .put(IndexSettings.MODE.getKey(), "time_series") + .put(IndexMetadata.INDEX_ROUTING_PATH.getKey(), "dim") + .build(), "\"properties\": {\"@timestamp\": {\"type\": \"date\"}, \"dim\": {\"type\": \"keyword\", \"time_series_dimension\": true}}" ); for (int i = 0; i < numDocs; i++) { diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/analytics/histogram.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/analytics/histogram.yml index d9a4eb706075c..a8bc88f7f7f07 100644 --- a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/analytics/histogram.yml +++ b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/analytics/histogram.yml @@ -186,6 +186,7 @@ histogram with time series mappings: settings: index: mode: time_series + routing_path: [metricset, k8s.pod.uid] number_of_replicas: 0 number_of_shards: 2 mappings: From 7d81f2a143c020e88ca2323ea5e900611b304b26 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 7 Oct 2021 10:29:17 -0400 Subject: [PATCH 19/31] Remove old skip --- rest-api-spec/build.gradle | 1 - 1 file changed, 1 deletion(-) diff --git a/rest-api-spec/build.gradle b/rest-api-spec/build.gradle index ac24ca3440453..009723d597dde 100644 --- a/rest-api-spec/build.gradle +++ b/rest-api-spec/build.gradle @@ -81,7 +81,6 @@ tasks.named("yamlRestTestV7CompatTransform").configure { task -> task.skipTest("search.aggregation/20_terms/string profiler via global ordinals native implementation", "The profiler results aren't backwards compatible.") task.skipTest("search.aggregation/20_terms/string profiler via map", "The profiler results aren't backwards compatible.") task.skipTest("search.aggregation/20_terms/numeric profiler", "The profiler results aren't backwards compatible.") - task.skipTest("tsdb/10_settings/enable", "The profiler results aren't backwards compatible.") // Remove once we remove the time_series mode setting from 7.x task.replaceValueInMatch("_type", "_doc") task.addAllowedWarningRegex("\\[types removal\\].*") task.replaceValueInMatch("nodes.\$node_id.roles.8", "ml", "node_info role test") From 99bbf13206ccef39ce58e145e4562b793ff17f36 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 7 Oct 2021 10:47:20 -0400 Subject: [PATCH 20/31] New tests for Retry --- .../elasticsearch/action/bulk/RetryTests.java | 46 ++++++++++++++----- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/action/bulk/RetryTests.java b/server/src/test/java/org/elasticsearch/action/bulk/RetryTests.java index e79a86ae5148b..c9bb61f789e28 100644 --- a/server/src/test/java/org/elasticsearch/action/bulk/RetryTests.java +++ b/server/src/test/java/org/elasticsearch/action/bulk/RetryTests.java @@ -13,8 +13,8 @@ import org.elasticsearch.action.delete.DeleteResponse; import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.action.update.UpdateRequest; -import org.elasticsearch.core.TimeValue; import org.elasticsearch.common.util.concurrent.EsRejectedExecutionException; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.client.NoOpClient; @@ -27,11 +27,12 @@ import java.util.concurrent.atomic.AtomicInteger; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; public class RetryTests extends ESTestCase { - // no need to wait fof a long time in tests + // no need to wait for a long time in tests private static final TimeValue DELAY = TimeValue.timeValueMillis(1L); private static final int CALLS_TO_FAIL = 5; @@ -86,12 +87,20 @@ public void testRetryFailsAfterBackoff() throws Exception { BackoffPolicy backoff = BackoffPolicy.constantBackoff(DELAY, CALLS_TO_FAIL - 1); BulkRequest bulkRequest = createBulkRequest(); - BulkResponse response = new Retry(backoff, bulkClient.threadPool()) - .withBackoff(bulkClient::bulk, bulkRequest) - .actionGet(); + try { + BulkResponse response = new Retry(backoff, bulkClient.threadPool()).withBackoff(bulkClient::bulk, bulkRequest).actionGet(); + /* + * If the last failure was an item failure we'll end up here + */ + assertTrue(response.hasFailures()); + assertThat(response.getItems().length, equalTo(bulkRequest.numberOfActions())); + } catch (EsRejectedExecutionException e) { + /* + * If the last failure was a rejection we'll end up here. + */ + assertThat(e.getMessage(), equalTo("pretend the coordinating thread pool is stuffed")); + } - assertTrue(response.hasFailures()); - assertThat(response.getItems().length, equalTo(bulkRequest.numberOfActions())); } public void testRetryWithListenerBacksOff() throws Exception { @@ -119,10 +128,20 @@ public void testRetryWithListenerFailsAfterBacksOff() throws Exception { listener.awaitCallbacksCalled(); - listener.assertOnResponseCalled(); - listener.assertResponseWithFailures(); - listener.assertResponseWithNumberOfItems(bulkRequest.numberOfActions()); - listener.assertOnFailureNeverCalled(); + if (listener.lastFailure == null) { + /* + * If the last failure was an item failure we'll end up here. + */ + listener.assertOnResponseCalled(); + listener.assertResponseWithFailures(); + listener.assertResponseWithNumberOfItems(bulkRequest.numberOfActions()); + } else { + /* + * If the last failure was a rejection we'll end up here. + */ + assertThat(listener.lastFailure, instanceOf(EsRejectedExecutionException.class)); + assertThat(listener.lastFailure.getMessage(), equalTo("pretend the coordinating thread pool is stuffed")); + } } private static class AssertingListener implements ActionListener { @@ -202,6 +221,11 @@ public void bulk(BulkRequest request, ActionListener listener) { boolean shouldFail = numberOfCallsToFail > 0; numberOfCallsToFail--; + if (shouldFail && randomBoolean()) { + listener.onFailure(new EsRejectedExecutionException("pretend the coordinating thread pool is stuffed")); + return; + } + BulkItemResponse[] itemResponses = new BulkItemResponse[request.requests().size()]; // if we have to fail, we need to fail at least once "reliably", the rest can be random int itemToFail = randomInt(request.requests().size() - 1); From 671c83fdd65485c47346185418ad687280e81057 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 7 Oct 2021 11:09:42 -0400 Subject: [PATCH 21/31] More tests --- .../action/bulk/TransportBulkActionTests.java | 34 +++++++++++-------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTests.java b/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTests.java index 7e0aad6542346..063549fdbb4eb 100644 --- a/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTests.java @@ -30,8 +30,8 @@ import org.elasticsearch.cluster.node.DiscoveryNodeRole; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.core.TimeValue; import org.elasticsearch.common.util.concurrent.EsRejectedExecutionException; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.index.IndexingPressure; import org.elasticsearch.index.VersionType; @@ -73,6 +73,7 @@ class TestTransportBulkAction extends TransportBulkAction { volatile boolean failIndexCreation = false; boolean indexCreated = false; // set when the "real" index is created + Runnable beforeIndexCreation = null; TestTransportBulkAction() { super(TransportBulkActionTests.this.threadPool, transportService, clusterService, null, @@ -83,6 +84,9 @@ null, new ActionFilters(Collections.emptySet()), new Resolver(), @Override void createIndex(String index, TimeValue timeout, Version minNodeVersion, ActionListener listener) { indexCreated = true; + if (beforeIndexCreation != null) { + beforeIndexCreation.run(); + } if (failIndexCreation) { listener.onFailure(new ResourceAlreadyExistsException("index already exists")); } else { @@ -259,27 +263,29 @@ public void testOnlySystem() { assertFalse(bulkAction.isOnlySystem(buildBulkRequest(mixed), indicesLookup, systemIndices)); } -// public void testAllRoutingOkOnTransportThread() { -// SortedMap indicesLookup = new TreeMap<>(); -// Settings settings = Settings.builder().put("index.version.created", Version.CURRENT).build(); -// indicesLookup.put(".foo", -// new Index(IndexMetadata.builder(".foo").settings(settings).system(true).numberOfShards(1).numberOfReplicas(0).build())); -// indicesLookup.put(".bar", -// new Index(IndexMetadata.builder(".bar").settings(settings).system(true).numberOfShards(1).numberOfReplicas(0).build())); -// List onlySystem = List.of(".foo", ".bar"); -// assertTrue(bulkAction.allRoutingOkOnTransportThread(buildBulkRequest(onlySystem), indicesLookup)); -// -// } + public void testRejectCoordination() throws Exception { + BulkRequest bulkRequest = new BulkRequest().add(new IndexRequest("index").id("id").source(Collections.emptyMap())); + + try { + threadPool.startForcingRejections(); + PlainActionFuture future = PlainActionFuture.newFuture(); + ActionTestUtils.execute(bulkAction, null, bulkRequest, future); + expectThrows(EsRejectedExecutionException.class, future::actionGet); + } finally { + threadPool.stopForcingRejections(); + } + } public void testRejectionAfterCreateIndexIsPropagated() throws Exception { BulkRequest bulkRequest = new BulkRequest().add(new IndexRequest("index").id("id").source(Collections.emptyMap())); - bulkAction.failIndexCreation = randomBoolean(); + bulkAction.failIndexCreation = randomBoolean(); try { - threadPool.startForcingRejections(); + bulkAction.beforeIndexCreation = threadPool::startForcingRejections; PlainActionFuture future = PlainActionFuture.newFuture(); ActionTestUtils.execute(bulkAction, null, bulkRequest, future); expectThrows(EsRejectedExecutionException.class, future::actionGet); + assertTrue(bulkAction.indexCreated); } finally { threadPool.stopForcingRejections(); } From 0f86b77873b58956cadab79bc94476eab74c8987 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 7 Oct 2021 11:10:41 -0400 Subject: [PATCH 22/31] Revert unrelated --- rest-api-spec/build.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/rest-api-spec/build.gradle b/rest-api-spec/build.gradle index 009723d597dde..48c87321e9aaa 100644 --- a/rest-api-spec/build.gradle +++ b/rest-api-spec/build.gradle @@ -81,6 +81,7 @@ tasks.named("yamlRestTestV7CompatTransform").configure { task -> task.skipTest("search.aggregation/20_terms/string profiler via global ordinals native implementation", "The profiler results aren't backwards compatible.") task.skipTest("search.aggregation/20_terms/string profiler via map", "The profiler results aren't backwards compatible.") task.skipTest("search.aggregation/20_terms/numeric profiler", "The profiler results aren't backwards compatible.") + task.replaceValueInMatch("_type", "_doc") task.addAllowedWarningRegex("\\[types removal\\].*") task.replaceValueInMatch("nodes.\$node_id.roles.8", "ml", "node_info role test") From b838554b87ade3799afbb53257d29d3d39c90f5e Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Fri, 15 Oct 2021 09:46:13 -0400 Subject: [PATCH 23/31] One dispatch please --- .../org/elasticsearch/action/bulk/TransportBulkAction.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) 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 e5d82a36a8faf..caf0c76fb87eb 100644 --- a/server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java +++ b/server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java @@ -632,12 +632,7 @@ private void dispatchRetry() { threadPool.executor(executorName).submit(new ActionRunnable<>(listener) { @Override protected void doRun() throws Exception { - threadPool.executor(executorName).execute(new ActionRunnable<>(listener) { - @Override - protected void doRun() throws Exception { - performBulkRequests(); - } - }); + performBulkRequests(); } }); } From b341a64fb8df265485d41faebc623f85d48f2bb6 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Fri, 15 Oct 2021 09:46:20 -0400 Subject: [PATCH 24/31] Stuff moved --- .../elasticsearch/cluster/routing/IndexRouting.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) 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 b8d00abb4b8d9..07cf8a483e065 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java @@ -11,14 +11,14 @@ import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.bytes.BytesReference; -import org.elasticsearch.common.xcontent.DeprecationHandler; -import org.elasticsearch.common.xcontent.NamedXContentRegistry; -import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.common.xcontent.XContentParser.Token; -import org.elasticsearch.common.xcontent.XContentType; -import org.elasticsearch.common.xcontent.support.filtering.FilterPath; import org.elasticsearch.core.Nullable; import org.elasticsearch.transport.Transports; +import org.elasticsearch.xcontent.DeprecationHandler; +import org.elasticsearch.xcontent.NamedXContentRegistry; +import org.elasticsearch.xcontent.XContentParser; +import org.elasticsearch.xcontent.XContentParser.Token; +import org.elasticsearch.xcontent.XContentType; +import org.elasticsearch.xcontent.support.filtering.FilterPath; import java.io.IOException; import java.util.ArrayList; From 5c147d46630dfee9e6bcb8dda23076f0cdf20ae8 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Fri, 15 Oct 2021 10:02:45 -0400 Subject: [PATCH 25/31] More moving --- .../cluster/routing/IndexRoutingTests.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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 4c19700c69769..7899ac5053d63 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/IndexRoutingTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/IndexRoutingTests.java @@ -10,15 +10,15 @@ import org.elasticsearch.Version; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.bytes.BytesReference; -import org.elasticsearch.common.xcontent.DeprecationHandler; -import org.elasticsearch.common.xcontent.NamedXContentRegistry; -import org.elasticsearch.common.xcontent.XContentType; -import org.elasticsearch.common.xcontent.json.JsonXContent; -import org.elasticsearch.common.xcontent.support.MapXContentParser; import org.elasticsearch.core.Nullable; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.VersionUtils; +import org.elasticsearch.xcontent.DeprecationHandler; +import org.elasticsearch.xcontent.NamedXContentRegistry; +import org.elasticsearch.xcontent.XContentType; +import org.elasticsearch.xcontent.json.JsonXContent; +import org.elasticsearch.xcontent.support.MapXContentParser; import java.io.IOException; import java.util.Arrays; From 75392e25209df97d431673e912a1a1e0ef150118 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Fri, 15 Oct 2021 10:16:40 -0400 Subject: [PATCH 26/31] Explain why fork --- .../action/bulk/TransportBulkAction.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) 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 caf0c76fb87eb..3f7a440cfa5cc 100644 --- a/server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java +++ b/server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java @@ -152,8 +152,17 @@ protected void doExecute(Task task, BulkRequest bulkRequest, ActionListener Date: Fri, 15 Oct 2021 10:28:38 -0400 Subject: [PATCH 27/31] Back to ActionRunnable --- .../action/bulk/TransportBulkAction.java | 24 ++++++++----------- 1 file changed, 10 insertions(+), 14 deletions(-) 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 3f7a440cfa5cc..79f9d685762d6 100644 --- a/server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java +++ b/server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java @@ -421,7 +421,7 @@ private long buildTookInMillis(long startTimeNanos) { * retries on retryable cluster blocks, resolves item requests, * constructs shard bulk requests and delegates execution to shard bulk action * */ - private final class BulkOperation { + private final class BulkOperation extends ActionRunnable { private final Task task; private BulkRequest bulkRequest; // set to null once all requests are sent out private final ActionListener listener; @@ -440,6 +440,7 @@ private final class BulkOperation { long startTimeNanos, Map indicesThatCannotBeCreated ) { + super(listener); this.task = task; this.bulkRequest = bulkRequest; this.listener = listener; @@ -450,13 +451,14 @@ private final class BulkOperation { this.observer = new ClusterStateObserver(clusterService, bulkRequest.timeout(), logger, threadPool.getThreadContext()); } - private void performBulkRequests() { + @Override + protected void doRun() { assert bulkRequest != null; final ClusterState clusterState = observer.setAndGetObservedState(); if (handleBlockExceptions(clusterState)) { return; } - ConcreteIndices concreteIndices = new ConcreteIndices(clusterState, indexNameExpressionResolver); + final ConcreteIndices concreteIndices = new ConcreteIndices(clusterState, indexNameExpressionResolver); Metadata metadata = clusterState.metadata(); // Group the requests by ShardId -> Operations mapping Map> requestsByShard = new HashMap<>(); @@ -601,7 +603,7 @@ private boolean handleBlockExceptions(ClusterState state) { logger.trace("cluster is blocked, scheduling a retry", blockException); retry(blockException); } else { - listener.onFailure(blockException); + onFailure(blockException); } return true; } @@ -612,7 +614,7 @@ void retry(Exception failure) { assert failure != null; if (observer.isTimedOut()) { // we running as a last attempt after a timeout has happened. don't retry - listener.onFailure(failure); + onFailure(failure); return; } observer.waitForNextChange(new ClusterStateObserver.Listener() { @@ -623,7 +625,7 @@ public void onNewClusterState(ClusterState state) { @Override public void onClusterServiceClose() { - listener.onFailure(new NodeClosedException(clusterService.localNode())); + onFailure(new NodeClosedException(clusterService.localNode())); } @Override @@ -638,12 +640,7 @@ private void dispatchRetry() { * thread pools to dispatch the bulk request onto its * appropriate thread pool. */ - threadPool.executor(executorName).submit(new ActionRunnable<>(listener) { - @Override - protected void doRun() throws Exception { - performBulkRequests(); - } - }); + threadPool.executor(executorName).submit(BulkOperation.this); } }); } @@ -705,8 +702,7 @@ void executeBulk( AtomicArray responses, Map indicesThatCannotBeCreated ) { - new BulkOperation(task, bulkRequest, listener, executorName, responses, startTimeNanos, indicesThatCannotBeCreated) - .performBulkRequests(); + new BulkOperation(task, bulkRequest, listener, executorName, responses, startTimeNanos, indicesThatCannotBeCreated).run(); } private static class ConcreteIndices { From 076afa514ea389d39a62e873c717169d4d582f77 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Fri, 15 Oct 2021 10:31:14 -0400 Subject: [PATCH 28/31] Update comment --- .../action/bulk/TransportBulkAction.java | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) 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 79f9d685762d6..bdd8c77f85f24 100644 --- a/server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java +++ b/server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java @@ -620,6 +620,12 @@ void retry(Exception failure) { observer.waitForNextChange(new ClusterStateObserver.Listener() { @Override public void onNewClusterState(ClusterState state) { + /* + * This is called on the cluster state update thread pool + * but we'd prefer to coordinate the bulk request on the + * write thread pool just to make sure the cluster state + * update thread doesn't get clogged up. + */ dispatchRetry(); } @@ -630,16 +636,16 @@ public void onClusterServiceClose() { @Override public void onTimeout(TimeValue timeout) { - // Try one more time... + /* + * Try one more time.... This is called on the generic + * thread pool but out of an abundance of caution we + * switch over to the write thread pool that we expect + * to coordinate the bulk request. + */ dispatchRetry(); } private void dispatchRetry() { - /* - * This is called on the cluster state update and timer - * thread pools to dispatch the bulk request onto its - * appropriate thread pool. - */ threadPool.executor(executorName).submit(BulkOperation.this); } }); From db1f179fcc3a652158197ac29ac99f4da36f1d03 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Fri, 15 Oct 2021 10:34:01 -0400 Subject: [PATCH 29/31] Utility method --- .../action/update/TransportUpdateAction.java | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) 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 bc80ebab6fd35..8914f7f7b5075 100644 --- a/server/src/main/java/org/elasticsearch/action/update/TransportUpdateAction.java +++ b/server/src/main/java/org/elasticsearch/action/update/TransportUpdateAction.java @@ -166,15 +166,7 @@ protected ShardIterator shards(ClusterState clusterState, UpdateRequest request) } IndexRouting indexRouting = IndexRouting.fromIndexMetadata(indexMetadata); int shardId = indexRouting.updateShard(request.id(), request.routing()); - ShardIterator shardIterator = RoutingTable.shardRoutingTable(clusterState.routingTable().index(request.concreteIndex()), shardId) - .shardsIt(); - ShardRouting shard; - while ((shard = shardIterator.nextOrNull()) != null) { - if (shard.primary()) { - return new PlainShardIterator(shardIterator.shardId(), Collections.singletonList(shard)); - } - } - return new PlainShardIterator(shardIterator.shardId(), Collections.emptyList()); + return RoutingTable.shardRoutingTable(clusterState.routingTable().index(request.concreteIndex()), shardId).primaryShardIt(); } @Override From 90afc695008a81bc0ad7c4bc8688491aff535710 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Fri, 15 Oct 2021 10:44:46 -0400 Subject: [PATCH 30/31] Move routing_path under feature flag --- .../org/elasticsearch/common/settings/IndexScopedSettings.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java b/server/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java index 8350f4f636512..9e227f6cdd717 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java @@ -60,7 +60,6 @@ public final class IndexScopedSettings extends AbstractScopedSettings { IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING, IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING, IndexMetadata.INDEX_ROUTING_PARTITION_SIZE_SETTING, - IndexMetadata.INDEX_ROUTING_PATH, IndexMetadata.INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING, IndexMetadata.INDEX_READ_ONLY_SETTING, IndexMetadata.INDEX_BLOCKS_READ_SETTING, @@ -185,6 +184,7 @@ private static Set> builtInIndexSettings() { } Set> result = new HashSet<>(ALWAYS_ENABLED_BUILT_IN_INDEX_SETTINGS); result.add(IndexSettings.MODE); + result.add(IndexMetadata.INDEX_ROUTING_PATH); return Set.copyOf(result); } From 63fa3bdfb9193d7d5a8e7468584c5494ab1581a3 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Fri, 15 Oct 2021 11:02:05 -0400 Subject: [PATCH 31/31] Imports --- .../elasticsearch/action/update/TransportUpdateAction.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) 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 8914f7f7b5075..f1f4ec4034b37 100644 --- a/server/src/main/java/org/elasticsearch/action/update/TransportUpdateAction.java +++ b/server/src/main/java/org/elasticsearch/action/update/TransportUpdateAction.java @@ -29,17 +29,14 @@ import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.cluster.routing.IndexRouting; -import org.elasticsearch.cluster.routing.PlainShardIterator; import org.elasticsearch.cluster.routing.RoutingTable; import org.elasticsearch.cluster.routing.ShardIterator; -import org.elasticsearch.cluster.routing.ShardRouting; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.io.stream.NotSerializableExceptionWrapper; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.xcontent.XContentHelper; -import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.core.Tuple; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.index.IndexService; @@ -51,9 +48,9 @@ import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.threadpool.ThreadPool.Names; import org.elasticsearch.transport.TransportService; +import org.elasticsearch.xcontent.XContentType; import java.io.IOException; -import java.util.Collections; import java.util.Map; import static org.elasticsearch.ExceptionsHelper.unwrapCause;