From c7115f83642f8f8347eeed3c2bcb8caf77875c17 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Fri, 3 Apr 2015 19:54:14 +0200 Subject: [PATCH 01/24] Mappings: Bring back numeric_resolution. We had an undocumented parameter called `numeric_resolution` which allows to configure how to deal with dates when provided as a number. The default is to handle them as milliseconds, but you can also opt-on for eg. seconds. Close #10072 --- .../mapping/types/core-types.asciidoc | 3 ++ .../index/mapper/core/DateFieldMapper.java | 8 ++-- .../mapper/date/SimpleDateMappingTests.java | 38 ++++++++++++++++++- 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/docs/reference/mapping/types/core-types.asciidoc b/docs/reference/mapping/types/core-types.asciidoc index 51dbd044854..e650ded89ca 100644 --- a/docs/reference/mapping/types/core-types.asciidoc +++ b/docs/reference/mapping/types/core-types.asciidoc @@ -378,6 +378,9 @@ defaults to `true` or to the parent `object` type setting. |`ignore_malformed` |Ignored a malformed number. Defaults to `false`. +|`numeric_resolution` |The unit to use when passed in a numeric values. Possible +values include `seconds` and `milliseconds` (default). + |======================================================================= [float] diff --git a/src/main/java/org/elasticsearch/index/mapper/core/DateFieldMapper.java b/src/main/java/org/elasticsearch/index/mapper/core/DateFieldMapper.java index 58744bced0d..a518369ea3e 100644 --- a/src/main/java/org/elasticsearch/index/mapper/core/DateFieldMapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/core/DateFieldMapper.java @@ -488,13 +488,14 @@ public class DateFieldMapper extends NumberFieldMapper { } if (value != null) { + final long timestamp = timeUnit.toMillis(value); if (fieldType.indexOptions() != IndexOptions.NONE || fieldType.stored()) { - CustomLongNumericField field = new CustomLongNumericField(this, value, fieldType); + CustomLongNumericField field = new CustomLongNumericField(this, timestamp, fieldType); field.setBoost(boost); fields.add(field); } if (hasDocValues()) { - addDocValue(context, fields, value); + addDocValue(context, fields, timestamp); } } } @@ -553,8 +554,7 @@ public class DateFieldMapper extends NumberFieldMapper { return dateTimeFormatter.parser().parseMillis(value); } catch (RuntimeException e) { try { - long time = Long.parseLong(value); - return timeUnit.toMillis(time); + return Long.parseLong(value); } catch (NumberFormatException e1) { throw new MapperParsingException("failed to parse date field [" + value + "], tried both date format [" + dateTimeFormatter.format() + "], and timestamp number with locale [" + dateTimeFormatter.locale() + "]", e); } diff --git a/src/test/java/org/elasticsearch/index/mapper/date/SimpleDateMappingTests.java b/src/test/java/org/elasticsearch/index/mapper/date/SimpleDateMappingTests.java index 5d168f3969c..0db26ab1dff 100644 --- a/src/test/java/org/elasticsearch/index/mapper/date/SimpleDateMappingTests.java +++ b/src/test/java/org/elasticsearch/index/mapper/date/SimpleDateMappingTests.java @@ -22,6 +22,7 @@ package org.elasticsearch.index.mapper.date; import org.apache.lucene.analysis.NumericTokenStream.NumericTermAttribute; import org.apache.lucene.analysis.TokenStream; import org.apache.lucene.index.DocValuesType; +import org.apache.lucene.index.IndexableField; import org.apache.lucene.search.Filter; import org.apache.lucene.search.NumericRangeFilter; import org.elasticsearch.ElasticsearchIllegalArgumentException; @@ -38,10 +39,12 @@ import org.elasticsearch.index.mapper.DocumentMapperParser; import org.elasticsearch.index.mapper.FieldMapper; import org.elasticsearch.index.mapper.MapperParsingException; import org.elasticsearch.index.mapper.ParseContext; +import org.elasticsearch.index.mapper.ParseContext.Document; import org.elasticsearch.index.mapper.ParsedDocument; import org.elasticsearch.index.mapper.core.DateFieldMapper; import org.elasticsearch.index.mapper.core.LongFieldMapper; import org.elasticsearch.index.mapper.core.StringFieldMapper; +import org.elasticsearch.search.aggregations.support.ValuesSource.Numeric; import org.elasticsearch.search.internal.SearchContext; import org.elasticsearch.test.ElasticsearchSingleNodeTest; import org.elasticsearch.test.TestSearchContext; @@ -55,8 +58,8 @@ import java.util.List; import java.util.Locale; import java.util.Map; -import static org.elasticsearch.index.mapper.string.SimpleStringMappingTests.docValuesType; import static org.elasticsearch.common.settings.ImmutableSettings.settingsBuilder; +import static org.elasticsearch.index.mapper.string.SimpleStringMappingTests.docValuesType; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.instanceOf; @@ -396,4 +399,37 @@ public class SimpleDateMappingTests extends ElasticsearchSingleNodeTest { assertThat(dateFieldMapperMap.get("field"), is(instanceOf(Map.class))); return (Map) dateFieldMapperMap.get("field"); } + + private static long getDateAsMillis(Document doc, String field) { + for (IndexableField f : doc.getFields(field)) { + if (f.numericValue() != null) { + return f.numericValue().longValue(); + } + } + throw new AssertionError("missing"); + } + + public void testNumericResolution() throws Exception { + String mapping = XContentFactory.jsonBuilder().startObject().startObject("type") + .startObject("properties").startObject("date_field").field("type", "date").field("format", "date_time").field("numeric_resolution", "seconds").endObject().endObject() + .endObject().endObject().string(); + + DocumentMapper defaultMapper = mapper(mapping); + + // provided as an int + ParsedDocument doc = defaultMapper.parse("type", "1", XContentFactory.jsonBuilder() + .startObject() + .field("date_field", 42) + .endObject() + .bytes()); + assertThat(getDateAsMillis(doc.rootDoc(), "date_field"), equalTo(42000L)); + + // provided as a string + doc = defaultMapper.parse("type", "2", XContentFactory.jsonBuilder() + .startObject() + .field("date_field", "43") + .endObject() + .bytes()); + assertThat(getDateAsMillis(doc.rootDoc(), "date_field"), equalTo(43000L)); + } } \ No newline at end of file From 5fd9aee16e001a942d8e23f622ebc5e3881e052b Mon Sep 17 00:00:00 2001 From: Leonardo Menezes Date: Fri, 3 Apr 2015 12:35:09 +0200 Subject: [PATCH 02/24] Cluster state REST api: routing_nodes as an independent metric option Cluster state api returns both routing_table and routing_nodes sections whenever routing_table is requested. That is pretty much the same info, just grouped differently. This commit allows to differentiate between the two. Yet, routing_table still returns both for bw comp reasons. Closes #10352 Closes #10412 --- rest-api-spec/api/cluster.state.json | 2 +- rest-api-spec/test/cluster.state/20_filtering.yaml | 12 ++++++++++++ .../java/org/elasticsearch/cluster/ClusterState.java | 6 +++--- .../admin/cluster/state/RestClusterStateAction.java | 3 ++- 4 files changed, 18 insertions(+), 5 deletions(-) diff --git a/rest-api-spec/api/cluster.state.json b/rest-api-spec/api/cluster.state.json index 9eb2a1f62b8..0f4bf6f21ad 100644 --- a/rest-api-spec/api/cluster.state.json +++ b/rest-api-spec/api/cluster.state.json @@ -16,7 +16,7 @@ }, "metric" : { "type" : "list", - "options" : ["_all", "blocks", "metadata", "nodes", "routing_table", "master_node", "version"], + "options" : ["_all", "blocks", "metadata", "nodes", "routing_table", "routing_nodes", "master_node", "version"], "description" : "Limit the information returned to the specified metrics" } }, diff --git a/rest-api-spec/test/cluster.state/20_filtering.yaml b/rest-api-spec/test/cluster.state/20_filtering.yaml index ee8df1a5331..fa8523a7bb1 100644 --- a/rest-api-spec/test/cluster.state/20_filtering.yaml +++ b/rest-api-spec/test/cluster.state/20_filtering.yaml @@ -83,6 +83,18 @@ setup: - is_true: routing_table - is_true: routing_nodes +--- +"Filtering the cluster state by routing nodes only should work": + - do: + cluster.state: + metric: [ routing_nodes ] + + - is_false: blocks + - is_false: nodes + - is_false: metadata + - is_false: routing_table + - is_true: routing_nodes + --- "Filtering the cluster state by indices should work in routing table and metadata": - do: diff --git a/src/main/java/org/elasticsearch/cluster/ClusterState.java b/src/main/java/org/elasticsearch/cluster/ClusterState.java index c7c3b3da3c3..5452b36142b 100644 --- a/src/main/java/org/elasticsearch/cluster/ClusterState.java +++ b/src/main/java/org/elasticsearch/cluster/ClusterState.java @@ -23,7 +23,6 @@ import com.carrotsearch.hppc.cursors.ObjectCursor; import com.carrotsearch.hppc.cursors.ObjectObjectCursor; import com.google.common.collect.ImmutableSet; import org.elasticsearch.ElasticsearchIllegalArgumentException; -import org.elasticsearch.Version; import org.elasticsearch.cluster.block.ClusterBlock; import org.elasticsearch.cluster.block.ClusterBlocks; import org.elasticsearch.cluster.metadata.IndexMetaData; @@ -33,7 +32,6 @@ import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.cluster.routing.*; - import org.elasticsearch.cluster.routing.allocation.RoutingAllocation; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.Strings; @@ -261,6 +259,7 @@ public class ClusterState implements ToXContent { NODES("nodes"), METADATA("metadata"), ROUTING_TABLE("routing_table"), + ROUTING_NODES("routing_nodes"), CUSTOMS("customs"); private static Map valueToEnum; @@ -465,7 +464,8 @@ public class ClusterState implements ToXContent { } // routing nodes - if (metrics.contains(Metric.ROUTING_TABLE)) { + // gets printed out even if only routing_table was requested for bw comp reasons + if (metrics.contains(Metric.ROUTING_TABLE) || metrics.contains(Metric.ROUTING_NODES)) { builder.startObject("routing_nodes"); builder.startArray("unassigned"); for (ShardRouting shardRouting : readOnlyRoutingNodes().unassigned()) { diff --git a/src/main/java/org/elasticsearch/rest/action/admin/cluster/state/RestClusterStateAction.java b/src/main/java/org/elasticsearch/rest/action/admin/cluster/state/RestClusterStateAction.java index 4826bb9cff0..bac21dd13e4 100644 --- a/src/main/java/org/elasticsearch/rest/action/admin/cluster/state/RestClusterStateAction.java +++ b/src/main/java/org/elasticsearch/rest/action/admin/cluster/state/RestClusterStateAction.java @@ -72,7 +72,8 @@ public class RestClusterStateAction extends BaseRestHandler { EnumSet metrics = ClusterState.Metric.parseString(request.param("metric"), true); // do not ask for what we do not need. clusterStateRequest.nodes(metrics.contains(ClusterState.Metric.NODES) || metrics.contains(ClusterState.Metric.MASTER_NODE)); - clusterStateRequest.routingTable(metrics.contains(ClusterState.Metric.ROUTING_TABLE)); + //there is no distinction in Java api between routing_table and routing_nodes, it's the same info set over the wire, one single flag to ask for it + clusterStateRequest.routingTable(metrics.contains(ClusterState.Metric.ROUTING_TABLE) || metrics.contains(ClusterState.Metric.ROUTING_NODES)); clusterStateRequest.metaData(metrics.contains(ClusterState.Metric.METADATA)); clusterStateRequest.blocks(metrics.contains(ClusterState.Metric.BLOCKS)); } From 174d141512ff344a0dbef7319063d667abdcf074 Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Wed, 8 Apr 2015 15:39:02 +0200 Subject: [PATCH 03/24] Test: increase recovery concurrency in OldIndexBackwardsCompatibilityTests.testOldIndices Also added some logging to help pin point slowness --- .../common/xcontent/XContentHelper.java | 3 +++ .../OldIndexBackwardsCompatibilityTests.java | 12 ++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/elasticsearch/common/xcontent/XContentHelper.java b/src/main/java/org/elasticsearch/common/xcontent/XContentHelper.java index aa6469904bd..7c3221a4d62 100644 --- a/src/main/java/org/elasticsearch/common/xcontent/XContentHelper.java +++ b/src/main/java/org/elasticsearch/common/xcontent/XContentHelper.java @@ -208,6 +208,9 @@ public class XContentHelper { if (params.paramAsBoolean("pretty", true)) { builder.prettyPrint(); } + if (params.paramAsBoolean("human", true)) { + builder.humanReadable(true); + } builder.startObject(); toXContent.toXContent(builder, params); builder.endObject(); diff --git a/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java b/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java index 62a53b86159..1b564ecdd4b 100644 --- a/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java +++ b/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java @@ -31,11 +31,14 @@ import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.common.io.FileSystemUtils; import org.elasticsearch.common.settings.ImmutableSettings; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.env.NodeEnvironment; import org.elasticsearch.index.engine.EngineConfig; import org.elasticsearch.index.merge.policy.MergePolicyModule; import org.elasticsearch.index.query.FilterBuilders; import org.elasticsearch.index.query.QueryBuilders; +import org.elasticsearch.indices.recovery.RecoverySettings; import org.elasticsearch.node.Node; import org.elasticsearch.rest.action.admin.indices.upgrade.UpgradeTest; import org.elasticsearch.search.SearchHit; @@ -46,6 +49,7 @@ import org.elasticsearch.search.sort.SortOrder; import org.elasticsearch.test.ElasticsearchIntegrationTest; import org.elasticsearch.test.hamcrest.ElasticsearchAssertions; import org.elasticsearch.test.index.merge.NoMergePolicyProvider; +import org.elasticsearch.test.junit.annotations.TestLogging; import org.elasticsearch.test.rest.client.http.HttpRequestBuilder; import org.hamcrest.Matchers; import org.junit.AfterClass; @@ -97,6 +101,7 @@ public class OldIndexBackwardsCompatibilityTests extends ElasticsearchIntegratio return ImmutableSettings.builder() .put(Node.HTTP_ENABLED, true) // for _upgrade .put(MergePolicyModule.MERGE_POLICY_TYPE_KEY, NoMergePolicyProvider.class) // disable merging so no segments will be upgraded + .put(RecoverySettings.INDICES_RECOVERY_CONCURRENT_SMALL_FILE_STREAMS, 30) // increase recovery speed for small files .build(); } @@ -190,7 +195,7 @@ public class OldIndexBackwardsCompatibilityTests extends ElasticsearchIntegratio } } - @LuceneTestCase.AwaitsFix(bugUrl = "times out often , see : https://github.com/elastic/elasticsearch/issues/10434") + @TestLogging("indices.recovery:TRACE") public void testOldIndexes() throws Exception { setupCluster(); @@ -296,11 +301,14 @@ public class OldIndexBackwardsCompatibilityTests extends ElasticsearchIntegratio void assertNewReplicasWork(String indexName) throws Exception { final int numReplicas = randomIntBetween(1, 2); - logger.debug("Creating [{}] replicas for index [{}]", numReplicas, indexName); + final long startTime = System.currentTimeMillis(); + logger.debug("--> creating [{}] replicas for index [{}]", numReplicas, indexName); assertAcked(client().admin().indices().prepareUpdateSettings(indexName).setSettings(ImmutableSettings.builder() .put("number_of_replicas", numReplicas) ).execute().actionGet()); ensureGreen(indexName); + logger.debug("--> index [{}] is green, took [{}]", indexName, TimeValue.timeValueMillis(System.currentTimeMillis() - startTime)); + logger.debug("--> recovery status:\n{}", XContentHelper.toString(client().admin().indices().prepareRecoveries(indexName).get())); // TODO: do something with the replicas! query? index? } From acabf2d55af88fbb1f4f9af9bdbd44fd836dd2cc Mon Sep 17 00:00:00 2001 From: javanna Date: Wed, 8 Apr 2015 15:46:49 +0200 Subject: [PATCH 04/24] Cluster state REST api: print routing_nodes out only when requested through specific flag For bacwards compatibility reasons routing_nodes were previously printed out when routing_table was requested, together with the actual routing_table. Now they are printed out only when requests through `routing_nodes` flag. Relates to #10412 Closes #10486 --- docs/reference/migration/migrate_2_0.asciidoc | 6 ++++++ rest-api-spec/test/cluster.state/20_filtering.yaml | 2 +- src/main/java/org/elasticsearch/cluster/ClusterState.java | 3 +-- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/docs/reference/migration/migrate_2_0.asciidoc b/docs/reference/migration/migrate_2_0.asciidoc index 72272d20697..dee483e6e77 100644 --- a/docs/reference/migration/migrate_2_0.asciidoc +++ b/docs/reference/migration/migrate_2_0.asciidoc @@ -371,3 +371,9 @@ over AJAX instead: http.cors.enabled: true http.cors.allow-origin: /https?:\/\/localhost(:[0-9]+)?/ --------------- + +=== Cluster state REST api + +The cluster state api doesn't return the `routing_nodes` section anymore when +`routing_table` is requested. The newly introduced `routing_nodes` flag can +be used separately to control whether `routing_nodes` should be returned. diff --git a/rest-api-spec/test/cluster.state/20_filtering.yaml b/rest-api-spec/test/cluster.state/20_filtering.yaml index fa8523a7bb1..3b1f83eecf3 100644 --- a/rest-api-spec/test/cluster.state/20_filtering.yaml +++ b/rest-api-spec/test/cluster.state/20_filtering.yaml @@ -81,7 +81,7 @@ setup: - is_false: nodes - is_false: metadata - is_true: routing_table - - is_true: routing_nodes + - is_false: routing_nodes --- "Filtering the cluster state by routing nodes only should work": diff --git a/src/main/java/org/elasticsearch/cluster/ClusterState.java b/src/main/java/org/elasticsearch/cluster/ClusterState.java index 5452b36142b..ef4d67740dc 100644 --- a/src/main/java/org/elasticsearch/cluster/ClusterState.java +++ b/src/main/java/org/elasticsearch/cluster/ClusterState.java @@ -464,8 +464,7 @@ public class ClusterState implements ToXContent { } // routing nodes - // gets printed out even if only routing_table was requested for bw comp reasons - if (metrics.contains(Metric.ROUTING_TABLE) || metrics.contains(Metric.ROUTING_NODES)) { + if (metrics.contains(Metric.ROUTING_NODES)) { builder.startObject("routing_nodes"); builder.startArray("unassigned"); for (ShardRouting shardRouting : readOnlyRoutingNodes().unassigned()) { From 6df978e76e26ebfa44c598599736e234b669c8c0 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 8 Apr 2015 08:44:39 -0700 Subject: [PATCH 05/24] Test: increasing replicas timeout to avoid slowness on virtualized hardware (aka jenkins) --- .../bwcompat/OldIndexBackwardsCompatibilityTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java b/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java index 1b564ecdd4b..261cbb1b2a2 100644 --- a/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java +++ b/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java @@ -306,7 +306,7 @@ public class OldIndexBackwardsCompatibilityTests extends ElasticsearchIntegratio assertAcked(client().admin().indices().prepareUpdateSettings(indexName).setSettings(ImmutableSettings.builder() .put("number_of_replicas", numReplicas) ).execute().actionGet()); - ensureGreen(indexName); + ensureGreen(TimeValue.timeValueMinutes(1), indexName); logger.debug("--> index [{}] is green, took [{}]", indexName, TimeValue.timeValueMillis(System.currentTimeMillis() - startTime)); logger.debug("--> recovery status:\n{}", XContentHelper.toString(client().admin().indices().prepareRecoveries(indexName).get())); From f687377e2f11c4be05eaca1f84f824a33d33237a Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Wed, 8 Apr 2015 11:47:09 +0200 Subject: [PATCH 06/24] [ENGINE] sync translog before closing engine If the translog is buffered we must make sure everything is synced to disk before we rollback the writer otherwise we open a window for potential dataloss due to stupid errors preventing the translog from being closed. --- .../java/org/elasticsearch/index/engine/InternalEngine.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/main/java/org/elasticsearch/index/engine/InternalEngine.java b/src/main/java/org/elasticsearch/index/engine/InternalEngine.java index 9b2c6e66c06..1fa47e326df 100644 --- a/src/main/java/org/elasticsearch/index/engine/InternalEngine.java +++ b/src/main/java/org/elasticsearch/index/engine/InternalEngine.java @@ -902,6 +902,11 @@ public class InternalEngine extends Engine { if (isClosed.compareAndSet(false, true)) { assert rwl.isWriteLockedByCurrentThread() || failEngineLock.isHeldByCurrentThread() : "Either the write lock must be held or the engine must be currently be failing itself"; try { + try { + translog.sync(); + } catch (IOException ex) { + logger.warn("failed to sync translog"); + } this.versionMap.clear(); logger.trace("close searcherManager"); try { From 178f650552c4b45335fdfd4f43317ec2f74c2cf3 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 8 Apr 2015 09:00:38 -0700 Subject: [PATCH 07/24] Tests: Fix static bwc replicas at 1 --- .../bwcompat/OldIndexBackwardsCompatibilityTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java b/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java index 261cbb1b2a2..bd498439fe0 100644 --- a/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java +++ b/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java @@ -106,7 +106,7 @@ public class OldIndexBackwardsCompatibilityTests extends ElasticsearchIntegratio } void setupCluster() throws Exception { - ListenableFuture> replicas = internalCluster().startNodesAsync(2); // for replicas + ListenableFuture> replicas = internalCluster().startNodesAsync(1); // for replicas Path dataDir = newTempDirPath(LifecycleScope.SUITE); ImmutableSettings.Builder nodeSettings = ImmutableSettings.builder() @@ -300,7 +300,7 @@ public class OldIndexBackwardsCompatibilityTests extends ElasticsearchIntegratio } void assertNewReplicasWork(String indexName) throws Exception { - final int numReplicas = randomIntBetween(1, 2); + final int numReplicas = 1; final long startTime = System.currentTimeMillis(); logger.debug("--> creating [{}] replicas for index [{}]", numReplicas, indexName); assertAcked(client().admin().indices().prepareUpdateSettings(indexName).setSettings(ImmutableSettings.builder() From ab395c1267afce7b83894c3b3f101d93e51cb6d1 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 8 Apr 2015 13:57:22 -0700 Subject: [PATCH 08/24] Tests: allow up to 30s to delete indexes in old static tests When deleting an index, the tests run checkindex, which requires a flush. This can be very slow on virtualized hardware.. --- .../bwcompat/OldIndexBackwardsCompatibilityTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java b/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java index bd498439fe0..3b647cc4f68 100644 --- a/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java +++ b/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java @@ -158,7 +158,7 @@ public class OldIndexBackwardsCompatibilityTests extends ElasticsearchIntegratio } void unloadIndex(String indexName) throws Exception { - ElasticsearchAssertions.assertAcked(client().admin().indices().prepareDelete(indexName).get()); + ElasticsearchAssertions.assertAcked(client().admin().indices().prepareDelete(indexName).setTimeout(TimeValue.timeValueSeconds(30)).get()); ElasticsearchAssertions.assertAllFilesClosed(); } From 0cd1848e88868640414f3109778d9e02a172a03d Mon Sep 17 00:00:00 2001 From: Spencer Alger Date: Wed, 8 Apr 2015 15:33:57 -0700 Subject: [PATCH 09/24] Indexed scripts/templates: fix yaml test indentation --- rest-api-spec/test/template/10_basic.yaml | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/rest-api-spec/test/template/10_basic.yaml b/rest-api-spec/test/template/10_basic.yaml index d95cd37ba41..838a21d5a56 100644 --- a/rest-api-spec/test/template/10_basic.yaml +++ b/rest-api-spec/test/template/10_basic.yaml @@ -8,8 +8,8 @@ - match: { _id: "1" } - do: - get_template: - id: 1 + get_template: + id: 1 - match: { found: true } - match: { lang: mustache } - match: { _id: "1" } @@ -17,9 +17,9 @@ - match: { template: /.*query\S\S\S\Smatch_all.*/ } - do: - catch: missing - get_template: - id: 2 + catch: missing + get_template: + id: 2 - match: { found: false } - match: { lang: mustache } - match: { _id: "2" } @@ -27,17 +27,17 @@ - is_false: template - do: - delete_template: - id: "1" + delete_template: + id: "1" - match: { found: true } - match: { _index: ".scripts" } - match: { _id: "1" } - match: { _version: 2} - do: - catch: missing - delete_template: - id: "non_existing" + catch: missing + delete_template: + id: "non_existing" - match: { found: false } - match: { _index: ".scripts" } - match: { _id: "non_existing" } From b343d688c541d77c9cdce72e098bac6cc94d0081 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 8 Apr 2015 21:24:10 -0700 Subject: [PATCH 10/24] Tests: Increase delete timeout for static bwc tests to 1 minute --- .../bwcompat/OldIndexBackwardsCompatibilityTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java b/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java index 3b647cc4f68..e106029a675 100644 --- a/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java +++ b/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java @@ -158,7 +158,7 @@ public class OldIndexBackwardsCompatibilityTests extends ElasticsearchIntegratio } void unloadIndex(String indexName) throws Exception { - ElasticsearchAssertions.assertAcked(client().admin().indices().prepareDelete(indexName).setTimeout(TimeValue.timeValueSeconds(30)).get()); + ElasticsearchAssertions.assertAcked(client().admin().indices().prepareDelete(indexName).setTimeout(TimeValue.timeValueMinutes(1)).get()); ElasticsearchAssertions.assertAllFilesClosed(); } From b52d24a031c936b50e1461f95e2b91bc603273f3 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Thu, 9 Apr 2015 06:41:02 +0200 Subject: [PATCH 11/24] [TEST] increase logging for pulling snapshot from InternalEngine --- .../java/org/elasticsearch/index/engine/InternalEngine.java | 3 +++ .../bwcompat/OldIndexBackwardsCompatibilityTests.java | 2 +- .../org/elasticsearch/test/store/MockFSDirectoryService.java | 5 +++-- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/elasticsearch/index/engine/InternalEngine.java b/src/main/java/org/elasticsearch/index/engine/InternalEngine.java index 1fa47e326df..b914dfe6d06 100644 --- a/src/main/java/org/elasticsearch/index/engine/InternalEngine.java +++ b/src/main/java/org/elasticsearch/index/engine/InternalEngine.java @@ -770,9 +770,12 @@ public class InternalEngine extends Engine { public SnapshotIndexCommit snapshotIndex() throws EngineException { // we have to flush outside of the readlock otherwise we might have a problem upgrading // the to a write lock when we fail the engine in this operation + logger.trace("start flush for snapshot"); flush(false, false, true); + logger.trace("finish flush for snapshot"); try (ReleasableLock lock = readLock.acquire()) { ensureOpen(); + logger.trace("pulling snapshot"); return deletionPolicy.snapshot(); } catch (IOException e) { throw new SnapshotFailedEngineException(shardId, e); diff --git a/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java b/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java index e106029a675..43f10f2606c 100644 --- a/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java +++ b/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java @@ -195,7 +195,7 @@ public class OldIndexBackwardsCompatibilityTests extends ElasticsearchIntegratio } } - @TestLogging("indices.recovery:TRACE") + @TestLogging("test.engine:TRACE,index.engine:TRACE,test.engine.lucene:INFO,index.engine.lucene:INFO") public void testOldIndexes() throws Exception { setupCluster(); diff --git a/src/test/java/org/elasticsearch/test/store/MockFSDirectoryService.java b/src/test/java/org/elasticsearch/test/store/MockFSDirectoryService.java index 618e23bc569..89393fa24ac 100644 --- a/src/test/java/org/elasticsearch/test/store/MockFSDirectoryService.java +++ b/src/test/java/org/elasticsearch/test/store/MockFSDirectoryService.java @@ -86,13 +86,14 @@ public class MockFSDirectoryService extends FsDirectoryService { public void beforeIndexShardClosed(ShardId sid, @Nullable IndexShard indexShard, @IndexSettings Settings indexSettings) { if (indexShard != null && shardId.equals(sid)) { - logger.info("Shard state before potentially flushing is {}", indexShard.state()); + logger.info("{} shard state before potentially flushing is {}", indexShard.shardId(), indexShard.state()); if (validCheckIndexStates.contains(indexShard.state()) && IndexMetaData.isOnSharedFilesystem(indexSettings) == false) { // When the the internal engine closes we do a rollback, which removes uncommitted segments // By doing a commit flush we perform a Lucene commit, but don't clear the translog, // so that even in tests where don't flush we can check the integrity of the Lucene index + logger.info("{} flushing in order to run checkindex", indexShard.shardId()); Releasables.close(indexShard.engine().snapshotIndex()); // Keep translog for tests that rely on replaying it - logger.info("flush finished in beforeIndexShardClosed"); + logger.info("{} flush finished in beforeIndexShardClosed", indexShard.shardId()); canRun = true; } } From c821b8d3b4eff620509eedb2d15c828243e6ac52 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 8 Apr 2015 23:29:37 -0700 Subject: [PATCH 12/24] Tests: remove static bwc delete index timeout, so that the slow delete failure can repro with additional logging --- .../bwcompat/OldIndexBackwardsCompatibilityTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java b/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java index 43f10f2606c..58aa5e2e4ca 100644 --- a/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java +++ b/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java @@ -158,7 +158,7 @@ public class OldIndexBackwardsCompatibilityTests extends ElasticsearchIntegratio } void unloadIndex(String indexName) throws Exception { - ElasticsearchAssertions.assertAcked(client().admin().indices().prepareDelete(indexName).setTimeout(TimeValue.timeValueMinutes(1)).get()); + ElasticsearchAssertions.assertAcked(client().admin().indices().prepareDelete(indexName).get()); ElasticsearchAssertions.assertAllFilesClosed(); } From a5bfe332e5eec4fbffae1cc867556a0677c3a7a7 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Thu, 9 Apr 2015 00:25:05 -0700 Subject: [PATCH 13/24] Tests: increase logging for lucene commit during flush in InternalEngine --- .../java/org/elasticsearch/index/engine/InternalEngine.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/org/elasticsearch/index/engine/InternalEngine.java b/src/main/java/org/elasticsearch/index/engine/InternalEngine.java index b914dfe6d06..2c4d31edd4c 100644 --- a/src/main/java/org/elasticsearch/index/engine/InternalEngine.java +++ b/src/main/java/org/elasticsearch/index/engine/InternalEngine.java @@ -621,7 +621,9 @@ public class InternalEngine extends Engine { long translogId = translogIdGenerator.incrementAndGet(); translog.newTransientTranslog(translogId); indexWriter.setCommitData(Collections.singletonMap(Translog.TRANSLOG_ID_KEY, Long.toString(translogId))); + logger.trace("starting commit for flush"); commitIndexWriter(indexWriter); + logger.trace("finished commit for flush"); // we need to refresh in order to clear older version values refresh("version_table_flush"); // we need to move transient to current only after we refresh From e60f61d0a9e928bcd1ea6ee965e79f5003f76a95 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Tue, 7 Apr 2015 11:41:53 +0200 Subject: [PATCH 14/24] Fix maven-resources-plugin warnings Commit 168238dab6f5cb081c1e919c0136c13a3c837b72 declared multiple maven-resources-plugin usages instead of declaring multiple executions for the same plugin... resulting to Maven warnings. Closes #10433 --- pom.xml | 194 +++++++++++++++++++++++++++----------------------------- 1 file changed, 92 insertions(+), 102 deletions(-) diff --git a/pom.xml b/pom.xml index 0059a4e9ee4..7ca27190064 100644 --- a/pom.xml +++ b/pom.xml @@ -893,6 +893,98 @@ + + + copy-resources-deb + prepare-package + + copy-resources + + + ${project.build.directory}/generated-packaging/deb/ + + src/packaging/common/packaging.properties + src/packaging/deb/packaging.properties + + + + src/packaging/common/ + true + + **/* + + + packaging.properties + + + + src/packaging/deb/ + true + + **/* + + + packaging.properties + + + + ${project.basedir} + true + + bin/elasticsearch + bin/elasticsearch.in.sh + bin/plugin + + + + + + + + copy-resources-rpm + prepare-package + + copy-resources + + + ${project.build.directory}/generated-packaging/rpm/ + + src/packaging/common/packaging.properties + src/packaging/rpm/packaging.properties + + + + src/packaging/common/ + true + + **/* + + + packaging.properties + + + + src/packaging/rpm/ + true + + **/* + + + packaging.properties + + + + ${project.basedir} + true + + bin/elasticsearch + bin/elasticsearch.in.sh + bin/plugin + + + + + @@ -1101,57 +1193,6 @@ - - - maven-resources-plugin - - - copy-resources-deb - prepare-package - - copy-resources - - - ${project.build.directory}/generated-packaging/deb/ - - src/packaging/common/packaging.properties - src/packaging/deb/packaging.properties - - - - src/packaging/common/ - true - - **/* - - - packaging.properties - - - - src/packaging/deb/ - true - - **/* - - - packaging.properties - - - - ${project.basedir} - true - - bin/elasticsearch - bin/elasticsearch.in.sh - bin/plugin - - - - - - - @@ -1364,57 +1405,6 @@ - - - maven-resources-plugin - - - copy-resources-rpm - prepare-package - - copy-resources - - - ${project.build.directory}/generated-packaging/rpm/ - - src/packaging/common/packaging.properties - src/packaging/rpm/packaging.properties - - - - src/packaging/common/ - true - - **/* - - - packaging.properties - - - - src/packaging/rpm/ - true - - **/* - - - packaging.properties - - - - ${project.basedir} - true - - bin/elasticsearch - bin/elasticsearch.in.sh - bin/plugin - - - - - - - de.thetaphi From a243b3f9244bcc0cdfba53351652a6c6a80ca0f0 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Thu, 9 Apr 2015 06:57:42 +0200 Subject: [PATCH 15/24] [LOGGING] Use static logger name in Engine.java To ensure subclasses like MockInternalEngine which is in a different package (test.engine) are logging under the same logger name this commit moves to a static logger class to determin the logger name. This way all subclasses of engine will log under `index.engine` which also plays nicely with `@TestLogging` where log messages sometimes disappeared since they were enabled for the `index.engine` package but not for `test.engine` --- src/main/java/org/elasticsearch/index/engine/Engine.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/elasticsearch/index/engine/Engine.java b/src/main/java/org/elasticsearch/index/engine/Engine.java index 513a13b6d43..ca7d10130ad 100644 --- a/src/main/java/org/elasticsearch/index/engine/Engine.java +++ b/src/main/java/org/elasticsearch/index/engine/Engine.java @@ -88,7 +88,8 @@ public abstract class Engine implements Closeable { this.engineConfig = engineConfig; this.shardId = engineConfig.getShardId(); this.store = engineConfig.getStore(); - this.logger = Loggers.getLogger(getClass(), engineConfig.getIndexSettings(), engineConfig.getShardId()); + this.logger = Loggers.getLogger(Engine.class, // we use the engine class directly here to make sure all subclasses have the same logger name + engineConfig.getIndexSettings(), engineConfig.getShardId()); this.failedEngineListener = engineConfig.getFailedEngineListener(); this.deletionPolicy = engineConfig.getDeletionPolicy(); } From 953ae63d2b8d785381804b72592ac040f24e9456 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Thu, 9 Apr 2015 10:47:00 +0200 Subject: [PATCH 16/24] [TEST] beast slow index to fail with trace logging --- .../OldIndexBackwardsCompatibilityTests.java | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java b/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java index 58aa5e2e4ca..cb7a72d8e9d 100644 --- a/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java +++ b/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java @@ -195,12 +195,25 @@ public class OldIndexBackwardsCompatibilityTests extends ElasticsearchIntegratio } } - @TestLogging("test.engine:TRACE,index.engine:TRACE,test.engine.lucene:INFO,index.engine.lucene:INFO") public void testOldIndexes() throws Exception { setupCluster(); Collections.shuffle(indexes, getRandom()); for (String index : indexes) { + if (index.equals("index-0.90.13.zip") == false) { + long startTime = System.currentTimeMillis(); + logger.info("--> Testing old index " + index); + assertOldIndexWorks(index); + logger.info("--> Done testing " + index + ", took " + ((System.currentTimeMillis() - startTime) / 1000.0) + " seconds"); + } + } + } + + @TestLogging("test.engine:TRACE,index.engine:TRACE,test.engine.lucene:TRACE,index.engine.lucene:TRACE") + public void testShitSlowIndex() throws Exception { + setupCluster(); + for (int i = 0; i < 5; i++) { + String index = "index-0.90.13.zip"; long startTime = System.currentTimeMillis(); logger.info("--> Testing old index " + index); assertOldIndexWorks(index); From 9981d69e067cd5df3b960c38951c37cdbc762b0f Mon Sep 17 00:00:00 2001 From: Michael McCandless Date: Thu, 9 Apr 2015 05:00:40 -0400 Subject: [PATCH 17/24] Core: add trace logging for the commitTranslog=false case in InternalEngine.flush --- .../java/org/elasticsearch/index/engine/InternalEngine.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/elasticsearch/index/engine/InternalEngine.java b/src/main/java/org/elasticsearch/index/engine/InternalEngine.java index 2c4d31edd4c..9ae31e24244 100644 --- a/src/main/java/org/elasticsearch/index/engine/InternalEngine.java +++ b/src/main/java/org/elasticsearch/index/engine/InternalEngine.java @@ -621,7 +621,7 @@ public class InternalEngine extends Engine { long translogId = translogIdGenerator.incrementAndGet(); translog.newTransientTranslog(translogId); indexWriter.setCommitData(Collections.singletonMap(Translog.TRANSLOG_ID_KEY, Long.toString(translogId))); - logger.trace("starting commit for flush"); + logger.trace("starting commit for flush; commitTranslog=true"); commitIndexWriter(indexWriter); logger.trace("finished commit for flush"); // we need to refresh in order to clear older version values @@ -650,7 +650,9 @@ public class InternalEngine extends Engine { try { long translogId = translog.currentId(); indexWriter.setCommitData(Collections.singletonMap(Translog.TRANSLOG_ID_KEY, Long.toString(translogId))); + logger.trace("starting commit for flush; commitTranslog=false"); commitIndexWriter(indexWriter); + logger.trace("finished commit for flush"); } catch (Throwable e) { throw new FlushFailedEngineException(shardId, e); } From 17c06f06ba4e86504bd3b24a47198bc30536572c Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Thu, 9 Apr 2015 11:56:36 +0200 Subject: [PATCH 18/24] [TEST] issue additional flush --- .../bwcompat/OldIndexBackwardsCompatibilityTests.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java b/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java index cb7a72d8e9d..e5811c9e5b3 100644 --- a/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java +++ b/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java @@ -158,6 +158,7 @@ public class OldIndexBackwardsCompatibilityTests extends ElasticsearchIntegratio } void unloadIndex(String indexName) throws Exception { + client().admin().indices().prepareFlush(indexName).setWaitIfOngoing(true).setForce(true).get(); // temporary for debugging ElasticsearchAssertions.assertAcked(client().admin().indices().prepareDelete(indexName).get()); ElasticsearchAssertions.assertAllFilesClosed(); } From 6b16b32174063a17d9755960479766358bfc3700 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Fri, 3 Apr 2015 11:13:19 +0200 Subject: [PATCH 19/24] Aggregations: Fix multi-level breadth-first aggregations. The refactoring in #9544 introduced a regression that broke multi-level aggregations using breadth-first. This was due to sub-aggregators creating deferred collectors before their parent aggregator and then the parent aggregator trying to collect sub aggregators directly instead of going through the deferred wrapper. This commit fixes the issue but we should try to simplify all the pre/post collection logic that we have. Also `breadth_first` is now automatically ignored if the sub aggregators need scores (just like we ignore `execution_mode` when the value does not make sense like using ordinals on a script). Close #9823 --- .../percolator/QueryCollector.java | 1 + .../search/aggregations/AggregationPhase.java | 25 ++++---- .../search/aggregations/AggregatorBase.java | 8 ++- .../aggregations/AggregatorFactories.java | 13 +--- .../bucket/DeferringBucketCollector.java | 5 +- .../bucket/terms/TermsAggregator.java | 8 ++- ...RandomTests.java => EquivalenceTests.java} | 57 ++++++++++++++++- .../aggregations/bucket/TopHitsTests.java | 64 +++++++++---------- .../bucket/nested/NestedAggregatorTest.java | 1 + 9 files changed, 122 insertions(+), 60 deletions(-) rename src/test/java/org/elasticsearch/search/aggregations/{RandomTests.java => EquivalenceTests.java} (87%) diff --git a/src/main/java/org/elasticsearch/percolator/QueryCollector.java b/src/main/java/org/elasticsearch/percolator/QueryCollector.java index 2653c2de1b7..f289e188167 100644 --- a/src/main/java/org/elasticsearch/percolator/QueryCollector.java +++ b/src/main/java/org/elasticsearch/percolator/QueryCollector.java @@ -92,6 +92,7 @@ abstract class QueryCollector extends SimpleCollector { context.aggregations().aggregators(aggregators); } aggregatorCollector = BucketCollector.wrap(aggregatorCollectors); + aggregatorCollector.preCollection(); } public void postMatch(int doc) throws IOException { diff --git a/src/main/java/org/elasticsearch/search/aggregations/AggregationPhase.java b/src/main/java/org/elasticsearch/search/aggregations/AggregationPhase.java index 9d627310142..387d365c62d 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/AggregationPhase.java +++ b/src/main/java/org/elasticsearch/search/aggregations/AggregationPhase.java @@ -76,18 +76,20 @@ public class AggregationPhase implements SearchPhase { Aggregator[] aggregators; try { aggregators = context.aggregations().factories().createTopLevelAggregators(aggregationContext); + for (int i = 0; i < aggregators.length; i++) { + if (aggregators[i] instanceof GlobalAggregator == false) { + collectors.add(aggregators[i]); + } + } + context.aggregations().aggregators(aggregators); + if (!collectors.isEmpty()) { + final BucketCollector collector = BucketCollector.wrap(collectors); + collector.preCollection(); + context.searcher().queryCollectors().put(AggregationPhase.class, collector); + } } catch (IOException e) { throw new AggregationInitializationException("Could not initialize aggregators", e); } - for (int i = 0; i < aggregators.length; i++) { - if (aggregators[i] instanceof GlobalAggregator == false) { - collectors.add(aggregators[i]); - } - } - context.aggregations().aggregators(aggregators); - if (!collectors.isEmpty()) { - context.searcher().queryCollectors().put(AggregationPhase.class, (BucketCollector.wrap(collectors))); - } } } @@ -113,14 +115,15 @@ public class AggregationPhase implements SearchPhase { // optimize the global collector based execution if (!globals.isEmpty()) { - BucketCollector collector = BucketCollector.wrap(globals); + BucketCollector globalsCollector = BucketCollector.wrap(globals); Query query = new ConstantScoreQuery(Queries.MATCH_ALL_FILTER); Filter searchFilter = context.searchFilter(context.types()); if (searchFilter != null) { query = new FilteredQuery(query, searchFilter); } try { - context.searcher().search(query, collector); + globalsCollector.preCollection(); + context.searcher().search(query, globalsCollector); } catch (Exception e) { throw new QueryPhaseExecutionException(context, "Failed to execute global aggregators", e); } diff --git a/src/main/java/org/elasticsearch/search/aggregations/AggregatorBase.java b/src/main/java/org/elasticsearch/search/aggregations/AggregatorBase.java index 874ad71cfa8..4d83603a088 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/AggregatorBase.java +++ b/src/main/java/org/elasticsearch/search/aggregations/AggregatorBase.java @@ -99,7 +99,12 @@ public abstract class AggregatorBase extends Aggregator { */ @Override public boolean needsScores() { - return collectableSubAggregators.needsScores(); + for (Aggregator agg : subAggregators) { + if (agg.needsScores()) { + return true; + } + } + return false; } public Map metaData() { @@ -145,6 +150,7 @@ public abstract class AggregatorBase extends Aggregator { } collectableSubAggregators = BucketCollector.wrap(collectors); doPreCollection(); + collectableSubAggregators.preCollection(); } /** diff --git a/src/main/java/org/elasticsearch/search/aggregations/AggregatorFactories.java b/src/main/java/org/elasticsearch/search/aggregations/AggregatorFactories.java index 10ea7f74c2c..98cc7e39e1a 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/AggregatorFactories.java +++ b/src/main/java/org/elasticsearch/search/aggregations/AggregatorFactories.java @@ -44,15 +44,6 @@ public class AggregatorFactories { this.factories = factories; } - private static Aggregator createAndRegisterContextAware(AggregationContext context, AggregatorFactory factory, Aggregator parent, boolean collectsFromSingleBucket) throws IOException { - final Aggregator aggregator = factory.create(context, parent, collectsFromSingleBucket); - // Once the aggregator is fully constructed perform any initialisation - - // can't do everything in constructors if Aggregator base class needs - // to delegate to subclasses as part of construction. - aggregator.preCollection(); - return aggregator; - } - /** * Create all aggregators so that they can be consumed with multiple buckets. */ @@ -64,7 +55,7 @@ public class AggregatorFactories { // propagate the fact that only bucket 0 will be collected with single-bucket // aggs final boolean collectsFromSingleBucket = false; - aggregators[i] = createAndRegisterContextAware(parent.context(), factories[i], parent, collectsFromSingleBucket); + aggregators[i] = factories[i].create(parent.context(), parent, collectsFromSingleBucket); } return aggregators; } @@ -75,7 +66,7 @@ public class AggregatorFactories { for (int i = 0; i < factories.length; i++) { // top-level aggs only get called with bucket 0 final boolean collectsFromSingleBucket = true; - aggregators[i] = createAndRegisterContextAware(ctx, factories[i], null, collectsFromSingleBucket); + aggregators[i] = factories[i].create(ctx, null, collectsFromSingleBucket); } return aggregators; } diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/DeferringBucketCollector.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/DeferringBucketCollector.java index 9249da7334b..09686e662d5 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/DeferringBucketCollector.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/DeferringBucketCollector.java @@ -73,7 +73,7 @@ public final class DeferringBucketCollector extends BucketCollector { if (collector == null) { throw new ElasticsearchIllegalStateException(); } - return collector.needsScores(); + return false; } /** Set the deferred collectors. */ @@ -138,6 +138,9 @@ public final class DeferringBucketCollector extends BucketCollector { this.selectedBuckets = hash; collector.preCollection(); + if (collector.needsScores()) { + throw new ElasticsearchIllegalStateException("Cannot defer if scores are needed"); + } for (Entry entry : entries) { final LeafBucketCollector leafCollector = collector.getLeafCollector(entry.context); diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregator.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregator.java index ef254bb0594..4cfe549a452 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregator.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregator.java @@ -43,7 +43,7 @@ public abstract class TermsAggregator extends BucketsAggregator { private Explicit shardMinDocCount; private Explicit requiredSize; private Explicit shardSize; - + public BucketCountThresholds(long minDocCount, long shardMinDocCount, int requiredSize, int shardSize) { this.minDocCount = new Explicit<>(minDocCount, false); this.shardMinDocCount = new Explicit<>(shardMinDocCount, false); @@ -157,7 +157,9 @@ public abstract class TermsAggregator extends BucketsAggregator { @Override protected boolean shouldDefer(Aggregator aggregator) { - return (collectMode == SubAggCollectionMode.BREADTH_FIRST) && (!aggsUsedForSorting.contains(aggregator)); + return collectMode == SubAggCollectionMode.BREADTH_FIRST + && aggregator.needsScores() == false + && !aggsUsedForSorting.contains(aggregator); } - + } diff --git a/src/test/java/org/elasticsearch/search/aggregations/RandomTests.java b/src/test/java/org/elasticsearch/search/aggregations/EquivalenceTests.java similarity index 87% rename from src/test/java/org/elasticsearch/search/aggregations/RandomTests.java rename to src/test/java/org/elasticsearch/search/aggregations/EquivalenceTests.java index c87f00bdadc..5079e6730dd 100644 --- a/src/test/java/org/elasticsearch/search/aggregations/RandomTests.java +++ b/src/test/java/org/elasticsearch/search/aggregations/EquivalenceTests.java @@ -42,7 +42,10 @@ import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregatorFactory import org.elasticsearch.search.aggregations.metrics.sum.Sum; import org.elasticsearch.test.ElasticsearchIntegrationTest; +import java.io.IOException; +import java.util.ArrayList; import java.util.HashMap; +import java.util.Iterator; import java.util.List; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; @@ -67,7 +70,7 @@ import static org.hamcrest.core.IsNull.notNullValue; * the growth of dynamic arrays is tested. */ @Slow -public class RandomTests extends ElasticsearchIntegrationTest { +public class EquivalenceTests extends ElasticsearchIntegrationTest { // Make sure that unordered, reversed, disjoint and/or overlapping ranges are supported // Duel with filters @@ -380,4 +383,56 @@ public class RandomTests extends ElasticsearchIntegrationTest { assertEquals(value >= 6 ? value : 0, sum.getValue(), 0d); } + private void assertEquals(Terms t1, Terms t2) { + List t1Buckets = t1.getBuckets(); + List t2Buckets = t1.getBuckets(); + assertEquals(t1Buckets.size(), t2Buckets.size()); + for (Iterator it1 = t1Buckets.iterator(), it2 = t2Buckets.iterator(); it1.hasNext(); ) { + final Terms.Bucket b1 = it1.next(); + final Terms.Bucket b2 = it2.next(); + assertEquals(b1.getDocCount(), b2.getDocCount()); + assertEquals(b1.getKey(), b2.getKey()); + } + } + + public void testDuelDepthBreadthFirst() throws Exception { + createIndex("idx"); + final int numDocs = randomIntBetween(100, 500); + List reqs = new ArrayList<>(); + for (int i = 0; i < numDocs; ++i) { + final int v1 = randomInt(1 << randomInt(7)); + final int v2 = randomInt(1 << randomInt(7)); + final int v3 = randomInt(1 << randomInt(7)); + reqs.add(client().prepareIndex("idx", "type").setSource("f1", v1, "f2", v2, "f3", v3)); + } + indexRandom(true, reqs); + + final SearchResponse r1 = client().prepareSearch("idx").addAggregation( + terms("f1").field("f1").collectMode(SubAggCollectionMode.DEPTH_FIRST) + .subAggregation(terms("f2").field("f2").collectMode(SubAggCollectionMode.DEPTH_FIRST) + .subAggregation(terms("f3").field("f3").collectMode(SubAggCollectionMode.DEPTH_FIRST)))).get(); + assertSearchResponse(r1); + final SearchResponse r2 = client().prepareSearch("idx").addAggregation( + terms("f1").field("f1").collectMode(SubAggCollectionMode.BREADTH_FIRST) + .subAggregation(terms("f2").field("f2").collectMode(SubAggCollectionMode.BREADTH_FIRST) + .subAggregation(terms("f3").field("f3").collectMode(SubAggCollectionMode.BREADTH_FIRST)))).get(); + assertSearchResponse(r2); + + final Terms t1 = r1.getAggregations().get("f1"); + final Terms t2 = r2.getAggregations().get("f1"); + assertEquals(t1, t2); + for (Terms.Bucket b1 : t1.getBuckets()) { + final Terms.Bucket b2 = t2.getBucketByKey(b1.getKeyAsString()); + final Terms sub1 = b1.getAggregations().get("f2"); + final Terms sub2 = b2.getAggregations().get("f2"); + assertEquals(sub1, sub2); + for (Terms.Bucket subB1 : sub1.getBuckets()) { + final Terms.Bucket subB2 = sub2.getBucketByKey(subB1.getKeyAsString()); + final Terms subSub1 = subB1.getAggregations().get("f3"); + final Terms subSub2 = subB2.getAggregations().get("f3"); + assertEquals(subSub1, subSub2); + } + } + } + } diff --git a/src/test/java/org/elasticsearch/search/aggregations/bucket/TopHitsTests.java b/src/test/java/org/elasticsearch/search/aggregations/bucket/TopHitsTests.java index 3e51fe9ccd2..0b8b2ba810f 100644 --- a/src/test/java/org/elasticsearch/search/aggregations/bucket/TopHitsTests.java +++ b/src/test/java/org/elasticsearch/search/aggregations/bucket/TopHitsTests.java @@ -46,7 +46,6 @@ import org.junit.Test; import java.util.ArrayList; import java.util.Iterator; import java.util.List; -import java.util.Map; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.common.xcontent.XContentFactory.smileBuilder; @@ -271,6 +270,38 @@ public class TopHitsTests extends ElasticsearchIntegrationTest { } } + @Test + public void testBreadthFirst() throws Exception { + // breadth_first will be ignored since we need scores + SearchResponse response = client().prepareSearch("idx").setTypes("type") + .addAggregation(terms("terms") + .executionHint(randomExecutionHint()) + .collectMode(SubAggCollectionMode.BREADTH_FIRST) + .field(TERMS_AGGS_FIELD) + .subAggregation(topHits("hits").setSize(3)) + ).get(); + + assertSearchResponse(response); + + Terms terms = response.getAggregations().get("terms"); + assertThat(terms, notNullValue()); + assertThat(terms.getName(), equalTo("terms")); + assertThat(terms.getBuckets().size(), equalTo(5)); + + for (int i = 0; i < 5; i++) { + Terms.Bucket bucket = terms.getBucketByKey("val" + i); + assertThat(bucket, notNullValue()); + assertThat(key(bucket), equalTo("val" + i)); + assertThat(bucket.getDocCount(), equalTo(10l)); + TopHits topHits = bucket.getAggregations().get("hits"); + SearchHits hits = topHits.getHits(); + assertThat(hits.totalHits(), equalTo(10l)); + assertThat(hits.getHits().length, equalTo(3)); + + assertThat(hits.getAt(0).sourceAsMap().size(), equalTo(4)); + } + } + @Test public void testBasics_getProperty() throws Exception { SearchResponse searchResponse = client().prepareSearch("idx").setQuery(matchAllQuery()) @@ -531,37 +562,6 @@ public class TopHitsTests extends ElasticsearchIntegrationTest { assertThat(e.getMessage(), containsString("Aggregator [top_tags_hits] of type [top_hits] cannot accept sub-aggregations")); } } - - @Test - public void testFailDeferredOnlyWhenScorerIsUsed() throws Exception { - // No track_scores or score based sort defined in top_hits agg, so don't fail: - SearchResponse response = client().prepareSearch("idx") - .setTypes("type") - .addAggregation( - terms("terms").executionHint(randomExecutionHint()).field(TERMS_AGGS_FIELD) - .collectMode(SubAggCollectionMode.BREADTH_FIRST) - .subAggregation(topHits("hits").addSort(SortBuilders.fieldSort(SORT_FIELD).order(SortOrder.DESC)))) - .get(); - assertSearchResponse(response); - - // Score based, so fail with deferred aggs: - try { - client().prepareSearch("idx") - .setTypes("type") - .addAggregation( - terms("terms").executionHint(randomExecutionHint()).field(TERMS_AGGS_FIELD) - .collectMode(SubAggCollectionMode.BREADTH_FIRST) - .subAggregation(topHits("hits"))) - .get(); - fail(); - } catch (Exception e) { - // It is considered a parse failure if the search request asks for top_hits - // under an aggregation with collect_mode set to breadth_first as this would - // require the buffering of scores alongside each document ID and that is a - // a RAM cost we are not willing to pay - assertThat(e.getMessage(), containsString("ElasticsearchIllegalStateException")); - } - } @Test public void testEmptyIndex() throws Exception { diff --git a/src/test/java/org/elasticsearch/search/aggregations/bucket/nested/NestedAggregatorTest.java b/src/test/java/org/elasticsearch/search/aggregations/bucket/nested/NestedAggregatorTest.java index 7cdff38d7c8..cea6efd8747 100644 --- a/src/test/java/org/elasticsearch/search/aggregations/bucket/nested/NestedAggregatorTest.java +++ b/src/test/java/org/elasticsearch/search/aggregations/bucket/nested/NestedAggregatorTest.java @@ -125,6 +125,7 @@ public class NestedAggregatorTest extends ElasticsearchSingleNodeLuceneTestCase searchContext.aggregations(new SearchContextAggregations(factories)); Aggregator[] aggs = factories.createTopLevelAggregators(context); BucketCollector collector = BucketCollector.wrap(Arrays.asList(aggs)); + collector.preCollection(); // A regular search always exclude nested docs, so we use NonNestedDocsFilter.INSTANCE here (otherwise MatchAllDocsQuery would be sufficient) // We exclude root doc with uid type#2, this will trigger the bug if we don't reset the root doc when we process a new segment, because // root doc type#3 and root doc type#1 have the same segment docid From aecd9ac51527fa6ac57788c23527ec4e0dc32636 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Fri, 3 Apr 2015 18:04:22 +0200 Subject: [PATCH 20/24] Aggregations: Speed up include/exclude in terms aggregations with regexps. Today we check every regular expression eagerly against every possible term. This can be very slow if you have lots of unique terms, and even the bottleneck if your query is selective. This commit switches to Lucene regular expressions instead of Java (not exactly the same syntax yet most existing regular expressions should keep working) and uses the same logic as RegExpQuery to intersect the regular expression with the terms dictionary. I wrote a quick benchmark (in the PR) to make sure it made things faster and the same request that took 750ms on master now takes 74ms with this change. Close #7526 --- docs/reference/migration/migrate_2_0.asciidoc | 3 + .../bucket/terms-aggregation.asciidoc | 37 +- ...balOrdinalsSignificantTermsAggregator.java | 4 +- .../SignificantStringTermsAggregator.java | 2 +- .../SignificantTermsAggregatorFactory.java | 9 +- .../significant/SignificantTermsParser.java | 2 +- .../GlobalOrdinalsStringTermsAggregator.java | 6 +- .../bucket/terms/StringTermsAggregator.java | 4 +- .../bucket/terms/TermsAggregatorFactory.java | 9 +- .../bucket/terms/TermsBuilder.java | 53 +-- .../bucket/terms/TermsParser.java | 2 +- .../bucket/terms/support/IncludeExclude.java | 325 ++++++++++-------- ...ludeExcludeAggregationSearchBenchmark.java | 130 +++++++ .../aggregations/bucket/StringTermsTests.java | 80 ----- 14 files changed, 337 insertions(+), 329 deletions(-) create mode 100644 src/test/java/org/elasticsearch/benchmark/search/aggregations/IncludeExcludeAggregationSearchBenchmark.java diff --git a/docs/reference/migration/migrate_2_0.asciidoc b/docs/reference/migration/migrate_2_0.asciidoc index dee483e6e77..f875363bb8b 100644 --- a/docs/reference/migration/migrate_2_0.asciidoc +++ b/docs/reference/migration/migrate_2_0.asciidoc @@ -139,6 +139,9 @@ equivalent to the former `pre_zone` option. Setting `time_zone` to a value like being applied in the specified time zone but In addition to this, also the `pre_zone_adjust_large_interval` is removed because we now always return dates and bucket keys in UTC. +`include`/`exclude` filtering on the `terms` aggregation now uses the same syntax as regexp queries instead of the Java syntax. While simple +regexps should still work, more complex ones might need some rewriting. Also, the `flags` parameter is not supported anymore. + === Terms filter lookup caching The terms filter lookup mechanism does not support the `cache` option anymore diff --git a/docs/reference/search/aggregations/bucket/terms-aggregation.asciidoc b/docs/reference/search/aggregations/bucket/terms-aggregation.asciidoc index 38f6630628a..6b93e926cdd 100644 --- a/docs/reference/search/aggregations/bucket/terms-aggregation.asciidoc +++ b/docs/reference/search/aggregations/bucket/terms-aggregation.asciidoc @@ -482,42 +482,7 @@ with `water_` (so the tag `water_sports` will no be aggregated). The `include` r values are "allowed" to be aggregated, while the `exclude` determines the values that should not be aggregated. When both are defined, the `exclude` has precedence, meaning, the `include` is evaluated first and only then the `exclude`. -The regular expression are based on the Java(TM) http://docs.oracle.com/javase/7/docs/api/java/util/regex/Pattern.html[Pattern], -and as such, they it is also possible to pass in flags that will determine how the compiled regular expression will work: - -[source,js] --------------------------------------------------- -{ - "aggs" : { - "tags" : { - "terms" : { - "field" : "tags", - "include" : { - "pattern" : ".*sport.*", - "flags" : "CANON_EQ|CASE_INSENSITIVE" <1> - }, - "exclude" : { - "pattern" : "water_.*", - "flags" : "CANON_EQ|CASE_INSENSITIVE" - } - } - } - } -} --------------------------------------------------- - -<1> the flags are concatenated using the `|` character as a separator - -The possible flags that can be used are: -http://docs.oracle.com/javase/7/docs/api/java/util/regex/Pattern.html#CANON_EQ[`CANON_EQ`], -http://docs.oracle.com/javase/7/docs/api/java/util/regex/Pattern.html#CASE_INSENSITIVE[`CASE_INSENSITIVE`], -http://docs.oracle.com/javase/7/docs/api/java/util/regex/Pattern.html#COMMENTS[`COMMENTS`], -http://docs.oracle.com/javase/7/docs/api/java/util/regex/Pattern.html#DOTALL[`DOTALL`], -http://docs.oracle.com/javase/7/docs/api/java/util/regex/Pattern.html#LITERAL[`LITERAL`], -http://docs.oracle.com/javase/7/docs/api/java/util/regex/Pattern.html#MULTILINE[`MULTILINE`], -http://docs.oracle.com/javase/7/docs/api/java/util/regex/Pattern.html#UNICODE_CASE[`UNICODE_CASE`], -http://docs.oracle.com/javase/7/docs/api/java/util/regex/Pattern.html#UNICODE_CHARACTER_CLASS[`UNICODE_CHARACTER_CLASS`] and -http://docs.oracle.com/javase/7/docs/api/java/util/regex/Pattern.html#UNIX_LINES[`UNIX_LINES`] +The syntax is the same as <>. For matching based on exact values the `include` and `exclude` parameters can simply take an array of strings that represent the terms as they are found in the index: diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/GlobalOrdinalsSignificantTermsAggregator.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/GlobalOrdinalsSignificantTermsAggregator.java index fc8e5e4b7f7..49a7e56eefb 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/GlobalOrdinalsSignificantTermsAggregator.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/GlobalOrdinalsSignificantTermsAggregator.java @@ -48,7 +48,7 @@ public class GlobalOrdinalsSignificantTermsAggregator extends GlobalOrdinalsStri public GlobalOrdinalsSignificantTermsAggregator(String name, AggregatorFactories factories, ValuesSource.Bytes.WithOrdinals.FieldData valuesSource, BucketCountThresholds bucketCountThresholds, - IncludeExclude includeExclude, AggregationContext aggregationContext, Aggregator parent, + IncludeExclude.OrdinalsFilter includeExclude, AggregationContext aggregationContext, Aggregator parent, SignificantTermsAggregatorFactory termsAggFactory, Map metaData) throws IOException { super(name, factories, valuesSource, null, bucketCountThresholds, includeExclude, aggregationContext, parent, SubAggCollectionMode.DEPTH_FIRST, false, metaData); @@ -145,7 +145,7 @@ public class GlobalOrdinalsSignificantTermsAggregator extends GlobalOrdinalsStri private final LongHash bucketOrds; - public WithHash(String name, AggregatorFactories factories, ValuesSource.Bytes.WithOrdinals.FieldData valuesSource, BucketCountThresholds bucketCountThresholds, IncludeExclude includeExclude, AggregationContext aggregationContext, Aggregator parent, SignificantTermsAggregatorFactory termsAggFactory, Map metaData) throws IOException { + public WithHash(String name, AggregatorFactories factories, ValuesSource.Bytes.WithOrdinals.FieldData valuesSource, BucketCountThresholds bucketCountThresholds, IncludeExclude.OrdinalsFilter includeExclude, AggregationContext aggregationContext, Aggregator parent, SignificantTermsAggregatorFactory termsAggFactory, Map metaData) throws IOException { super(name, factories, valuesSource, bucketCountThresholds, includeExclude, aggregationContext, parent, termsAggFactory, metaData); bucketOrds = new LongHash(1, aggregationContext.bigArrays()); } diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantStringTermsAggregator.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantStringTermsAggregator.java index fb65fd7d6f8..532a71efae7 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantStringTermsAggregator.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantStringTermsAggregator.java @@ -47,7 +47,7 @@ public class SignificantStringTermsAggregator extends StringTermsAggregator { public SignificantStringTermsAggregator(String name, AggregatorFactories factories, ValuesSource valuesSource, BucketCountThresholds bucketCountThresholds, - IncludeExclude includeExclude, AggregationContext aggregationContext, Aggregator parent, + IncludeExclude.StringFilter includeExclude, AggregationContext aggregationContext, Aggregator parent, SignificantTermsAggregatorFactory termsAggFactory, Map metaData) throws IOException { super(name, factories, valuesSource, null, bucketCountThresholds, includeExclude, aggregationContext, parent, SubAggCollectionMode.DEPTH_FIRST, false, metaData); diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorFactory.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorFactory.java index e6058471eb9..ef837cfab82 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorFactory.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorFactory.java @@ -65,7 +65,8 @@ public class SignificantTermsAggregatorFactory extends ValuesSourceAggregatorFac Aggregator create(String name, AggregatorFactories factories, ValuesSource valuesSource, TermsAggregator.BucketCountThresholds bucketCountThresholds, IncludeExclude includeExclude, AggregationContext aggregationContext, Aggregator parent, SignificantTermsAggregatorFactory termsAggregatorFactory, Map metaData) throws IOException { - return new SignificantStringTermsAggregator(name, factories, valuesSource, bucketCountThresholds, includeExclude, aggregationContext, parent, termsAggregatorFactory, metaData); + final IncludeExclude.StringFilter filter = includeExclude == null ? null : includeExclude.convertToStringFilter(); + return new SignificantStringTermsAggregator(name, factories, valuesSource, bucketCountThresholds, filter, aggregationContext, parent, termsAggregatorFactory, metaData); } }, @@ -77,7 +78,8 @@ public class SignificantTermsAggregatorFactory extends ValuesSourceAggregatorFac AggregationContext aggregationContext, Aggregator parent, SignificantTermsAggregatorFactory termsAggregatorFactory, Map metaData) throws IOException { ValuesSource.Bytes.WithOrdinals valueSourceWithOrdinals = (ValuesSource.Bytes.WithOrdinals) valuesSource; IndexSearcher indexSearcher = aggregationContext.searchContext().searcher(); - return new GlobalOrdinalsSignificantTermsAggregator(name, factories, (ValuesSource.Bytes.WithOrdinals.FieldData) valuesSource, bucketCountThresholds, includeExclude, aggregationContext, parent, termsAggregatorFactory, metaData); + final IncludeExclude.OrdinalsFilter filter = includeExclude == null ? null : includeExclude.convertToOrdinalsFilter(); + return new GlobalOrdinalsSignificantTermsAggregator(name, factories, (ValuesSource.Bytes.WithOrdinals.FieldData) valuesSource, bucketCountThresholds, filter, aggregationContext, parent, termsAggregatorFactory, metaData); } }, @@ -87,7 +89,8 @@ public class SignificantTermsAggregatorFactory extends ValuesSourceAggregatorFac Aggregator create(String name, AggregatorFactories factories, ValuesSource valuesSource, TermsAggregator.BucketCountThresholds bucketCountThresholds, IncludeExclude includeExclude, AggregationContext aggregationContext, Aggregator parent, SignificantTermsAggregatorFactory termsAggregatorFactory, Map metaData) throws IOException { - return new GlobalOrdinalsSignificantTermsAggregator.WithHash(name, factories, (ValuesSource.Bytes.WithOrdinals.FieldData) valuesSource, bucketCountThresholds, includeExclude, aggregationContext, parent, termsAggregatorFactory, metaData); + final IncludeExclude.OrdinalsFilter filter = includeExclude == null ? null : includeExclude.convertToOrdinalsFilter(); + return new GlobalOrdinalsSignificantTermsAggregator.WithHash(name, factories, (ValuesSource.Bytes.WithOrdinals.FieldData) valuesSource, bucketCountThresholds, filter, aggregationContext, parent, termsAggregatorFactory, metaData); } }; diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsParser.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsParser.java index 7492b698018..28e0fb5a812 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsParser.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsParser.java @@ -57,7 +57,7 @@ public class SignificantTermsParser implements Aggregator.Parser { .scriptable(false) .formattable(true) .build(); - IncludeExclude.Parser incExcParser = new IncludeExclude.Parser(aggregationName, SignificantStringTerms.TYPE, context); + IncludeExclude.Parser incExcParser = new IncludeExclude.Parser(); aggParser.parse(aggregationName, parser, context, vsParser, incExcParser); TermsAggregator.BucketCountThresholds bucketCountThresholds = aggParser.getBucketCountThresholds(); diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java index 70904ac2ca3..767f2d50926 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java @@ -57,7 +57,7 @@ import java.util.Map; public class GlobalOrdinalsStringTermsAggregator extends AbstractStringTermsAggregator { protected final ValuesSource.Bytes.WithOrdinals.FieldData valuesSource; - protected final IncludeExclude includeExclude; + protected final IncludeExclude.OrdinalsFilter includeExclude; // TODO: cache the acceptedglobalValues per aggregation definition. // We can't cache this yet in ValuesSource, since ValuesSource is reused per field for aggs during the execution. @@ -71,7 +71,7 @@ public class GlobalOrdinalsStringTermsAggregator extends AbstractStringTermsAggr public GlobalOrdinalsStringTermsAggregator(String name, AggregatorFactories factories, ValuesSource.Bytes.WithOrdinals.FieldData valuesSource, Terms.Order order, BucketCountThresholds bucketCountThresholds, - IncludeExclude includeExclude, AggregationContext aggregationContext, Aggregator parent, SubAggCollectionMode collectionMode, boolean showTermDocCountError, Map metaData) throws IOException { + IncludeExclude.OrdinalsFilter includeExclude, AggregationContext aggregationContext, Aggregator parent, SubAggCollectionMode collectionMode, boolean showTermDocCountError, Map metaData) throws IOException { super(name, factories, aggregationContext, parent, order, bucketCountThresholds, collectionMode, showTermDocCountError, metaData); this.valuesSource = valuesSource; this.includeExclude = includeExclude; @@ -260,7 +260,7 @@ public class GlobalOrdinalsStringTermsAggregator extends AbstractStringTermsAggr private final LongHash bucketOrds; public WithHash(String name, AggregatorFactories factories, ValuesSource.Bytes.WithOrdinals.FieldData valuesSource, - Terms.Order order, BucketCountThresholds bucketCountThresholds, IncludeExclude includeExclude, AggregationContext aggregationContext, + Terms.Order order, BucketCountThresholds bucketCountThresholds, IncludeExclude.OrdinalsFilter includeExclude, AggregationContext aggregationContext, Aggregator parent, SubAggCollectionMode collectionMode, boolean showTermDocCountError, Map metaData) throws IOException { super(name, factories, valuesSource, order, bucketCountThresholds, includeExclude, aggregationContext, parent, collectionMode, showTermDocCountError, metaData); bucketOrds = new LongHash(1, aggregationContext.bigArrays()); diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/StringTermsAggregator.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/StringTermsAggregator.java index 9d731a25529..d625e3b9954 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/StringTermsAggregator.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/StringTermsAggregator.java @@ -45,11 +45,11 @@ public class StringTermsAggregator extends AbstractStringTermsAggregator { private final ValuesSource valuesSource; protected final BytesRefHash bucketOrds; - private final IncludeExclude includeExclude; + private final IncludeExclude.StringFilter includeExclude; public StringTermsAggregator(String name, AggregatorFactories factories, ValuesSource valuesSource, Terms.Order order, BucketCountThresholds bucketCountThresholds, - IncludeExclude includeExclude, AggregationContext aggregationContext, Aggregator parent, SubAggCollectionMode collectionMode, boolean showTermDocCountError, Map metaData) throws IOException { + IncludeExclude.StringFilter includeExclude, AggregationContext aggregationContext, Aggregator parent, SubAggCollectionMode collectionMode, boolean showTermDocCountError, Map metaData) throws IOException { super(name, factories, aggregationContext, parent, order, bucketCountThresholds, collectionMode, showTermDocCountError, metaData); this.valuesSource = valuesSource; diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorFactory.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorFactory.java index 6fbbd306411..3fa99d2b7fd 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorFactory.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorFactory.java @@ -50,7 +50,8 @@ public class TermsAggregatorFactory extends ValuesSourceAggregatorFactory metaData) throws IOException { - return new StringTermsAggregator(name, factories, valuesSource, order, bucketCountThresholds, includeExclude, aggregationContext, parent, subAggCollectMode, showTermDocCountError, metaData); + final IncludeExclude.StringFilter filter = includeExclude == null ? null : includeExclude.convertToStringFilter(); + return new StringTermsAggregator(name, factories, valuesSource, order, bucketCountThresholds, filter, aggregationContext, parent, subAggCollectMode, showTermDocCountError, metaData); } @Override @@ -65,7 +66,8 @@ public class TermsAggregatorFactory extends ValuesSourceAggregatorFactory metaData) throws IOException { - return new GlobalOrdinalsStringTermsAggregator(name, factories, (ValuesSource.Bytes.WithOrdinals.FieldData) valuesSource, order, bucketCountThresholds, includeExclude, aggregationContext, parent, subAggCollectMode, showTermDocCountError, metaData); + final IncludeExclude.OrdinalsFilter filter = includeExclude == null ? null : includeExclude.convertToOrdinalsFilter(); + return new GlobalOrdinalsStringTermsAggregator(name, factories, (ValuesSource.Bytes.WithOrdinals.FieldData) valuesSource, order, bucketCountThresholds, filter, aggregationContext, parent, subAggCollectMode, showTermDocCountError, metaData); } @Override @@ -80,7 +82,8 @@ public class TermsAggregatorFactory extends ValuesSourceAggregatorFactory metaData) throws IOException { - return new GlobalOrdinalsStringTermsAggregator.WithHash(name, factories, (ValuesSource.Bytes.WithOrdinals.FieldData) valuesSource, order, bucketCountThresholds, includeExclude, aggregationContext, parent, subAggCollectMode, showTermDocCountError, metaData); + final IncludeExclude.OrdinalsFilter filter = includeExclude == null ? null : includeExclude.convertToOrdinalsFilter(); + return new GlobalOrdinalsStringTermsAggregator.WithHash(name, factories, (ValuesSource.Bytes.WithOrdinals.FieldData) valuesSource, order, bucketCountThresholds, filter, aggregationContext, parent, subAggCollectMode, showTermDocCountError, metaData); } @Override diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsBuilder.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsBuilder.java index 2243654cf3a..ea2e40587d4 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsBuilder.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsBuilder.java @@ -19,6 +19,7 @@ package org.elasticsearch.search.aggregations.bucket.terms; +import org.apache.lucene.util.automaton.RegExp; import org.elasticsearch.ElasticsearchIllegalArgumentException; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.search.aggregations.Aggregator.SubAggCollectionMode; @@ -37,9 +38,7 @@ public class TermsBuilder extends ValuesSourceAggregationBuilder { private Terms.ValueType valueType; private Terms.Order order; private String includePattern; - private int includeFlags; private String excludePattern; - private int excludeFlags; private String executionHint; private SubAggCollectionMode collectionMode; private Boolean showTermDocCountError; @@ -88,26 +87,15 @@ public class TermsBuilder extends ValuesSourceAggregationBuilder { /** * Define a regular expression that will determine what terms should be aggregated. The regular expression is based - * on the {@link java.util.regex.Pattern} class. + * on the {@link RegExp} class. * - * @see #include(String, int) + * @see {@link RegExp#RegExp(String)} */ public TermsBuilder include(String regex) { - return include(regex, 0); - } - - /** - * Define a regular expression that will determine what terms should be aggregated. The regular expression is based - * on the {@link java.util.regex.Pattern} class. - * - * @see java.util.regex.Pattern#compile(String, int) - */ - public TermsBuilder include(String regex, int flags) { if (includeTerms != null) { throw new ElasticsearchIllegalArgumentException("exclude clause must be an array of strings or a regex, not both"); } this.includePattern = regex; - this.includeFlags = flags; return this; } @@ -160,29 +148,18 @@ public class TermsBuilder extends ValuesSourceAggregationBuilder { } return termsAsString; } - - /** - * Define a regular expression that will filter out terms that should be excluded from the aggregation. The regular - * expression is based on the {@link java.util.regex.Pattern} class. - * - * @see #exclude(String, int) - */ - public TermsBuilder exclude(String regex) { - return exclude(regex, 0); - } /** * Define a regular expression that will filter out terms that should be excluded from the aggregation. The regular - * expression is based on the {@link java.util.regex.Pattern} class. + * expression is based on the {@link RegExp} class. * - * @see java.util.regex.Pattern#compile(String, int) + * @see {@link RegExp#RegExp(String)} */ - public TermsBuilder exclude(String regex, int flags) { + public TermsBuilder exclude(String regex) { if (excludeTerms != null) { throw new ElasticsearchIllegalArgumentException("exclude clause must be an array of exact values or a regex, not both"); } this.excludePattern = regex; - this.excludeFlags = flags; return this; } @@ -287,27 +264,13 @@ public class TermsBuilder extends ValuesSourceAggregationBuilder { builder.array("include", includeTerms); } if (includePattern != null) { - if (includeFlags == 0) { - builder.field("include", includePattern); - } else { - builder.startObject("include") - .field("pattern", includePattern) - .field("flags", includeFlags) - .endObject(); - } + builder.field("include", includePattern); } if (excludeTerms != null) { builder.array("exclude", excludeTerms); } if (excludePattern != null) { - if (excludeFlags == 0) { - builder.field("exclude", excludePattern); - } else { - builder.startObject("exclude") - .field("pattern", excludePattern) - .field("flags", excludeFlags) - .endObject(); - } + builder.field("exclude", excludePattern); } return builder; } diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsParser.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsParser.java index 01a5cc7bc2f..478309d1bc0 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsParser.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsParser.java @@ -46,7 +46,7 @@ public class TermsParser implements Aggregator.Parser { public AggregatorFactory parse(String aggregationName, XContentParser parser, SearchContext context) throws IOException { TermsParametersParser aggParser = new TermsParametersParser(); ValuesSourceParser vsParser = ValuesSourceParser.any(aggregationName, StringTerms.TYPE, context).scriptable(true).formattable(true).build(); - IncludeExclude.Parser incExcParser = new IncludeExclude.Parser(aggregationName, StringTerms.TYPE, context); + IncludeExclude.Parser incExcParser = new IncludeExclude.Parser(); aggParser.parse(aggregationName, parser, context, vsParser, incExcParser); List orderElements = aggParser.getOrderElements(); diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/support/IncludeExclude.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/support/IncludeExclude.java index 6eff3fcedfc..e653ad853a9 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/support/IncludeExclude.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/support/IncludeExclude.java @@ -20,22 +20,30 @@ package org.elasticsearch.search.aggregations.bucket.terms.support; import com.carrotsearch.hppc.LongOpenHashSet; import com.carrotsearch.hppc.LongSet; + import org.apache.lucene.index.RandomAccessOrds; +import org.apache.lucene.index.SortedSetDocValues; +import org.apache.lucene.index.Terms; import org.apache.lucene.index.TermsEnum; -import org.apache.lucene.util.*; +import org.apache.lucene.util.BytesRef; +import org.apache.lucene.util.LongBitSet; +import org.apache.lucene.util.NumericUtils; +import org.apache.lucene.util.automaton.Automata; +import org.apache.lucene.util.automaton.Automaton; +import org.apache.lucene.util.automaton.ByteRunAutomaton; +import org.apache.lucene.util.automaton.CompiledAutomaton; +import org.apache.lucene.util.automaton.Operations; +import org.apache.lucene.util.automaton.RegExp; +import org.elasticsearch.ElasticsearchIllegalArgumentException; import org.elasticsearch.ElasticsearchParseException; -import org.elasticsearch.ExceptionsHelper; -import org.elasticsearch.common.regex.Regex; import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.search.aggregations.InternalAggregation; import org.elasticsearch.search.aggregations.support.ValuesSource; -import org.elasticsearch.search.internal.SearchContext; import java.io.IOException; import java.util.HashSet; import java.util.Set; -import java.util.regex.Matcher; -import java.util.regex.Pattern; +import java.util.SortedSet; +import java.util.TreeSet; /** * Defines the include/exclude regular expression filtering for string terms aggregation. In this filtering logic, @@ -43,8 +51,8 @@ import java.util.regex.Pattern; */ public class IncludeExclude { - // The includeValue and excludeValue ByteRefs which are the result of the parsing - // process are converted into a LongFilter when used on numeric fields + // The includeValue and excludeValue ByteRefs which are the result of the parsing + // process are converted into a LongFilter when used on numeric fields // in the index. public static class LongFilter { private LongSet valids; @@ -72,152 +80,145 @@ public class IncludeExclude { } } - private final Matcher include; - private final Matcher exclude; - private final CharsRefBuilder scratch = new CharsRefBuilder(); - private Set includeValues; - private Set excludeValues; - private final boolean hasRegexTest; + // Only used for the 'map' execution mode (ie. scripts) + public static class StringFilter { + + private final ByteRunAutomaton runAutomaton; + + private StringFilter(Automaton automaton) { + this.runAutomaton = new ByteRunAutomaton(automaton); + } + + /** + * Returns whether the given value is accepted based on the {@code include} & {@code exclude} patterns. + */ + public boolean accept(BytesRef value) { + return runAutomaton.run(value.bytes, value.offset, value.length); + } + } + + public static class OrdinalsFilter { + + private final CompiledAutomaton compiled; + + private OrdinalsFilter(Automaton automaton) { + this.compiled = new CompiledAutomaton(automaton); + } + + /** + * Computes which global ordinals are accepted by this IncludeExclude instance. + */ + public LongBitSet acceptedGlobalOrdinals(RandomAccessOrds globalOrdinals, ValuesSource.Bytes.WithOrdinals valueSource) throws IOException { + LongBitSet acceptedGlobalOrdinals = new LongBitSet(globalOrdinals.getValueCount()); + TermsEnum globalTermsEnum; + Terms globalTerms = new DocValuesTerms(globalOrdinals); + // TODO: specialize based on compiled.type: for ALL and prefixes (sinkState >= 0 ) we can avoid i/o and just set bits. + globalTermsEnum = compiled.getTermsEnum(globalTerms); + for (BytesRef term = globalTermsEnum.next(); term != null; term = globalTermsEnum.next()) { + acceptedGlobalOrdinals.set(globalTermsEnum.ord()); + } + return acceptedGlobalOrdinals; + } + + } + + private final RegExp include, exclude; + private final SortedSet includeValues, excludeValues; /** * @param include The regular expression pattern for the terms to be included - * (may only be {@code null} if one of the other arguments is none-null. - * @param includeValues The terms to be included - * (may only be {@code null} if one of the other arguments is none-null. * @param exclude The regular expression pattern for the terms to be excluded - * (may only be {@code null} if one of the other arguments is none-null. - * @param excludeValues The terms to be excluded - * (may only be {@code null} if one of the other arguments is none-null. */ - public IncludeExclude(Pattern include, Pattern exclude, Set includeValues, Set excludeValues) { - assert includeValues != null || include != null || - exclude != null || excludeValues != null : "includes & excludes cannot both be null"; // otherwise IncludeExclude object should be null - this.include = include != null ? include.matcher("") : null; - this.exclude = exclude != null ? exclude.matcher("") : null; - hasRegexTest = include != null || exclude != null; + public IncludeExclude(RegExp include, RegExp exclude) { + if (include == null && exclude == null) { + throw new IllegalArgumentException(); + } + this.include = include; + this.exclude = exclude; + this.includeValues = null; + this.excludeValues = null; + } + + /** + * @param includeValues The terms to be included + * @param excludeValues The terms to be excluded + */ + public IncludeExclude(SortedSet includeValues, SortedSet excludeValues) { + if (includeValues == null && excludeValues == null) { + throw new IllegalArgumentException(); + } + this.include = null; + this.exclude = null; this.includeValues = includeValues; this.excludeValues = excludeValues; } /** - * Returns whether the given value is accepted based on the {@code include} & {@code exclude} patterns. + * Terms adapter around doc values. */ - public boolean accept(BytesRef value) { + private static class DocValuesTerms extends Terms { - if (hasRegexTest) { - // We need to perform UTF8 to UTF16 conversion for use in the regex matching - scratch.copyUTF8Bytes(value); - } - return isIncluded(value, scratch.get()) && !isExcluded(value, scratch.get()); - } - - private boolean isIncluded(BytesRef value, CharsRef utf16Chars) { + private final SortedSetDocValues values; - if ((includeValues == null) && (include == null)) { - // No include criteria to be tested. - return true; + DocValuesTerms(SortedSetDocValues values) { + this.values = values; } - - if (include != null) { - if (include.reset(scratch.get()).matches()) { - return true; - } + + @Override + public TermsEnum iterator(TermsEnum reuse) throws IOException { + return values.termsEnum(); } - if (includeValues != null) { - if (includeValues.contains(value)) { - return true; - } + + @Override + public long size() throws IOException { + return -1; } - // Some include criteria was tested but no match found - return false; - } - - private boolean isExcluded(BytesRef value, CharsRef utf16Chars) { - if (exclude != null) { - if (exclude.reset(scratch.get()).matches()) { - return true; - } + + @Override + public long getSumTotalTermFreq() throws IOException { + return -1; } - if (excludeValues != null) { - if (excludeValues.contains(value)) { - return true; - } + + @Override + public long getSumDocFreq() throws IOException { + return -1; } - // No exclude criteria was tested or no match found - return false; + + @Override + public int getDocCount() throws IOException { + return -1; + } + + @Override + public boolean hasFreqs() { + return false; + } + + @Override + public boolean hasOffsets() { + return false; + } + + @Override + public boolean hasPositions() { + return false; + } + + @Override + public boolean hasPayloads() { + return false; + } + } - /** - * Computes which global ordinals are accepted by this IncludeExclude instance. - */ - public LongBitSet acceptedGlobalOrdinals(RandomAccessOrds globalOrdinals, ValuesSource.Bytes.WithOrdinals valueSource) { - LongBitSet acceptedGlobalOrdinals = new LongBitSet(globalOrdinals.getValueCount()); - // There are 3 ways of populating this bitset: - // 1) Looking up the global ordinals for known "include" terms - // 2) Looking up the global ordinals for known "exclude" terms - // 3) Traversing the term enum for all terms and running past regexes - // Option 3 is known to be very slow in the case of high-cardinality fields and - // should be avoided if possible. - if (includeValues != null) { - // optimize for the case where the set of accepted values is a set - // of known terms, not a regex that would have to be tested against all terms in the index - for (BytesRef includeValue : includeValues) { - // We need to perform UTF8 to UTF16 conversion for use in the regex matching - scratch.copyUTF8Bytes(includeValue); - if (!isExcluded(includeValue, scratch.get())) { - long ord = globalOrdinals.lookupTerm(includeValue); - if (ord >= 0) { - acceptedGlobalOrdinals.set(ord); - } - } - } - } else { - if(hasRegexTest) { - // We have includeVals that are a regex or only regex excludes - we need to do the potentially - // slow option of hitting termsEnum for every term in the index. - TermsEnum globalTermsEnum = globalOrdinals.termsEnum(); - try { - for (BytesRef term = globalTermsEnum.next(); term != null; term = globalTermsEnum.next()) { - if (accept(term)) { - acceptedGlobalOrdinals.set(globalTermsEnum.ord()); - } - } - } catch (IOException e) { - throw ExceptionsHelper.convertToElastic(e); - } - } else { - // we only have a set of known values to exclude - create a bitset with all good values and negate the known bads - acceptedGlobalOrdinals.set(0, acceptedGlobalOrdinals.length()); - for (BytesRef excludeValue : excludeValues) { - long ord = globalOrdinals.lookupTerm(excludeValue); - if (ord >= 0) { - acceptedGlobalOrdinals.clear(ord); - } - } - - } - } - return acceptedGlobalOrdinals; - } + public static class Parser { - private final String aggName; - private final InternalAggregation.Type aggType; - private final SearchContext context; - String include = null; - int includeFlags = 0; // 0 means no flags String exclude = null; - int excludeFlags = 0; // 0 means no flags - Set includeValues; - Set excludeValues; - - public Parser(String aggName, InternalAggregation.Type aggType, SearchContext context) { - this.aggName = aggName; - this.aggType = aggType; - this.context = context; - } + SortedSet includeValues; + SortedSet excludeValues; public boolean token(String currentFieldName, XContentParser.Token token, XContentParser parser) throws IOException { @@ -231,14 +232,14 @@ public class IncludeExclude { } return true; } - + if (token == XContentParser.Token.START_ARRAY) { if ("include".equals(currentFieldName)) { - includeValues = parseArrayToSet(parser); + includeValues = new TreeSet<>(parseArrayToSet(parser)); return true; - } + } if ("exclude".equals(currentFieldName)) { - excludeValues = parseArrayToSet(parser); + excludeValues = new TreeSet<>(parseArrayToSet(parser)); return true; } return false; @@ -252,12 +253,6 @@ public class IncludeExclude { } else if (token == XContentParser.Token.VALUE_STRING) { if ("pattern".equals(currentFieldName)) { include = parser.text(); - } else if ("flags".equals(currentFieldName)) { - includeFlags = Regex.flagsFromString(parser.text()); - } - } else if (token == XContentParser.Token.VALUE_NUMBER) { - if ("flags".equals(currentFieldName)) { - includeFlags = parser.intValue(); } } } @@ -268,12 +263,6 @@ public class IncludeExclude { } else if (token == XContentParser.Token.VALUE_STRING) { if ("pattern".equals(currentFieldName)) { exclude = parser.text(); - } else if ("flags".equals(currentFieldName)) { - excludeFlags = Regex.flagsFromString(parser.text()); - } - } else if (token == XContentParser.Token.VALUE_NUMBER) { - if ("flags".equals(currentFieldName)) { - excludeFlags = parser.intValue(); } } } @@ -298,19 +287,50 @@ public class IncludeExclude { } return set; } - + public IncludeExclude includeExclude() { - if (include == null && exclude == null && includeValues == null && excludeValues == null) { + RegExp includePattern = include != null ? new RegExp(include) : null; + RegExp excludePattern = exclude != null ? new RegExp(exclude) : null; + if (includePattern != null || excludePattern != null) { + if (includeValues != null || excludeValues != null) { + throw new ElasticsearchIllegalArgumentException("Can only use regular expression include/exclude or a set of values, not both"); + } + return new IncludeExclude(includePattern, excludePattern); + } else if (includeValues != null || excludeValues != null) { + return new IncludeExclude(includeValues, excludeValues); + } else { return null; } - Pattern includePattern = include != null ? Pattern.compile(include, includeFlags) : null; - Pattern excludePattern = exclude != null ? Pattern.compile(exclude, excludeFlags) : null; - return new IncludeExclude(includePattern, excludePattern, includeValues, excludeValues); } } public boolean isRegexBased() { - return hasRegexTest; + return include != null || exclude != null; + } + + private Automaton toAutomaton() { + Automaton a = null; + if (include != null) { + a = include.toAutomaton(); + } else if (includeValues != null) { + a = Automata.makeStringUnion(includeValues); + } else { + a = Automata.makeAnyString(); + } + if (exclude != null) { + a = Operations.minus(a, exclude.toAutomaton(), Operations.DEFAULT_MAX_DETERMINIZED_STATES); + } else if (excludeValues != null) { + a = Operations.minus(a, Automata.makeStringUnion(excludeValues), Operations.DEFAULT_MAX_DETERMINIZED_STATES); + } + return a; + } + + public StringFilter convertToStringFilter() { + return new StringFilter(toAutomaton()); + } + + public OrdinalsFilter convertToOrdinalsFilter() { + return new OrdinalsFilter(toAutomaton()); } public LongFilter convertToLongFilter() { @@ -329,6 +349,7 @@ public class IncludeExclude { } return result; } + public LongFilter convertToDoubleFilter() { int numValids = includeValues == null ? 0 : includeValues.size(); int numInvalids = excludeValues == null ? 0 : excludeValues.size(); diff --git a/src/test/java/org/elasticsearch/benchmark/search/aggregations/IncludeExcludeAggregationSearchBenchmark.java b/src/test/java/org/elasticsearch/benchmark/search/aggregations/IncludeExcludeAggregationSearchBenchmark.java new file mode 100644 index 00000000000..7de95e102a5 --- /dev/null +++ b/src/test/java/org/elasticsearch/benchmark/search/aggregations/IncludeExcludeAggregationSearchBenchmark.java @@ -0,0 +1,130 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.benchmark.search.aggregations; + +import org.apache.lucene.util.TestUtil; +import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse; +import org.elasticsearch.action.bulk.BulkRequestBuilder; +import org.elasticsearch.action.bulk.BulkResponse; +import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.client.Client; +import org.elasticsearch.common.StopWatch; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.index.query.QueryBuilders; +import org.elasticsearch.node.Node; + +import java.util.Random; +import java.util.concurrent.TimeUnit; + +import static org.elasticsearch.client.Requests.createIndexRequest; +import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_REPLICAS; +import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS; +import static org.elasticsearch.common.settings.ImmutableSettings.settingsBuilder; +import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; +import static org.elasticsearch.node.NodeBuilder.nodeBuilder; +import static org.elasticsearch.search.aggregations.AggregationBuilders.terms; + +public class IncludeExcludeAggregationSearchBenchmark { + + private static final Random R = new Random(); + private static final String CLUSTER_NAME = IncludeExcludeAggregationSearchBenchmark.class.getSimpleName(); + private static final int NUM_DOCS = 10000000; + private static final int BATCH = 100; + private static final int WARM = 3; + private static final int RUNS = 10; + private static final int ITERS = 3; + + public static void main(String[] args) { + Settings settings = settingsBuilder() + .put("index.refresh_interval", "-1") + .put(SETTING_NUMBER_OF_SHARDS, 1) + .put(SETTING_NUMBER_OF_REPLICAS, 0) + .build(); + + Node[] nodes = new Node[1]; + for (int i = 0; i < nodes.length; i++) { + nodes[i] = nodeBuilder().clusterName(CLUSTER_NAME) + .settings(settingsBuilder().put(settings).put("name", "node" + i)) + .node(); + } + + Node clientNode = nodeBuilder() + .clusterName(CLUSTER_NAME) + .settings(settingsBuilder().put(settings).put("name", "client")).client(true).node(); + + Client client = clientNode.client(); + + try { + client.admin().indices().create(createIndexRequest("index").settings(settings).mapping("type", + jsonBuilder().startObject().startObject("type").startObject("properties") + .startObject("str") + .field("type", "string") + .field("index", "not_analyzed") + .endObject() + .endObject().endObject().endObject())).actionGet(); + + System.out.println("Indexing " + NUM_DOCS + " documents"); + + StopWatch stopWatch = new StopWatch().start(); + for (int i = 0; i < NUM_DOCS; ) { + BulkRequestBuilder request = client.prepareBulk(); + for (int j = 0; j < BATCH && i < NUM_DOCS; ++j) { + request.add(client.prepareIndex("index", "type", Integer.toString(i)).setSource("str", TestUtil.randomSimpleString(R))); + ++i; + } + BulkResponse response = request.execute().actionGet(); + if (response.hasFailures()) { + System.err.println("--> failures..."); + System.err.println(response.buildFailureMessage()); + } + if ((i % 100000) == 0) { + System.out.println("--> Indexed " + i + " took " + stopWatch.stop().lastTaskTime()); + stopWatch.start(); + } + } + + client.admin().indices().prepareRefresh("index").execute().actionGet(); + } catch (Exception e) { + System.out.println("Index already exists, skipping index creation"); + } + + ClusterHealthResponse clusterHealthResponse = client.admin().cluster().prepareHealth().setWaitForGreenStatus().setTimeout("10m").execute().actionGet(); + if (clusterHealthResponse.isTimedOut()) { + System.err.println("--> Timed out waiting for cluster health"); + } + + for (int i = 0; i < WARM + RUNS; ++i) { + if (i >= WARM) { + System.out.println("RUN " + (i - WARM)); + } + long start = System.nanoTime(); + SearchResponse resp = null; + for (int j = 0; j < ITERS; ++j) { + resp = client.prepareSearch("index").setQuery(QueryBuilders.prefixQuery("str", "sf")).setSize(0).addAggregation(terms("t").field("str").include("s.*")).execute().actionGet(); + } + long end = System.nanoTime(); + if (i >= WARM) { + System.out.println(new TimeValue((end - start) / ITERS, TimeUnit.NANOSECONDS)); + } + } + } + +} diff --git a/src/test/java/org/elasticsearch/search/aggregations/bucket/StringTermsTests.java b/src/test/java/org/elasticsearch/search/aggregations/bucket/StringTermsTests.java index 9590222a72b..3ef59e06a90 100644 --- a/src/test/java/org/elasticsearch/search/aggregations/bucket/StringTermsTests.java +++ b/src/test/java/org/elasticsearch/search/aggregations/bucket/StringTermsTests.java @@ -387,86 +387,6 @@ public class StringTermsTests extends AbstractTermsTests { } } - @Test - public void singleValueField_WithRegexFiltering_WithFlags() throws Exception { - - // include without exclude - // we should be left with: val000, val001, val002, val003, val004, val005, val006, val007, val008, val009 - // with case insensitive flag on the include regex - - SearchResponse response = client().prepareSearch("idx").setTypes("high_card_type") - .addAggregation(terms("terms") - .executionHint(randomExecutionHint()) - .field(SINGLE_VALUED_FIELD_NAME) - .collectMode(randomFrom(SubAggCollectionMode.values())).include("VAL00.+", Pattern.CASE_INSENSITIVE)) - .execute().actionGet(); - - assertSearchResponse(response); - - Terms terms = response.getAggregations().get("terms"); - assertThat(terms, notNullValue()); - assertThat(terms.getName(), equalTo("terms")); - assertThat(terms.getBuckets().size(), equalTo(10)); - - for (int i = 0; i < 10; i++) { - Terms.Bucket bucket = terms.getBucketByKey("val00" + i); - assertThat(bucket, notNullValue()); - assertThat(key(bucket), equalTo("val00" + i)); - assertThat(bucket.getDocCount(), equalTo(1l)); - } - - // include and exclude - // we should be left with: val002, val003, val004, val005, val006, val007, val008, val009 - // with multi-flag masking on the exclude regex - - response = client().prepareSearch("idx").setTypes("high_card_type") - .addAggregation(terms("terms") - .executionHint(randomExecutionHint()) - .field(SINGLE_VALUED_FIELD_NAME) - .collectMode(randomFrom(SubAggCollectionMode.values())).include("val00.+").exclude("( val000 | VAL001 )#this is a comment", Pattern.CASE_INSENSITIVE | Pattern.COMMENTS)) - .execute().actionGet(); - - assertSearchResponse(response); - - terms = response.getAggregations().get("terms"); - assertThat(terms, notNullValue()); - assertThat(terms.getName(), equalTo("terms")); - assertThat(terms.getBuckets().size(), equalTo(8)); - - for (int i = 2; i < 10; i++) { - Terms.Bucket bucket = terms.getBucketByKey("val00" + i); - assertThat(bucket, notNullValue()); - assertThat(key(bucket), equalTo("val00" + i)); - assertThat(bucket.getDocCount(), equalTo(1l)); - } - - // exclude without include - // we should be left with: val000, val001, val002, val003, val004, val005, val006, val007, val008, val009 - // with a "no flag" flag - - response = client().prepareSearch("idx").setTypes("high_card_type") - .addAggregation(terms("terms") - .executionHint(randomExecutionHint()) - .field(SINGLE_VALUED_FIELD_NAME) - .collectMode(randomFrom(SubAggCollectionMode.values())).exclude("val0[1-9]+.+", 0)) - .execute().actionGet(); - - assertSearchResponse(response); - - terms = response.getAggregations().get("terms"); - assertThat(terms, notNullValue()); - assertThat(terms.getName(), equalTo("terms")); - assertThat(terms.getBuckets().size(), equalTo(10)); - - for (int i = 0; i < 10; i++) { - Terms.Bucket bucket = terms.getBucketByKey("val00" + i); - assertThat(bucket, notNullValue()); - assertThat(key(bucket), equalTo("val00" + i)); - assertThat(bucket.getDocCount(), equalTo(1l)); - } - } - - @Test public void singleValueField_WithExactTermFiltering() throws Exception { // include without exclude From 88ee7a5dca4c356c8bc366fde48b5e7f661c91d2 Mon Sep 17 00:00:00 2001 From: David Pilato Date: Thu, 9 Apr 2015 14:11:49 +0200 Subject: [PATCH 21/24] Deprecate rivers * In code, we mark `River`, `AbstractRiverComponent`, `RiverComponent` and `RiverName` classes as deprecated * We log that information when a cluster is still using it * We add this information in the plugins list as well --- docs/reference/modules/plugins.asciidoc | 2 ++ .../java/org/elasticsearch/river/AbstractRiverComponent.java | 5 +++-- src/main/java/org/elasticsearch/river/River.java | 2 ++ src/main/java/org/elasticsearch/river/RiverComponent.java | 3 ++- src/main/java/org/elasticsearch/river/RiverName.java | 3 ++- src/main/java/org/elasticsearch/river/RiversService.java | 1 + 6 files changed, 12 insertions(+), 4 deletions(-) diff --git a/docs/reference/modules/plugins.asciidoc b/docs/reference/modules/plugins.asciidoc index 2b226f54e12..25f01a4715e 100644 --- a/docs/reference/modules/plugins.asciidoc +++ b/docs/reference/modules/plugins.asciidoc @@ -217,6 +217,8 @@ You can disable that check using `plugins.check_lucene: false`. [[river]] ==== River Plugins +deprecated[1.5.0,Rivers have been deprecated. See https://www.elastic.co/blog/deprecating_rivers for more details] + .Supported by Elasticsearch * https://github.com/elasticsearch/elasticsearch-river-couchdb[CouchDB River Plugin] * https://github.com/elasticsearch/elasticsearch-river-rabbitmq[RabbitMQ River Plugin] diff --git a/src/main/java/org/elasticsearch/river/AbstractRiverComponent.java b/src/main/java/org/elasticsearch/river/AbstractRiverComponent.java index 804ffacc9cd..b8ce3985d5f 100644 --- a/src/main/java/org/elasticsearch/river/AbstractRiverComponent.java +++ b/src/main/java/org/elasticsearch/river/AbstractRiverComponent.java @@ -23,8 +23,9 @@ import org.elasticsearch.common.logging.ESLogger; import org.elasticsearch.common.logging.Loggers; /** - * + * @deprecated See blog post https://www.elastic.co/blog/deprecating_rivers */ +@Deprecated public class AbstractRiverComponent implements RiverComponent { protected final ESLogger logger; @@ -48,4 +49,4 @@ public class AbstractRiverComponent implements RiverComponent { public String nodeName() { return settings.globalSettings().get("name", ""); } -} \ No newline at end of file +} diff --git a/src/main/java/org/elasticsearch/river/River.java b/src/main/java/org/elasticsearch/river/River.java index 6f9638c5f9b..574b5fd4f0b 100644 --- a/src/main/java/org/elasticsearch/river/River.java +++ b/src/main/java/org/elasticsearch/river/River.java @@ -22,7 +22,9 @@ package org.elasticsearch.river; /** * Allows to import data into elasticsearch via plugin * Gets allocated on a node and eventually automatically re-allocated if needed + * @deprecated See blog post https://www.elastic.co/blog/deprecating_rivers */ +@Deprecated public interface River extends RiverComponent { /** diff --git a/src/main/java/org/elasticsearch/river/RiverComponent.java b/src/main/java/org/elasticsearch/river/RiverComponent.java index f07ac97b767..a8cfb898798 100644 --- a/src/main/java/org/elasticsearch/river/RiverComponent.java +++ b/src/main/java/org/elasticsearch/river/RiverComponent.java @@ -20,8 +20,9 @@ package org.elasticsearch.river; /** - * + * @deprecated See blog post https://www.elastic.co/blog/deprecating_rivers */ +@Deprecated public interface RiverComponent { RiverName riverName(); diff --git a/src/main/java/org/elasticsearch/river/RiverName.java b/src/main/java/org/elasticsearch/river/RiverName.java index e21497231e0..7078574c1b4 100644 --- a/src/main/java/org/elasticsearch/river/RiverName.java +++ b/src/main/java/org/elasticsearch/river/RiverName.java @@ -22,8 +22,9 @@ package org.elasticsearch.river; import java.io.Serializable; /** - * + * @deprecated See blog post https://www.elastic.co/blog/deprecating_rivers */ +@Deprecated public class RiverName implements Serializable { private final String type; diff --git a/src/main/java/org/elasticsearch/river/RiversService.java b/src/main/java/org/elasticsearch/river/RiversService.java index f9a801e120f..0d7863a6468 100644 --- a/src/main/java/org/elasticsearch/river/RiversService.java +++ b/src/main/java/org/elasticsearch/river/RiversService.java @@ -126,6 +126,7 @@ public class RiversService extends AbstractLifecycleComponent { return; } + logger.info("rivers have been deprecated. Read https://www.elastic.co/blog/deprecating_rivers"); logger.debug("creating river [{}][{}]", riverName.type(), riverName.name()); try { From abc7de96ae80cf9dbacb565fb71e79383b3029ef Mon Sep 17 00:00:00 2001 From: Clinton Gormley Date: Thu, 9 Apr 2015 14:50:11 +0200 Subject: [PATCH 22/24] Docs: Updated version annotations in master --- docs/reference/cluster/health.asciidoc | 2 -- docs/reference/index.asciidoc | 2 +- docs/reference/indices/recovery.asciidoc | 2 -- docs/reference/mapping/fields/timestamp-field.asciidoc | 2 -- docs/reference/modules/scripting.asciidoc | 2 +- docs/reference/modules/transport.asciidoc | 1 - .../reference/query-dsl/queries/function-score-query.asciidoc | 2 -- .../aggregations/bucket/significantterms-aggregation.asciidoc | 2 -- .../aggregations/metrics/extendedstats-aggregation.asciidoc | 4 +--- docs/reference/search/search-template.asciidoc | 2 +- docs/reference/setup/configuration.asciidoc | 2 +- 11 files changed, 5 insertions(+), 18 deletions(-) diff --git a/docs/reference/cluster/health.asciidoc b/docs/reference/cluster/health.asciidoc index d11a2385f24..a58a0924fce 100644 --- a/docs/reference/cluster/health.asciidoc +++ b/docs/reference/cluster/health.asciidoc @@ -22,8 +22,6 @@ $ curl -XGET 'http://localhost:9200/_cluster/health?pretty=true' } -------------------------------------------------- -coming[1.5.0, number of pending tasks was added in 1.5.0] - The API can also be executed against one or more indices to get just the specified indices health: diff --git a/docs/reference/index.asciidoc b/docs/reference/index.asciidoc index 3a5945d9931..3288cad5855 100644 --- a/docs/reference/index.asciidoc +++ b/docs/reference/index.asciidoc @@ -1,7 +1,7 @@ [[elasticsearch-reference]] = Reference -:version: 1.5.0 +:version: 1.5.1 :branch: 1.5 :jdk: 1.8.0_25 :defguide: https://www.elastic.co/guide/en/elasticsearch/guide/current diff --git a/docs/reference/indices/recovery.asciidoc b/docs/reference/indices/recovery.asciidoc index b50b5c4dce9..defc86d25c1 100644 --- a/docs/reference/indices/recovery.asciidoc +++ b/docs/reference/indices/recovery.asciidoc @@ -19,7 +19,6 @@ curl -XGET http://localhost:9200/_recovery?pretty&human -------------------------------------------------- Response: -coming[1.5.0, this syntax was change to fix inconsistencies with other API] [source,js] -------------------------------------------------- { @@ -94,7 +93,6 @@ In some cases a higher level of detail may be preferable. Setting "detailed=true curl -XGET http://localhost:9200/_recovery?pretty&human&detailed=true -------------------------------------------------- -coming[1.5.0, this syntax was change to fix inconsistencies with other API] Response: [source,js] diff --git a/docs/reference/mapping/fields/timestamp-field.asciidoc b/docs/reference/mapping/fields/timestamp-field.asciidoc index 0af1e749716..ce7520708f8 100644 --- a/docs/reference/mapping/fields/timestamp-field.asciidoc +++ b/docs/reference/mapping/fields/timestamp-field.asciidoc @@ -122,8 +122,6 @@ You can also set the default value to any date respecting < `sigma` controls how many standard deviations +/- from the mean should be displayed added[1.4.3] +<1> `sigma` controls how many standard deviations +/- from the mean should be displayed `sigma` can be any non-negative double, meaning you can request non-integer values such as `1.5`. A value of `0` is valid, but will simply return the average for both `upper` and `lower` bounds. diff --git a/docs/reference/search/search-template.asciidoc b/docs/reference/search/search-template.asciidoc index 2b2fd2550fd..bb33628ba3b 100644 --- a/docs/reference/search/search-template.asciidoc +++ b/docs/reference/search/search-template.asciidoc @@ -28,7 +28,7 @@ documentation of the mustache project]. NOTE: The mustache language is implemented in elasticsearch as a sandboxed scripting language, hence it obeys settings that may be used to enable or disable scripts per language, source and operation as described in -<> coming[1.6.0, `mustache` scripts were always on before and it wasn't possible to disable them]. +<> [float] ==== More template examples diff --git a/docs/reference/setup/configuration.asciidoc b/docs/reference/setup/configuration.asciidoc index 0795c8a3b3f..c768a490f53 100644 --- a/docs/reference/setup/configuration.asciidoc +++ b/docs/reference/setup/configuration.asciidoc @@ -323,6 +323,6 @@ appender section contains the destinations for the logs. Extensive information on how to customize logging and all the supported appenders can be found on the http://logging.apache.org/log4j/1.2/manual.html[log4j documentation]. -coming[1.5.0] Additional Appenders and other logging classes provided by +Additional Appenders and other logging classes provided by http://logging.apache.org/log4j/extras/[log4j-extras] are also available, out of the box. From 3b412992739ef2125ba9072d756ff8121f325bdb Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Thu, 9 Apr 2015 11:05:56 +0200 Subject: [PATCH 23/24] Add missing hashCode method to RecoveryState#File --- .../indices/recovery/RecoveryState.java | 9 +++++++++ .../indices/recovery/RecoveryStateTest.java | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/src/main/java/org/elasticsearch/indices/recovery/RecoveryState.java b/src/main/java/org/elasticsearch/indices/recovery/RecoveryState.java index a51d779b41b..3e6e5c47828 100644 --- a/src/main/java/org/elasticsearch/indices/recovery/RecoveryState.java +++ b/src/main/java/org/elasticsearch/indices/recovery/RecoveryState.java @@ -655,6 +655,15 @@ public class RecoveryState implements ToXContent, Streamable { return false; } + @Override + public int hashCode() { + int result = name.hashCode(); + result = 31 * result + (int) (length ^ (length >>> 32)); + result = 31 * result + (int) (recovered ^ (recovered >>> 32)); + result = 31 * result + (reused ? 1 : 0); + return result; + } + @Override public String toString() { return "file (name [" + name + "], reused [" + reused + "], length [" + length + "], recovered [" + recovered + "])"; diff --git a/src/test/java/org/elasticsearch/indices/recovery/RecoveryStateTest.java b/src/test/java/org/elasticsearch/indices/recovery/RecoveryStateTest.java index ad57f0d8c9a..91850bb6b9f 100644 --- a/src/test/java/org/elasticsearch/indices/recovery/RecoveryStateTest.java +++ b/src/test/java/org/elasticsearch/indices/recovery/RecoveryStateTest.java @@ -507,4 +507,23 @@ public class RecoveryStateTest extends ElasticsearchTestCase { readWriteIndex.join(); assertThat(readWriteIndex.error.get(), equalTo(null)); } + + @Test + public void testFileHashCodeAndEquals() { + File f = new File("foo", randomIntBetween(0, 100), randomBoolean()); + File anotherFile = new File(f.name(), f.length(), f.reused()); + assertEquals(f, anotherFile); + assertEquals(f.hashCode(), anotherFile.hashCode()); + int iters = randomIntBetween(10, 100); + for (int i = 0; i < iters; i++) { + f = new File("foo", randomIntBetween(0, 100), randomBoolean()); + anotherFile = new File(f.name(), randomIntBetween(0, 100), randomBoolean()); + if (f.equals(anotherFile)) { + assertEquals(f.hashCode(), anotherFile.hashCode()); + } else if (f.hashCode() != anotherFile.hashCode()) { + assertFalse(f.equals(anotherFile)); + } + } + + } } From fcc09f62b91820e207edb2d69328c774cae30f7e Mon Sep 17 00:00:00 2001 From: Colin Goodheart-Smithe Date: Thu, 9 Apr 2015 14:28:37 +0100 Subject: [PATCH 24/24] Aggregations: removed aggregations from ReduceContext ReduceContext contains the list of aggregations to reduce but these aggregations are set as null half of the time. This change makes the reduce(ReduceContext) method changed to reduce(List, ReduceContext) and ReduceContext now only holds the BigArrays and Script services. --- .../percolator/PercolatorService.java | 27 +++++++++++++++---- .../aggregations/InternalAggregation.java | 11 ++------ .../aggregations/InternalAggregations.java | 2 +- .../InternalSingleBucketAggregation.java | 3 +-- .../bucket/filters/InternalFilters.java | 3 +-- .../bucket/geogrid/InternalGeoHashGrid.java | 3 +-- .../bucket/histogram/InternalHistogram.java | 7 +++-- .../bucket/range/InternalRange.java | 3 +-- .../significant/InternalSignificantTerms.java | 3 +-- .../significant/UnmappedSignificantTerms.java | 7 +++-- .../bucket/terms/InternalTerms.java | 3 +-- .../bucket/terms/UnmappedTerms.java | 6 ++--- .../aggregations/metrics/avg/InternalAvg.java | 5 ++-- .../cardinality/InternalCardinality.java | 3 +-- .../metrics/geobounds/InternalGeoBounds.java | 4 +-- .../aggregations/metrics/max/InternalMax.java | 5 ++-- .../aggregations/metrics/min/InternalMin.java | 5 ++-- .../AbstractInternalPercentiles.java | 3 +-- .../scripted/InternalScriptedMetric.java | 6 ++--- .../metrics/stats/InternalStats.java | 5 ++-- .../stats/extended/InternalExtendedStats.java | 7 ++--- .../aggregations/metrics/sum/InternalSum.java | 5 ++-- .../metrics/tophits/InternalTopHits.java | 9 +++---- .../valuecount/InternalValueCount.java | 5 ++-- .../controller/SearchPhaseController.java | 18 ++++++++++--- 25 files changed, 88 insertions(+), 70 deletions(-) diff --git a/src/main/java/org/elasticsearch/percolator/PercolatorService.java b/src/main/java/org/elasticsearch/percolator/PercolatorService.java index f19b3b076e7..cd5dbf471eb 100644 --- a/src/main/java/org/elasticsearch/percolator/PercolatorService.java +++ b/src/main/java/org/elasticsearch/percolator/PercolatorService.java @@ -19,11 +19,19 @@ package org.elasticsearch.percolator; import com.carrotsearch.hppc.ByteObjectOpenHashMap; + import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.ReaderUtil; import org.apache.lucene.index.memory.ExtendedMemoryIndex; import org.apache.lucene.index.memory.MemoryIndex; -import org.apache.lucene.search.*; +import org.apache.lucene.search.BooleanClause; +import org.apache.lucene.search.ConstantScoreQuery; +import org.apache.lucene.search.Filter; +import org.apache.lucene.search.FilteredQuery; +import org.apache.lucene.search.MatchAllDocsQuery; +import org.apache.lucene.search.Query; +import org.apache.lucene.search.ScoreDoc; +import org.apache.lucene.search.TopDocs; import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.CloseableThreadLocal; import org.elasticsearch.ElasticsearchException; @@ -58,14 +66,21 @@ import org.elasticsearch.index.IndexService; import org.elasticsearch.index.engine.Engine; import org.elasticsearch.index.fielddata.IndexFieldData; import org.elasticsearch.index.fielddata.SortedBinaryDocValues; -import org.elasticsearch.index.mapper.*; +import org.elasticsearch.index.mapper.DocumentMapper; +import org.elasticsearch.index.mapper.FieldMapper; +import org.elasticsearch.index.mapper.MapperService; +import org.elasticsearch.index.mapper.ParsedDocument; +import org.elasticsearch.index.mapper.Uid; import org.elasticsearch.index.mapper.internal.UidFieldMapper; import org.elasticsearch.index.percolator.stats.ShardPercolateService; import org.elasticsearch.index.query.ParsedQuery; import org.elasticsearch.index.search.nested.NonNestedDocsFilter; import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.indices.IndicesService; -import org.elasticsearch.percolator.QueryCollector.*; +import org.elasticsearch.percolator.QueryCollector.Count; +import org.elasticsearch.percolator.QueryCollector.Match; +import org.elasticsearch.percolator.QueryCollector.MatchAndScore; +import org.elasticsearch.percolator.QueryCollector.MatchAndSort; import org.elasticsearch.script.ScriptService; import org.elasticsearch.search.SearchParseElement; import org.elasticsearch.search.SearchShardTarget; @@ -83,7 +98,9 @@ import java.util.List; import java.util.Map; import static org.elasticsearch.index.mapper.SourceToParse.source; -import static org.elasticsearch.percolator.QueryCollector.*; +import static org.elasticsearch.percolator.QueryCollector.count; +import static org.elasticsearch.percolator.QueryCollector.match; +import static org.elasticsearch.percolator.QueryCollector.matchAndScore; public class PercolatorService extends AbstractComponent { @@ -834,7 +851,7 @@ public class PercolatorService extends AbstractComponent { for (PercolateShardResponse shardResult : shardResults) { aggregationsList.add(shardResult.aggregations()); } - return InternalAggregations.reduce(aggregationsList, new ReduceContext(null, bigArrays, scriptService)); + return InternalAggregations.reduce(aggregationsList, new ReduceContext(bigArrays, scriptService)); } } diff --git a/src/main/java/org/elasticsearch/search/aggregations/InternalAggregation.java b/src/main/java/org/elasticsearch/search/aggregations/InternalAggregation.java index 0a90c8831ce..5fe69e74060 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/InternalAggregation.java +++ b/src/main/java/org/elasticsearch/search/aggregations/InternalAggregation.java @@ -18,7 +18,6 @@ */ package org.elasticsearch.search.aggregations; -import org.elasticsearch.Version; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.StreamInput; @@ -89,20 +88,14 @@ public abstract class InternalAggregation implements Aggregation, ToXContent, St public static class ReduceContext { - private final List aggregations; private final BigArrays bigArrays; private ScriptService scriptService; - public ReduceContext(List aggregations, BigArrays bigArrays, ScriptService scriptService) { - this.aggregations = aggregations; + public ReduceContext(BigArrays bigArrays, ScriptService scriptService) { this.bigArrays = bigArrays; this.scriptService = scriptService; } - public List aggregations() { - return aggregations; - } - public BigArrays bigArrays() { return bigArrays; } @@ -146,7 +139,7 @@ public abstract class InternalAggregation implements Aggregation, ToXContent, St * try reusing an existing get instance (typically the first in the given list) to save on redundant object * construction. */ - public abstract InternalAggregation reduce(ReduceContext reduceContext); + public abstract InternalAggregation reduce(List aggregations, ReduceContext reduceContext); @Override public Object getProperty(String path) { diff --git a/src/main/java/org/elasticsearch/search/aggregations/InternalAggregations.java b/src/main/java/org/elasticsearch/search/aggregations/InternalAggregations.java index bd8be5a167c..7537878ae04 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/InternalAggregations.java +++ b/src/main/java/org/elasticsearch/search/aggregations/InternalAggregations.java @@ -169,7 +169,7 @@ public class InternalAggregations implements Aggregations, ToXContent, Streamabl for (Map.Entry> entry : aggByName.entrySet()) { List aggregations = entry.getValue(); InternalAggregation first = aggregations.get(0); // the list can't be empty as it's created on demand - reducedAggregations.add(first.reduce(new InternalAggregation.ReduceContext(aggregations, context.bigArrays(), context.scriptService()))); + reducedAggregations.add(first.reduce(aggregations, context)); } return new InternalAggregations(reducedAggregations); } diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/InternalSingleBucketAggregation.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/InternalSingleBucketAggregation.java index 31d105d5ead..f278be9f663 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/InternalSingleBucketAggregation.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/InternalSingleBucketAggregation.java @@ -69,8 +69,7 @@ public abstract class InternalSingleBucketAggregation extends InternalAggregatio protected abstract InternalSingleBucketAggregation newAggregation(String name, long docCount, InternalAggregations subAggregations); @Override - public InternalAggregation reduce(ReduceContext reduceContext) { - List aggregations = reduceContext.aggregations(); + public InternalAggregation reduce(List aggregations, ReduceContext reduceContext) { long docCount = 0L; List subAggregationsList = new ArrayList<>(aggregations.size()); for (InternalAggregation aggregation : aggregations) { diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/filters/InternalFilters.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/filters/InternalFilters.java index 2642c99a2de..91624557740 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/filters/InternalFilters.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/filters/InternalFilters.java @@ -191,8 +191,7 @@ public class InternalFilters extends InternalMultiBucketAggregation implements F } @Override - public InternalAggregation reduce(ReduceContext reduceContext) { - List aggregations = reduceContext.aggregations(); + public InternalAggregation reduce(List aggregations, ReduceContext reduceContext) { List> bucketsList = null; for (InternalAggregation aggregation : aggregations) { InternalFilters filters = (InternalFilters) aggregation; diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/InternalGeoHashGrid.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/InternalGeoHashGrid.java index 0d09f05694d..40ab098b624 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/InternalGeoHashGrid.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/InternalGeoHashGrid.java @@ -188,8 +188,7 @@ public class InternalGeoHashGrid extends InternalMultiBucketAggregation implemen } @Override - public InternalGeoHashGrid reduce(ReduceContext reduceContext) { - List aggregations = reduceContext.aggregations(); + public InternalGeoHashGrid reduce(List aggregations, ReduceContext reduceContext) { LongObjectPagedHashMap> buckets = null; for (InternalAggregation aggregation : aggregations) { diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalHistogram.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalHistogram.java index c7909442016..491422d20cf 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalHistogram.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalHistogram.java @@ -297,8 +297,7 @@ public class InternalHistogram extends Inter } - private List reduceBuckets(ReduceContext reduceContext) { - List aggregations = reduceContext.aggregations(); + private List reduceBuckets(List aggregations, ReduceContext reduceContext) { final PriorityQueue> pq = new PriorityQueue>(aggregations.size()) { @Override @@ -412,8 +411,8 @@ public class InternalHistogram extends Inter } @Override - public InternalAggregation reduce(ReduceContext reduceContext) { - List reducedBuckets = reduceBuckets(reduceContext); + public InternalAggregation reduce(List aggregations, ReduceContext reduceContext) { + List reducedBuckets = reduceBuckets(aggregations, reduceContext); // adding empty buckets if needed if (minDocCount == 0) { diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalRange.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalRange.java index a6f480f9d3e..5f8b7baa2ac 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalRange.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalRange.java @@ -260,8 +260,7 @@ public class InternalRange extends InternalMulti } @Override - public InternalAggregation reduce(ReduceContext reduceContext) { - List aggregations = reduceContext.aggregations(); + public InternalAggregation reduce(List aggregations, ReduceContext reduceContext) { @SuppressWarnings("unchecked") List[] rangeList = new List[ranges.size()]; for (int i = 0; i < rangeList.length; ++i) { diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/InternalSignificantTerms.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/InternalSignificantTerms.java index f877752daf0..6ea57b606a9 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/InternalSignificantTerms.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/InternalSignificantTerms.java @@ -156,8 +156,7 @@ public abstract class InternalSignificantTerms extends InternalMultiBucketAggreg } @Override - public InternalAggregation reduce(ReduceContext reduceContext) { - List aggregations = reduceContext.aggregations(); + public InternalAggregation reduce(List aggregations, ReduceContext reduceContext) { long globalSubsetSize = 0; long globalSupersetSize = 0; diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/UnmappedSignificantTerms.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/UnmappedSignificantTerms.java index c457c1331b8..bf29dd630a3 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/UnmappedSignificantTerms.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/UnmappedSignificantTerms.java @@ -26,7 +26,6 @@ import org.elasticsearch.search.aggregations.InternalAggregation; import org.elasticsearch.search.aggregations.bucket.significant.heuristics.JLHScore; import java.io.IOException; -import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Map; @@ -68,10 +67,10 @@ public class UnmappedSignificantTerms extends InternalSignificantTerms { } @Override - public InternalAggregation reduce(ReduceContext reduceContext) { - for (InternalAggregation aggregation : reduceContext.aggregations()) { + public InternalAggregation reduce(List aggregations, ReduceContext reduceContext) { + for (InternalAggregation aggregation : aggregations) { if (!(aggregation instanceof UnmappedSignificantTerms)) { - return aggregation.reduce(reduceContext); + return aggregation.reduce(aggregations, reduceContext); } } return this; diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalTerms.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalTerms.java index a2bda335333..ff7cf1ab78d 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalTerms.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalTerms.java @@ -162,8 +162,7 @@ public abstract class InternalTerms extends InternalMultiBucketAggregation imple } @Override - public InternalAggregation reduce(ReduceContext reduceContext) { - List aggregations = reduceContext.aggregations(); + public InternalAggregation reduce(List aggregations, ReduceContext reduceContext) { Multimap buckets = ArrayListMultimap.create(); long sumDocCountError = 0; diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/UnmappedTerms.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/UnmappedTerms.java index a515596868e..2cbdd4eabdc 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/UnmappedTerms.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/UnmappedTerms.java @@ -81,10 +81,10 @@ public class UnmappedTerms extends InternalTerms { } @Override - public InternalAggregation reduce(ReduceContext reduceContext) { - for (InternalAggregation agg : reduceContext.aggregations()) { + public InternalAggregation reduce(List aggregations, ReduceContext reduceContext) { + for (InternalAggregation agg : aggregations) { if (!(agg instanceof UnmappedTerms)) { - return agg.reduce(reduceContext); + return agg.reduce(aggregations, reduceContext); } } return this; diff --git a/src/main/java/org/elasticsearch/search/aggregations/metrics/avg/InternalAvg.java b/src/main/java/org/elasticsearch/search/aggregations/metrics/avg/InternalAvg.java index 71441598736..ace13ca0496 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/metrics/avg/InternalAvg.java +++ b/src/main/java/org/elasticsearch/search/aggregations/metrics/avg/InternalAvg.java @@ -29,6 +29,7 @@ import org.elasticsearch.search.aggregations.support.format.ValueFormatter; import org.elasticsearch.search.aggregations.support.format.ValueFormatterStreams; import java.io.IOException; +import java.util.List; import java.util.Map; /** @@ -79,10 +80,10 @@ public class InternalAvg extends InternalNumericMetricsAggregation.SingleValue i } @Override - public InternalAvg reduce(ReduceContext reduceContext) { + public InternalAvg reduce(List aggregations, ReduceContext reduceContext) { long count = 0; double sum = 0; - for (InternalAggregation aggregation : reduceContext.aggregations()) { + for (InternalAggregation aggregation : aggregations) { count += ((InternalAvg) aggregation).count; sum += ((InternalAvg) aggregation).sum; } diff --git a/src/main/java/org/elasticsearch/search/aggregations/metrics/cardinality/InternalCardinality.java b/src/main/java/org/elasticsearch/search/aggregations/metrics/cardinality/InternalCardinality.java index 2fd964e5f1f..daef68a9811 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/metrics/cardinality/InternalCardinality.java +++ b/src/main/java/org/elasticsearch/search/aggregations/metrics/cardinality/InternalCardinality.java @@ -99,8 +99,7 @@ public final class InternalCardinality extends InternalNumericMetricsAggregation } @Override - public InternalAggregation reduce(ReduceContext reduceContext) { - List aggregations = reduceContext.aggregations(); + public InternalAggregation reduce(List aggregations, ReduceContext reduceContext) { InternalCardinality reduced = null; for (InternalAggregation aggregation : aggregations) { final InternalCardinality cardinality = (InternalCardinality) aggregation; diff --git a/src/main/java/org/elasticsearch/search/aggregations/metrics/geobounds/InternalGeoBounds.java b/src/main/java/org/elasticsearch/search/aggregations/metrics/geobounds/InternalGeoBounds.java index cdda6597c14..a59abe1ad2b 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/metrics/geobounds/InternalGeoBounds.java +++ b/src/main/java/org/elasticsearch/search/aggregations/metrics/geobounds/InternalGeoBounds.java @@ -73,7 +73,7 @@ public class InternalGeoBounds extends InternalMetricsAggregation implements Geo } @Override - public InternalAggregation reduce(ReduceContext reduceContext) { + public InternalAggregation reduce(List aggregations, ReduceContext reduceContext) { double top = Double.NEGATIVE_INFINITY; double bottom = Double.POSITIVE_INFINITY; double posLeft = Double.POSITIVE_INFINITY; @@ -81,7 +81,7 @@ public class InternalGeoBounds extends InternalMetricsAggregation implements Geo double negLeft = Double.POSITIVE_INFINITY; double negRight = Double.NEGATIVE_INFINITY; - for (InternalAggregation aggregation : reduceContext.aggregations()) { + for (InternalAggregation aggregation : aggregations) { InternalGeoBounds bounds = (InternalGeoBounds) aggregation; if (bounds.top > top) { diff --git a/src/main/java/org/elasticsearch/search/aggregations/metrics/max/InternalMax.java b/src/main/java/org/elasticsearch/search/aggregations/metrics/max/InternalMax.java index cb106448aa4..8d45a7cb45f 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/metrics/max/InternalMax.java +++ b/src/main/java/org/elasticsearch/search/aggregations/metrics/max/InternalMax.java @@ -29,6 +29,7 @@ import org.elasticsearch.search.aggregations.support.format.ValueFormatter; import org.elasticsearch.search.aggregations.support.format.ValueFormatterStreams; import java.io.IOException; +import java.util.List; import java.util.Map; /** @@ -77,9 +78,9 @@ public class InternalMax extends InternalNumericMetricsAggregation.SingleValue i } @Override - public InternalMax reduce(ReduceContext reduceContext) { + public InternalMax reduce(List aggregations, ReduceContext reduceContext) { double max = Double.NEGATIVE_INFINITY; - for (InternalAggregation aggregation : reduceContext.aggregations()) { + for (InternalAggregation aggregation : aggregations) { max = Math.max(max, ((InternalMax) aggregation).max); } return new InternalMax(name, max, valueFormatter, getMetaData()); diff --git a/src/main/java/org/elasticsearch/search/aggregations/metrics/min/InternalMin.java b/src/main/java/org/elasticsearch/search/aggregations/metrics/min/InternalMin.java index ad9d47a34ee..9a4d2e1a413 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/metrics/min/InternalMin.java +++ b/src/main/java/org/elasticsearch/search/aggregations/metrics/min/InternalMin.java @@ -29,6 +29,7 @@ import org.elasticsearch.search.aggregations.support.format.ValueFormatter; import org.elasticsearch.search.aggregations.support.format.ValueFormatterStreams; import java.io.IOException; +import java.util.List; import java.util.Map; /** @@ -78,9 +79,9 @@ public class InternalMin extends InternalNumericMetricsAggregation.SingleValue i } @Override - public InternalMin reduce(ReduceContext reduceContext) { + public InternalMin reduce(List aggregations, ReduceContext reduceContext) { double min = Double.POSITIVE_INFINITY; - for (InternalAggregation aggregation : reduceContext.aggregations()) { + for (InternalAggregation aggregation : aggregations) { min = Math.min(min, ((InternalMin) aggregation).min); } return new InternalMin(getName(), min, this.valueFormatter, getMetaData()); diff --git a/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/AbstractInternalPercentiles.java b/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/AbstractInternalPercentiles.java index 67f33934bf6..b3416a1531f 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/AbstractInternalPercentiles.java +++ b/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/AbstractInternalPercentiles.java @@ -60,8 +60,7 @@ abstract class AbstractInternalPercentiles extends InternalNumericMetricsAggrega public abstract double value(double key); @Override - public AbstractInternalPercentiles reduce(ReduceContext reduceContext) { - List aggregations = reduceContext.aggregations(); + public AbstractInternalPercentiles reduce(List aggregations, ReduceContext reduceContext) { TDigestState merged = null; for (InternalAggregation aggregation : aggregations) { final AbstractInternalPercentiles percentiles = (AbstractInternalPercentiles) aggregation; diff --git a/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/InternalScriptedMetric.java b/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/InternalScriptedMetric.java index ed0119e84ae..a2e03a3b460 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/InternalScriptedMetric.java +++ b/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/InternalScriptedMetric.java @@ -82,13 +82,13 @@ public class InternalScriptedMetric extends InternalMetricsAggregation implement } @Override - public InternalAggregation reduce(ReduceContext reduceContext) { + public InternalAggregation reduce(List aggregations, ReduceContext reduceContext) { List aggregationObjects = new ArrayList<>(); - for (InternalAggregation aggregation : reduceContext.aggregations()) { + for (InternalAggregation aggregation : aggregations) { InternalScriptedMetric mapReduceAggregation = (InternalScriptedMetric) aggregation; aggregationObjects.add(mapReduceAggregation.aggregation()); } - InternalScriptedMetric firstAggregation = ((InternalScriptedMetric) reduceContext.aggregations().get(0)); + InternalScriptedMetric firstAggregation = ((InternalScriptedMetric) aggregations.get(0)); Object aggregation; if (firstAggregation.reduceScript != null) { Map params; diff --git a/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/InternalStats.java b/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/InternalStats.java index 86bda11cd8e..a888d5f55d7 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/InternalStats.java +++ b/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/InternalStats.java @@ -30,6 +30,7 @@ import org.elasticsearch.search.aggregations.support.format.ValueFormatter; import org.elasticsearch.search.aggregations.support.format.ValueFormatterStreams; import java.io.IOException; +import java.util.List; import java.util.Map; /** @@ -148,12 +149,12 @@ public class InternalStats extends InternalNumericMetricsAggregation.MultiValue } @Override - public InternalStats reduce(ReduceContext reduceContext) { + public InternalStats reduce(List aggregations, ReduceContext reduceContext) { long count = 0; double min = Double.POSITIVE_INFINITY; double max = Double.NEGATIVE_INFINITY; double sum = 0; - for (InternalAggregation aggregation : reduceContext.aggregations()) { + for (InternalAggregation aggregation : aggregations) { InternalStats stats = (InternalStats) aggregation; count += stats.getCount(); min = Math.min(min, stats.getMin()); diff --git a/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/extended/InternalExtendedStats.java b/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/extended/InternalExtendedStats.java index 9a700690530..0d36096505b 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/extended/InternalExtendedStats.java +++ b/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/extended/InternalExtendedStats.java @@ -30,6 +30,7 @@ import org.elasticsearch.search.aggregations.metrics.stats.InternalStats; import org.elasticsearch.search.aggregations.support.format.ValueFormatter; import java.io.IOException; +import java.util.List; import java.util.Map; /** @@ -143,13 +144,13 @@ public class InternalExtendedStats extends InternalStats implements ExtendedStat } @Override - public InternalExtendedStats reduce(ReduceContext reduceContext) { + public InternalExtendedStats reduce(List aggregations, ReduceContext reduceContext) { double sumOfSqrs = 0; - for (InternalAggregation aggregation : reduceContext.aggregations()) { + for (InternalAggregation aggregation : aggregations) { InternalExtendedStats stats = (InternalExtendedStats) aggregation; sumOfSqrs += stats.getSumOfSquares(); } - final InternalStats stats = super.reduce(reduceContext); + final InternalStats stats = super.reduce(aggregations, reduceContext); return new InternalExtendedStats(name, stats.getCount(), stats.getSum(), stats.getMin(), stats.getMax(), sumOfSqrs, sigma, valueFormatter, getMetaData()); } diff --git a/src/main/java/org/elasticsearch/search/aggregations/metrics/sum/InternalSum.java b/src/main/java/org/elasticsearch/search/aggregations/metrics/sum/InternalSum.java index 11044f9798f..a9638bce6a9 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/metrics/sum/InternalSum.java +++ b/src/main/java/org/elasticsearch/search/aggregations/metrics/sum/InternalSum.java @@ -29,6 +29,7 @@ import org.elasticsearch.search.aggregations.support.format.ValueFormatter; import org.elasticsearch.search.aggregations.support.format.ValueFormatterStreams; import java.io.IOException; +import java.util.List; import java.util.Map; /** @@ -77,9 +78,9 @@ public class InternalSum extends InternalNumericMetricsAggregation.SingleValue i } @Override - public InternalSum reduce(ReduceContext reduceContext) { + public InternalSum reduce(List aggregations, ReduceContext reduceContext) { double sum = 0; - for (InternalAggregation aggregation : reduceContext.aggregations()) { + for (InternalAggregation aggregation : aggregations) { sum += ((InternalSum) aggregation).sum; } return new InternalSum(name, sum, valueFormatter, getMetaData()); diff --git a/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/InternalTopHits.java b/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/InternalTopHits.java index ec18b7e93e3..b33f8bb092e 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/InternalTopHits.java +++ b/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/InternalTopHits.java @@ -18,9 +18,6 @@ */ package org.elasticsearch.search.aggregations.metrics.tophits; -import java.io.IOException; -import java.util.List; - import org.apache.lucene.search.ScoreDoc; import org.apache.lucene.search.Sort; import org.apache.lucene.search.TopDocs; @@ -38,6 +35,9 @@ import org.elasticsearch.search.aggregations.metrics.InternalMetricsAggregation; import org.elasticsearch.search.internal.InternalSearchHit; import org.elasticsearch.search.internal.InternalSearchHits; +import java.io.IOException; +import java.util.List; + /** */ public class InternalTopHits extends InternalMetricsAggregation implements TopHits { @@ -85,8 +85,7 @@ public class InternalTopHits extends InternalMetricsAggregation implements TopHi } @Override - public InternalAggregation reduce(ReduceContext reduceContext) { - List aggregations = reduceContext.aggregations(); + public InternalAggregation reduce(List aggregations, ReduceContext reduceContext) { InternalSearchHits[] shardHits = new InternalSearchHits[aggregations.size()]; final TopDocs reducedTopDocs; diff --git a/src/main/java/org/elasticsearch/search/aggregations/metrics/valuecount/InternalValueCount.java b/src/main/java/org/elasticsearch/search/aggregations/metrics/valuecount/InternalValueCount.java index 062e88fce5f..1ac855d4c90 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/metrics/valuecount/InternalValueCount.java +++ b/src/main/java/org/elasticsearch/search/aggregations/metrics/valuecount/InternalValueCount.java @@ -28,6 +28,7 @@ import org.elasticsearch.search.aggregations.metrics.InternalNumericMetricsAggre import org.elasticsearch.search.aggregations.support.format.ValueFormatter; import java.io.IOException; +import java.util.List; import java.util.Map; /** @@ -76,9 +77,9 @@ public class InternalValueCount extends InternalNumericMetricsAggregation.Single } @Override - public InternalAggregation reduce(ReduceContext reduceContext) { + public InternalAggregation reduce(List aggregations, ReduceContext reduceContext) { long valueCount = 0; - for (InternalAggregation aggregation : reduceContext.aggregations()) { + for (InternalAggregation aggregation : aggregations) { valueCount += ((InternalValueCount) aggregation).value; } return new InternalValueCount(name, valueCount, valueFormatter, getMetaData()); diff --git a/src/main/java/org/elasticsearch/search/controller/SearchPhaseController.java b/src/main/java/org/elasticsearch/search/controller/SearchPhaseController.java index 91d8948878b..1894e26277d 100644 --- a/src/main/java/org/elasticsearch/search/controller/SearchPhaseController.java +++ b/src/main/java/org/elasticsearch/search/controller/SearchPhaseController.java @@ -23,7 +23,14 @@ import com.carrotsearch.hppc.IntArrayList; import com.carrotsearch.hppc.ObjectObjectOpenHashMap; import org.apache.lucene.index.Term; -import org.apache.lucene.search.*; +import org.apache.lucene.search.CollectionStatistics; +import org.apache.lucene.search.FieldDoc; +import org.apache.lucene.search.ScoreDoc; +import org.apache.lucene.search.Sort; +import org.apache.lucene.search.SortField; +import org.apache.lucene.search.TermStatistics; +import org.apache.lucene.search.TopDocs; +import org.apache.lucene.search.TopFieldDocs; import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.common.collect.HppcMaps; import org.elasticsearch.common.component.AbstractComponent; @@ -47,7 +54,12 @@ import org.elasticsearch.search.query.QuerySearchResultProvider; import org.elasticsearch.search.suggest.Suggest; import java.io.IOException; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Comparator; +import java.util.HashMap; +import java.util.List; +import java.util.Map; /** * @@ -387,7 +399,7 @@ public class SearchPhaseController extends AbstractComponent { for (AtomicArray.Entry entry : queryResults) { aggregationsList.add((InternalAggregations) entry.value.queryResult().aggregations()); } - aggregations = InternalAggregations.reduce(aggregationsList, new ReduceContext(null, bigArrays, scriptService)); + aggregations = InternalAggregations.reduce(aggregationsList, new ReduceContext(bigArrays, scriptService)); } }