From f843a1375f6d8bdbd9250284fe4b7725af7ed958 Mon Sep 17 00:00:00 2001 From: David Pilato Date: Mon, 10 Aug 2015 10:40:06 +0200 Subject: [PATCH 01/10] [cloud-aws] add support for base_path in elasticsearch.yml Related to https://github.com/elastic/elasticsearch-cloud-aws/issues/230 We now can support setting a global `base_path` in `elasticsearch.yml` using `repositories.s3.base_path` key. --- plugins/cloud-aws/README.md | 2 +- .../repositories/s3/S3Repository.java | 2 +- .../s3/AbstractS3SnapshotRestoreTest.java | 15 +++++++++++---- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/plugins/cloud-aws/README.md b/plugins/cloud-aws/README.md index 08957b0ebd5..1f5f53997c8 100644 --- a/plugins/cloud-aws/README.md +++ b/plugins/cloud-aws/README.md @@ -187,7 +187,7 @@ The following settings are supported: * `region`: The region where bucket is located. Defaults to US Standard * `endpoint`: The endpoint to the S3 API. Defaults to AWS's default S3 endpoint. Note that setting a region overrides the endpoint setting. * `protocol`: The protocol to use (`http` or `https`). Defaults to value of `cloud.aws.protocol` or `cloud.aws.s3.protocol`. -* `base_path`: Specifies the path within bucket to repository data. Defaults to root directory. +* `base_path`: Specifies the path within bucket to repository data. Defaults to value of `repositories.s3.base_path` or to root directory if not set. * `access_key`: The access key to use for authentication. Defaults to value of `cloud.aws.access_key`. * `secret_key`: The secret key to use for authentication. Defaults to value of `cloud.aws.secret_key`. * `chunk_size`: Big files can be broken down into chunks during snapshotting if needed. The chunk size can be specified in bytes or by using size value notation, i.e. `1g`, `10m`, `5k`. Defaults to `100m`. diff --git a/plugins/cloud-aws/src/main/java/org/elasticsearch/repositories/s3/S3Repository.java b/plugins/cloud-aws/src/main/java/org/elasticsearch/repositories/s3/S3Repository.java index ecd391956e7..4be35ba1098 100644 --- a/plugins/cloud-aws/src/main/java/org/elasticsearch/repositories/s3/S3Repository.java +++ b/plugins/cloud-aws/src/main/java/org/elasticsearch/repositories/s3/S3Repository.java @@ -123,7 +123,7 @@ public class S3Repository extends BlobStoreRepository { bucket, region, endpoint, protocol, chunkSize, serverSideEncryption, bufferSize, maxRetries); blobStore = new S3BlobStore(settings, s3Service.client(endpoint, protocol, region, repositorySettings.settings().get("access_key"), repositorySettings.settings().get("secret_key"), maxRetries), bucket, region, serverSideEncryption, bufferSize, maxRetries); - String basePath = repositorySettings.settings().get("base_path", null); + String basePath = repositorySettings.settings().get("base_path", settings.get("repositories.s3.base_path")); if (Strings.hasLength(basePath)) { BlobPath path = new BlobPath(); for(String elem : Strings.splitStringToArray(basePath, '/')) { diff --git a/plugins/cloud-aws/src/test/java/org/elasticsearch/repositories/s3/AbstractS3SnapshotRestoreTest.java b/plugins/cloud-aws/src/test/java/org/elasticsearch/repositories/s3/AbstractS3SnapshotRestoreTest.java index 25dd8b96def..23441d5f509 100644 --- a/plugins/cloud-aws/src/test/java/org/elasticsearch/repositories/s3/AbstractS3SnapshotRestoreTest.java +++ b/plugins/cloud-aws/src/test/java/org/elasticsearch/repositories/s3/AbstractS3SnapshotRestoreTest.java @@ -64,6 +64,7 @@ abstract public class AbstractS3SnapshotRestoreTest extends AbstractAwsTest { .put(MockFSDirectoryService.RANDOM_NO_DELETE_OPEN_FILE, false) .put("cloud.enabled", true) .put("plugin.types", CloudAwsPlugin.class.getName()) + .put("repositories.s3.base_path", basePath) .build(); } @@ -85,11 +86,17 @@ abstract public class AbstractS3SnapshotRestoreTest extends AbstractAwsTest { @Test @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch-cloud-aws/issues/211") public void testSimpleWorkflow() { Client client = client(); + Settings.Builder settings = Settings.settingsBuilder() + .put("chunk_size", randomIntBetween(1000, 10000)); + + // We sometime test getting the base_path from node settings using repositories.s3.base_path + if (usually()) { + settings.put("base_path", basePath); + } + logger.info("--> creating s3 repository with bucket[{}] and path [{}]", internalCluster().getInstance(Settings.class).get("repositories.s3.bucket"), basePath); PutRepositoryResponse putRepositoryResponse = client.admin().cluster().preparePutRepository("test-repo") - .setType("s3").setSettings(Settings.settingsBuilder() - .put("base_path", basePath) - .put("chunk_size", randomIntBetween(1000, 10000)) + .setType("s3").setSettings(settings ).get(); assertThat(putRepositoryResponse.isAcknowledged(), equalTo(true)); @@ -342,7 +349,7 @@ abstract public class AbstractS3SnapshotRestoreTest extends AbstractAwsTest { PutRepositoryResponse putRepositoryResponse = client.admin().cluster().preparePutRepository("test-repo") .setType("s3").setSettings(Settings.settingsBuilder() .put("base_path", basePath) - ).get(); + ).get(); assertThat(putRepositoryResponse.isAcknowledged(), equalTo(true)); logger.info("--> restore non existing snapshot"); From e59bbf8e31fe449360e2849b63a3a3654aa6849c Mon Sep 17 00:00:00 2001 From: saschamarkus Date: Wed, 12 Aug 2015 15:13:03 +0200 Subject: [PATCH 02/10] Issue 11510: Throw Exception for missing settings file An Exception will be thrown when the givven YAML-file doesn't exist. --- .../org/elasticsearch/common/settings/Settings.java | 2 +- .../settings/loader/YamlSettingsLoaderTests.java | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/elasticsearch/common/settings/Settings.java b/core/src/main/java/org/elasticsearch/common/settings/Settings.java index 2671874f86f..4d422575480 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/Settings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/Settings.java @@ -1129,7 +1129,7 @@ public final class Settings implements ToXContent { } InputStream is = classLoader.getResourceAsStream(resourceName); if (is == null) { - return this; + throw new SettingsException("Failed to load settings from [" + resourceName + "]"); } return loadFromStream(resourceName, is); diff --git a/core/src/test/java/org/elasticsearch/common/settings/loader/YamlSettingsLoaderTests.java b/core/src/test/java/org/elasticsearch/common/settings/loader/YamlSettingsLoaderTests.java index e43f5e7f2aa..362188ea03d 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/loader/YamlSettingsLoaderTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/loader/YamlSettingsLoaderTests.java @@ -63,4 +63,17 @@ public class YamlSettingsLoaderTests extends ESTestCase { .loadFromClasspath("org/elasticsearch/common/settings/loader/indentation-with-explicit-document-start-settings.yml") .build(); } + + + @Test + public void testYamlSettingsNoFile() throws Exception { + String invalidResourceName = "org/elasticsearch/common/settings/loader/no-test-settings.yml"; + try { + Settings defaultSettings = settingsBuilder().loadFromClasspath(invalidResourceName).build(); + fail("For a not exiting file an exception should be thrown."); + } catch (Exception e) { + assertTrue(e instanceof SettingsException); + assertThat(e.getMessage(), equalTo("Failed to load settings from [" + invalidResourceName + "]")); + } + } } \ No newline at end of file From a9a4903a0a3e3e1db372aef90c19f782abdb541a Mon Sep 17 00:00:00 2001 From: Colin Goodheart-Smithe Date: Thu, 13 Aug 2015 09:47:05 +0100 Subject: [PATCH 03/10] Aggregations: Removed unused factor parameter in DateHistogramBuilder DateHistogramParser does not recognise this parameter and therefore setting it would throw an exception as noted in https://github.com/elastic/elasticsearch/issues/6490. It is also not documented. --- .../bucket/histogram/DateHistogramBuilder.java | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramBuilder.java index b37378652b9..6b7305db9e6 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramBuilder.java @@ -40,7 +40,6 @@ public class DateHistogramBuilder extends ValuesSourceAggregationBuilder Date: Tue, 11 Aug 2015 22:03:34 +0100 Subject: [PATCH 04/10] Fixes #11571 - update "Cluster Stats" documentation with valid example --- docs/reference/cluster/stats.asciidoc | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/docs/reference/cluster/stats.asciidoc b/docs/reference/cluster/stats.asciidoc index 00f73cc333a..8093dd32d7d 100644 --- a/docs/reference/cluster/stats.asciidoc +++ b/docs/reference/cluster/stats.asciidoc @@ -15,6 +15,7 @@ Will return, for example: ["source","js",subs="attributes,callouts"] -------------------------------------------------- { + "timestamp": 1439326129256, "cluster_name": "elasticsearch", "status": "green", "indices": { @@ -61,12 +62,35 @@ Will return, for example: "memory_size_in_bytes": 0, "evictions": 0 }, + "id_cache": { + "memory_size": "0b", + "memory_size_in_bytes": 0 + }, "completion": { "size": "0b", "size_in_bytes": 0 }, "segments": { - "count": 2 + "count": 2, + "memory": "6.4kb", + "memory_in_bytes": 6596, + "index_writer_memory": "0b", + "index_writer_memory_in_bytes": 0, + "index_writer_max_memory": "275.7mb", + "index_writer_max_memory_in_bytes": 289194639, + "version_map_memory": "0b", + "version_map_memory_in_bytes": 0, + "fixed_bit_set": "0b", + "fixed_bit_set_memory_in_bytes": 0 + }, + "percolate": { + "total": 0, + "get_time": "0s", + "time_in_millis": 0, + "current": 0, + "memory_size_in_bytes": -1, + "memory_size": "-1b", + "queries": 0 } }, "nodes": { From fbd8f6927380aae4557166fe43795f49b774eb34 Mon Sep 17 00:00:00 2001 From: David Pilato Date: Tue, 11 Aug 2015 15:22:58 +0200 Subject: [PATCH 05/10] Build of QA: Smoke Test Shaded Jar fails under maven 3.3.x Build fails with maven 3.3.1 and 3.3.3. To reproduce, install one of the 3.3.x versions of maven and run `mvn clean verify` in the root directory of the project. The build will fail in the QA: Smoke Test Shaded Jar module with the following error: ``` Started J0 PID(99979@flea.local). Suite: org.elasticsearch.shaded.test.ShadedIT 2> NOTE: reproduce with: ant test -Dtestcase=ShadedIT -Dtests.method=testJodaIsNotOnTheCP -Dtests.seed=2F4D23A7462CF921 -Dtests.locale= -Dtests.timezone=Asia/Baku -Dtests.asserts=true -Dtests.file.encoding=UTF-8 FAILURE 0.06s | ShadedIT.testJodaIsNotOnTheCP <<< > Throwable #1: junit.framework.AssertionFailedError: Expected an exception but the test passed: java.lang.ClassNotFoundException > at __randomizedtesting.SeedInfo.seed([2F4D23A7462CF921:3A9404F1F69FD80]:0) > at junit.framework.Assert.fail(Assert.java:57) > at java.lang.Thread.run(Thread.java:745) 2> NOTE: reproduce with: ant test -Dtestcase=ShadedIT -Dtests.method=testGuavaIsNotOnTheCP -Dtests.seed=2F4D23A7462CF921 -Dtests.locale= -Dtests.timezone=Asia/Baku -Dtests.asserts=true -Dtests.file.encoding=UTF-8 FAILURE 0.01s | ShadedIT.testGuavaIsNotOnTheCP <<< > Throwable #1: junit.framework.AssertionFailedError: Expected an exception but the test passed: java.lang.ClassNotFoundException > at __randomizedtesting.SeedInfo.seed([2F4D23A7462CF921:C2502FD54D83433D]:0) > at junit.framework.Assert.fail(Assert.java:57) > at java.lang.Thread.run(Thread.java:745) 2> NOTE: reproduce with: ant test -Dtestcase=ShadedIT -Dtests.method=testjsr166eIsNotOnTheCP -Dtests.seed=2F4D23A7462CF921 -Dtests.locale= -Dtests.timezone=Asia/Baku -Dtests.asserts=true -Dtests.file.encoding=UTF-8 FAILURE 0.01s | ShadedIT.testjsr166eIsNotOnTheCP <<< > Throwable #1: junit.framework.AssertionFailedError: Expected an exception but the test passed: java.lang.ClassNotFoundException > at __randomizedtesting.SeedInfo.seed([2F4D23A7462CF921:35593286F4269392]:0) > at junit.framework.Assert.fail(Assert.java:57) > at java.lang.Thread.run(Thread.java:745) 2> NOTE: leaving temporary files on disk at: /Users/Shared/Jenkins/Home/workspace/elasticsearch-master/qa/smoke-test-shaded/target/J0/temp/org.elasticsearch.shaded.test.ShadedIT_2F4D23A7462CF921-001 2> NOTE: test params are: codec=CheapBastard, sim=DefaultSimilarity, locale=, timezone=Asia/Baku 2> NOTE: Mac OS X 10.10.4 x86_64/Oracle Corporation 1.8.0_25 (64-bit)/cpus=8,threads=1,free=482137936,total=514850816 2> NOTE: All tests run in this JVM: [ShadedIT] Completed [1/1] in 6.61s, 5 tests, 3 failures <<< FAILURES! Tests with failures: - org.elasticsearch.shaded.test.ShadedIT.testJodaIsNotOnTheCP - org.elasticsearch.shaded.test.ShadedIT.testGuavaIsNotOnTheCP - org.elasticsearch.shaded.test.ShadedIT.testjsr166eIsNotOnTheCP ``` Please note that build doesn't fail with maven 3.2.x and it doesn't fail if mvn command is executed inside the qa/smoke-test-shaded directory. Only when the build is started from the root directory the error above can be observed. The reason is because of the shaded version which depends on elasticsearch core. When Maven build the module only, then elasticsearch core is not added to the dependency tree. ```sh mvn dependency:tree -pl :smoke-test-shaded ``` ``` [INFO] --- maven-dependency-plugin:2.8:tree (default-cli) @ smoke-test-shaded --- [INFO] org.elasticsearch.qa:smoke-test-shaded:jar:2.0.0-beta1-SNAPSHOT [INFO] +- org.elasticsearch.distribution.shaded:elasticsearch:jar:2.0.0-beta1-SNAPSHOT:compile [INFO] | +- org.apache.lucene:lucene-core:jar:5.2.1:compile [INFO] | +- org.apache.lucene:lucene-backward-codecs:jar:5.2.1:compile [INFO] | +- org.apache.lucene:lucene-analyzers-common:jar:5.2.1:compile [INFO] | +- org.apache.lucene:lucene-queries:jar:5.2.1:compile [INFO] | +- org.apache.lucene:lucene-memory:jar:5.2.1:compile [INFO] | +- org.apache.lucene:lucene-highlighter:jar:5.2.1:compile [INFO] | +- org.apache.lucene:lucene-queryparser:jar:5.2.1:compile [INFO] | +- org.apache.lucene:lucene-sandbox:jar:5.2.1:compile [INFO] | +- org.apache.lucene:lucene-suggest:jar:5.2.1:compile [INFO] | +- org.apache.lucene:lucene-misc:jar:5.2.1:compile [INFO] | +- org.apache.lucene:lucene-join:jar:5.2.1:compile [INFO] | +- org.apache.lucene:lucene-grouping:jar:5.2.1:compile [INFO] | +- org.apache.lucene:lucene-spatial:jar:5.2.1:compile [INFO] | \- com.spatial4j:spatial4j:jar:0.4.1:compile [INFO] +- org.hamcrest:hamcrest-all:jar:1.3:test [INFO] \- org.apache.lucene:lucene-test-framework:jar:5.2.1:test [INFO] +- org.apache.lucene:lucene-codecs:jar:5.2.1:test [INFO] +- com.carrotsearch.randomizedtesting:randomizedtesting-runner:jar:2.1.16:test [INFO] +- junit:junit:jar:4.11:test [INFO] \- org.apache.ant:ant:jar:1.8.2:test ``` But if shaded plugin is involved during the build, it modifies the `projectArtifactMap`: ```sh mvn dependency:tree -pl org.elasticsearch.distribution.shaded:elasticsearch,:smoke-test-shaded ``` ``` [INFO] --- maven-dependency-plugin:2.8:tree (default-cli) @ smoke-test-shaded --- [INFO] org.elasticsearch.qa:smoke-test-shaded:jar:2.0.0-beta1-SNAPSHOT [INFO] +- org.elasticsearch.distribution.shaded:elasticsearch:jar:2.0.0-beta1-SNAPSHOT:compile [INFO] | \- org.elasticsearch:elasticsearch:jar:2.0.0-beta1-SNAPSHOT:compile [INFO] | +- org.apache.lucene:lucene-backward-codecs:jar:5.2.1:compile [INFO] | +- org.apache.lucene:lucene-analyzers-common:jar:5.2.1:compile [INFO] | +- org.apache.lucene:lucene-queries:jar:5.2.1:compile [INFO] | +- org.apache.lucene:lucene-memory:jar:5.2.1:compile [INFO] | +- org.apache.lucene:lucene-highlighter:jar:5.2.1:compile [INFO] | +- org.apache.lucene:lucene-queryparser:jar:5.2.1:compile [INFO] | | \- org.apache.lucene:lucene-sandbox:jar:5.2.1:compile [INFO] | +- org.apache.lucene:lucene-suggest:jar:5.2.1:compile [INFO] | | \- org.apache.lucene:lucene-misc:jar:5.2.1:compile [INFO] | +- org.apache.lucene:lucene-join:jar:5.2.1:compile [INFO] | | \- org.apache.lucene:lucene-grouping:jar:5.2.1:compile [INFO] | +- org.apache.lucene:lucene-spatial:jar:5.2.1:compile [INFO] | | \- com.spatial4j:spatial4j:jar:0.4.1:compile [INFO] | +- com.google.guava:guava:jar:18.0:compile [INFO] | +- com.carrotsearch:hppc:jar:0.7.1:compile [INFO] | +- joda-time:joda-time:jar:2.8:compile [INFO] | +- org.joda:joda-convert:jar:1.2:compile [INFO] | +- com.fasterxml.jackson.core:jackson-core:jar:2.5.3:compile [INFO] | +- com.fasterxml.jackson.dataformat:jackson-dataformat-smile:jar:2.5.3:compile [INFO] | +- com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:jar:2.5.3:compile [INFO] | | \- org.yaml:snakeyaml:jar:1.12:compile [INFO] | +- com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:jar:2.5.3:compile [INFO] | +- io.netty:netty:jar:3.10.3.Final:compile [INFO] | +- com.ning:compress-lzf:jar:1.0.2:compile [INFO] | +- com.tdunning:t-digest:jar:3.0:compile [INFO] | +- org.hdrhistogram:HdrHistogram:jar:2.1.6:compile [INFO] | +- org.apache.commons:commons-lang3:jar:3.3.2:compile [INFO] | +- commons-cli:commons-cli:jar:1.3.1:compile [INFO] | \- com.twitter:jsr166e:jar:1.1.0:compile [INFO] +- org.hamcrest:hamcrest-all:jar:1.3:test [INFO] \- org.apache.lucene:lucene-test-framework:jar:5.2.1:test [INFO] +- org.apache.lucene:lucene-codecs:jar:5.2.1:test [INFO] +- org.apache.lucene:lucene-core:jar:5.2.1:compile [INFO] +- com.carrotsearch.randomizedtesting:randomizedtesting-runner:jar:2.1.16:test [INFO] +- junit:junit:jar:4.11:test [INFO] \- org.apache.ant:ant:jar:1.8.2:test ``` A fix could consist of fixing something on Maven side. Probably something changed in a recent version and introduced this "issue" but it might be not really an issue. More a fix. There are two workarounds: 1) exclude manually elasticsearch core from shaded version in smoke-test-shaded module and add manually each lucene lib needed by elasticsearch 2) add a new `elasticsearch-lucene` (lucene) POM module which simply declares all needed lucene libs in subprojects (such as the smoke tester one). I choose the later. Closes #12791. --- core/pom.xml | 42 +++--------------------- distribution/shaded/pom.xml | 1 + lucene/pom.xml | 62 ++++++++++++++++++++++++++++++++++++ pom.xml | 1 + qa/smoke-test-shaded/pom.xml | 14 +++++++- 5 files changed, 81 insertions(+), 39 deletions(-) create mode 100644 lucene/pom.xml diff --git a/core/pom.xml b/core/pom.xml index 8782a9ee95e..81b6498a8d7 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -44,44 +44,10 @@ - org.apache.lucene - lucene-core - - - org.apache.lucene - lucene-backward-codecs - - - org.apache.lucene - lucene-analyzers-common - - - org.apache.lucene - lucene-queries - - - org.apache.lucene - lucene-memory - - - org.apache.lucene - lucene-highlighter - - - org.apache.lucene - lucene-queryparser - - - org.apache.lucene - lucene-suggest - - - org.apache.lucene - lucene-join - - - org.apache.lucene - lucene-spatial + org.elasticsearch + elasticsearch-lucene + ${elasticsearch.version} + pom org.apache.lucene diff --git a/distribution/shaded/pom.xml b/distribution/shaded/pom.xml index 7024504a7ec..2302d1db2a6 100644 --- a/distribution/shaded/pom.xml +++ b/distribution/shaded/pom.xml @@ -77,6 +77,7 @@ ${project.build.directory}/dependency-reduced-pom.xml + org.elasticsearch:elasticsearch-lucene org.apache.lucene:* com.spatial4j:* diff --git a/lucene/pom.xml b/lucene/pom.xml new file mode 100644 index 00000000000..e423d9e9a70 --- /dev/null +++ b/lucene/pom.xml @@ -0,0 +1,62 @@ + + + + elasticsearch-parent + org.elasticsearch + 2.0.0-beta1-SNAPSHOT + + 4.0.0 + + elasticsearch-lucene + Lucene dependencies for elasticsearch + pom + + + true + + + + + org.apache.lucene + lucene-core + + + org.apache.lucene + lucene-backward-codecs + + + org.apache.lucene + lucene-analyzers-common + + + org.apache.lucene + lucene-queries + + + org.apache.lucene + lucene-memory + + + org.apache.lucene + lucene-highlighter + + + org.apache.lucene + lucene-queryparser + + + org.apache.lucene + lucene-suggest + + + org.apache.lucene + lucene-join + + + org.apache.lucene + lucene-spatial + + + diff --git a/pom.xml b/pom.xml index a66f5ba5b43..8fc826bc7e5 100644 --- a/pom.xml +++ b/pom.xml @@ -1487,6 +1487,7 @@ org.eclipse.jdt.ui.text.custom_code_templates= dev-tools rest-api-spec + lucene core distribution plugins diff --git a/qa/smoke-test-shaded/pom.xml b/qa/smoke-test-shaded/pom.xml index 0b968e13d55..d0bb0ede85f 100644 --- a/qa/smoke-test-shaded/pom.xml +++ b/qa/smoke-test-shaded/pom.xml @@ -23,7 +23,19 @@ org.elasticsearch.distribution.shaded elasticsearch - 2.0.0-SNAPSHOT + ${elasticsearch.version} + + + org.elasticsearch + elasticsearch + + + + + org.elasticsearch + elasticsearch-lucene + ${elasticsearch.version} + pom org.hamcrest From 409de690207d2af79de3404b7561dc9932a787ed Mon Sep 17 00:00:00 2001 From: David Pilato Date: Thu, 13 Aug 2015 12:24:48 +0200 Subject: [PATCH 06/10] [build] elasticsearch-lucene module needs to depend on 2.0.0-SNAPSHOT --- lucene/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lucene/pom.xml b/lucene/pom.xml index e423d9e9a70..c2e4b8975d1 100644 --- a/lucene/pom.xml +++ b/lucene/pom.xml @@ -5,7 +5,7 @@ elasticsearch-parent org.elasticsearch - 2.0.0-beta1-SNAPSHOT + 2.0.0-SNAPSHOT 4.0.0 From 74f18d8c16ff4e8aa9505a354abf28c83fc21ad8 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Wed, 12 Aug 2015 12:02:58 +0200 Subject: [PATCH 07/10] Introduce a formal ExtensionPoint class to stream line extensions This commit tries to add some infrastructure to streamline how extension points should be strucutred. It's a simple approache with 4 implementations for `highlighter`, `suggester`, `allocation_decider` and `shards_allocator`. It simplifies adding new extension points and forces to register classes instead of strings. --- .../routing/allocation/AllocationModule.java | 42 ++-- .../common/util/ExtensionPoint.java | 194 ++++++++++++++++++ .../ScoreFunctionParserMapper.java | 1 - .../elasticsearch/search/SearchModule.java | 45 ++-- .../SignificanceHeuristicStreams.java | 3 + .../movavg/models/MovAvgModelStreams.java | 3 + .../highlight/FastVectorHighlighter.java | 5 - .../search/highlight/Highlighter.java | 2 - .../search/highlight/Highlighters.java | 74 +++++-- .../search/highlight/PlainHighlighter.java | 5 - .../search/highlight/PostingsHighlighter.java | 5 - .../search/suggest/Suggester.java | 2 - .../search/suggest/Suggesters.java | 45 ++-- .../completion/CompletionSuggester.java | 5 - .../suggest/phrase/PhraseSuggester.java | 5 - .../search/suggest/term/TermSuggester.java | 5 - .../allocation/AllocationModuleTests.java | 7 +- .../common/inject/ModuleTestCase.java | 31 +++ .../search/SearchModuleTests.java | 69 +++++++ .../search/highlight/CustomHighlighter.java | 5 - .../highlight/CustomHighlighterPlugin.java | 2 +- .../search/suggest/CustomSuggester.java | 5 - .../search/suggest/CustomSuggesterPlugin.java | 2 +- 23 files changed, 412 insertions(+), 150 deletions(-) create mode 100644 core/src/main/java/org/elasticsearch/common/util/ExtensionPoint.java create mode 100644 core/src/test/java/org/elasticsearch/search/SearchModuleTests.java diff --git a/core/src/main/java/org/elasticsearch/cluster/routing/allocation/AllocationModule.java b/core/src/main/java/org/elasticsearch/cluster/routing/allocation/AllocationModule.java index 23c721d187a..62e14ec0471 100644 --- a/core/src/main/java/org/elasticsearch/cluster/routing/allocation/AllocationModule.java +++ b/core/src/main/java/org/elasticsearch/cluster/routing/allocation/AllocationModule.java @@ -21,6 +21,7 @@ package org.elasticsearch.cluster.routing.allocation; import org.elasticsearch.cluster.routing.allocation.allocator.BalancedShardsAllocator; import org.elasticsearch.cluster.routing.allocation.allocator.ShardsAllocator; +import org.elasticsearch.cluster.routing.allocation.allocator.ShardsAllocators; import org.elasticsearch.cluster.routing.allocation.decider.AllocationDecider; import org.elasticsearch.cluster.routing.allocation.decider.AllocationDeciders; import org.elasticsearch.cluster.routing.allocation.decider.AwarenessAllocationDecider; @@ -42,6 +43,7 @@ import org.elasticsearch.common.inject.multibindings.Multibinder; import org.elasticsearch.common.logging.ESLogger; import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.ExtensionPoint; import org.elasticsearch.gateway.GatewayAllocator; import java.util.Arrays; @@ -84,55 +86,43 @@ public class AllocationModule extends AbstractModule { DiskThresholdDecider.class, SnapshotInProgressAllocationDecider.class)); + private final Settings settings; - private final Map> shardsAllocators = new HashMap<>(); - private final Set> allocationDeciders = new HashSet<>(); + private final ExtensionPoint.TypeExtensionPoint shardsAllocators = new ExtensionPoint.TypeExtensionPoint<>("shards_allocator", ShardsAllocator.class); + private final ExtensionPoint.SetExtensionPoint allocationDeciders = new ExtensionPoint.SetExtensionPoint<>("allocation_decider", AllocationDecider.class, AllocationDeciders.class); public AllocationModule(Settings settings) { this.settings = settings; - this.allocationDeciders.addAll(DEFAULT_ALLOCATION_DECIDERS); - registerShardAllocator(BALANCED_ALLOCATOR, BalancedShardsAllocator.class); - registerShardAllocator(EVEN_SHARD_COUNT_ALLOCATOR, BalancedShardsAllocator.class); + for (Class decider : DEFAULT_ALLOCATION_DECIDERS) { + allocationDeciders.registerExtension(decider); + } + shardsAllocators.registerExtension(BALANCED_ALLOCATOR, BalancedShardsAllocator.class); + shardsAllocators.registerExtension(EVEN_SHARD_COUNT_ALLOCATOR, BalancedShardsAllocator.class); } /** Register a custom allocation decider */ public void registerAllocationDecider(Class allocationDecider) { - boolean isNew = allocationDeciders.add(allocationDecider); - if (isNew == false) { - throw new IllegalArgumentException("Cannot register AllocationDecider " + allocationDecider.getName() + " twice"); - } + allocationDeciders.registerExtension(allocationDecider); } /** Register a custom shard allocator with the given name */ public void registerShardAllocator(String name, Class clazz) { - Class existing = shardsAllocators.put(name, clazz); - if (existing != null) { - throw new IllegalArgumentException("Cannot register ShardAllocator [" + name + "] to " + clazz.getName() + ", already registered to " + existing.getName()); - } + shardsAllocators.registerExtension(name, clazz); } @Override protected void configure() { - // bind ShardsAllocator - final String shardsAllocatorType = settings.get(AllocationModule.SHARDS_ALLOCATOR_TYPE_KEY, AllocationModule.BALANCED_ALLOCATOR); - final Class shardsAllocator = shardsAllocators.get(shardsAllocatorType); - if (shardsAllocator == null) { - throw new IllegalArgumentException("Unknown ShardsAllocator type [" + shardsAllocatorType + "]"); - } else if (shardsAllocatorType.equals(EVEN_SHARD_COUNT_ALLOCATOR)) { + String shardsAllocatorType = shardsAllocators.bindType(binder(), settings, AllocationModule.SHARDS_ALLOCATOR_TYPE_KEY, AllocationModule.BALANCED_ALLOCATOR); + if (shardsAllocatorType.equals(EVEN_SHARD_COUNT_ALLOCATOR)) { final ESLogger logger = Loggers.getLogger(getClass(), settings); logger.warn("{} allocator has been removed in 2.0 using {} instead", AllocationModule.EVEN_SHARD_COUNT_ALLOCATOR, AllocationModule.BALANCED_ALLOCATOR); } - bind(ShardsAllocator.class).to(shardsAllocator).asEagerSingleton(); - // bind AllocationDeciders - Multibinder allocationMultibinder = Multibinder.newSetBinder(binder(), AllocationDecider.class); - for (Class allocation : allocationDeciders) { - allocationMultibinder.addBinding().to(allocation).asEagerSingleton(); - } + allocationDeciders.bind(binder()); bind(GatewayAllocator.class).asEagerSingleton(); - bind(AllocationDeciders.class).asEagerSingleton(); bind(AllocationService.class).asEagerSingleton(); } + } diff --git a/core/src/main/java/org/elasticsearch/common/util/ExtensionPoint.java b/core/src/main/java/org/elasticsearch/common/util/ExtensionPoint.java new file mode 100644 index 00000000000..435c3ae4066 --- /dev/null +++ b/core/src/main/java/org/elasticsearch/common/util/ExtensionPoint.java @@ -0,0 +1,194 @@ +/* + * 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.common.util; + +import org.elasticsearch.common.inject.Binder; +import org.elasticsearch.common.inject.multibindings.MapBinder; +import org.elasticsearch.common.inject.multibindings.Multibinder; +import org.elasticsearch.common.settings.Settings; + +import java.util.*; + +/** + * This class defines an official elasticsearch extension point. It registers + * all extensions by a single name and ensures that extensions are not registered + * more than once. + */ +public abstract class ExtensionPoint { + protected final String name; + protected final Class extensionClass; + protected final Class[] singletons; + + /** + * Creates a new extension point + * + * @param name the human readable underscore case name of the extension poing. This is used in error messages etc. + * @param extensionClass the base class that should be extended + * @param singletons a list of singletons to bind with this extension point - these are bound in {@link #bind(Binder)} + */ + public ExtensionPoint(String name, Class extensionClass, Class... singletons) { + this.name = name; + this.extensionClass = extensionClass; + this.singletons = singletons; + } + + /** + * Binds the extension as well as the singletons to the given guice binder. + * + * @param binder the binder to use + */ + public final void bind(Binder binder) { + if (singletons == null || singletons.length == 0) { + throw new IllegalStateException("Can't bind empty or null singletons"); + } + for (Class c : singletons) { + binder.bind(c).asEagerSingleton(); + } + bindExtensions(binder); + } + + /** + * Subclasses can bind their type, map or set exentions here. + */ + protected abstract void bindExtensions(Binder binder); + + /** + * A map based extension point which allows to register keyed implementations ie. parsers or some kind of strategies. + */ + public static class MapExtensionPoint extends ExtensionPoint { + private final Map> extensions = new HashMap<>(); + private final Set reservedKeys; + + /** + * Creates a new {@link org.elasticsearch.common.util.ExtensionPoint.MapExtensionPoint} + * + * @param name the human readable underscore case name of the extension poing. This is used in error messages etc. + * @param extensionClass the base class that should be extended + * @param singletons a list of singletons to bind with this extension point - these are bound in {@link #bind(Binder)} + * @param reservedKeys a set of reserved keys by internal implementations + */ + public MapExtensionPoint(String name, Class extensionClass, Set reservedKeys, Class... singletons) { + super(name, extensionClass, singletons); + this.reservedKeys = reservedKeys; + + } + + /** + * Returns the extension for the given key or null + */ + public Class getExtension(String type) { + return extensions.get(type); + } + + /** + * Registers an extension class for a given key. This method will thr + * + * @param key the extensions key + * @param extension the extension + * @throws IllegalArgumentException iff the key is already registered or if the key is a reserved key for an internal implementation + */ + public final void registerExtension(String key, Class extension) { + if (extensions.containsKey(key) || reservedKeys.contains(key)) { + throw new IllegalArgumentException("Can't register the same [" + this.name + "] more than once for [" + key + "]"); + } + extensions.put(key, extension); + } + + @Override + protected final void bindExtensions(Binder binder) { + MapBinder parserMapBinder = MapBinder.newMapBinder(binder, String.class, extensionClass); + for (Map.Entry> clazz : extensions.entrySet()) { + parserMapBinder.addBinding(clazz.getKey()).to(clazz.getValue()); + } + } + } + + /** + * A Type extension point which basically allows to registerd keyed extensions like {@link org.elasticsearch.common.util.ExtensionPoint.MapExtensionPoint} + * but doesn't instantiate and bind all the registered key value pairs but instead replace a singleton based on a given setting via {@link #bindType(Binder, Settings, String, String)} + * Note: {@link #bind(Binder)} is not supported by this class + */ + public static final class TypeExtensionPoint extends MapExtensionPoint { + + public TypeExtensionPoint(String name, Class extensionClass) { + super(name, extensionClass, Collections.EMPTY_SET); + } + + /** + * Binds the extension class to the class that is registered for the give configured for the settings key in + * the settings object. + * + * @param binder the binder to use + * @param settings the settings to look up the key to find the implemetation to bind + * @param settingsKey the key to use with the settings + * @param defaultValue the default value if they settings doesn't contain the key + * @return the actual bound type key + */ + public String bindType(Binder binder, Settings settings, String settingsKey, String defaultValue) { + final String type = settings.get(settingsKey, defaultValue); + final Class instance = getExtension(type); + if (instance == null) { + throw new IllegalArgumentException("Unknown [" + this.name + "] type [" + type + "]"); + } + binder.bind(extensionClass).to(instance).asEagerSingleton(); + return type; + } + + } + + /** + * A set based extension point which allows to register extended classes that might be used to chain additional functionality etc. + */ + public final static class SetExtensionPoint extends ExtensionPoint { + private final Set> extensions = new HashSet<>(); + + /** + * Creates a new {@link org.elasticsearch.common.util.ExtensionPoint.SetExtensionPoint} + * + * @param name the human readable underscore case name of the extension poing. This is used in error messages etc. + * @param extensionClass the base class that should be extended + * @param singletons a list of singletons to bind with this extension point - these are bound in {@link #bind(Binder)} + */ + public SetExtensionPoint(String name, Class extensionClass, Class... singletons) { + super(name, extensionClass, singletons); + } + + /** + * Registers a new extension + * + * @param extension the extension to register + * @throws IllegalArgumentException iff the class is already registered + */ + public final void registerExtension(Class extension) { + if (extensions.contains(extension)) { + throw new IllegalArgumentException("Can't register the same [" + this.name + "] more than once for [" + extension.getName() + "]"); + } + extensions.add(extension); + } + + @Override + protected final void bindExtensions(Binder binder) { + Multibinder allocationMultibinder = Multibinder.newSetBinder(binder, extensionClass); + for (Class clazz : extensions) { + allocationMultibinder.addBinding().to(clazz); + } + } + } +} diff --git a/core/src/main/java/org/elasticsearch/index/query/functionscore/ScoreFunctionParserMapper.java b/core/src/main/java/org/elasticsearch/index/query/functionscore/ScoreFunctionParserMapper.java index 4fd5233889a..837837a92b2 100644 --- a/core/src/main/java/org/elasticsearch/index/query/functionscore/ScoreFunctionParserMapper.java +++ b/core/src/main/java/org/elasticsearch/index/query/functionscore/ScoreFunctionParserMapper.java @@ -19,7 +19,6 @@ package org.elasticsearch.index.query.functionscore; -import org.elasticsearch.common.Nullable; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.index.query.QueryParsingException; diff --git a/core/src/main/java/org/elasticsearch/search/SearchModule.java b/core/src/main/java/org/elasticsearch/search/SearchModule.java index b4d15b35d16..fcebb12485e 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchModule.java +++ b/core/src/main/java/org/elasticsearch/search/SearchModule.java @@ -19,8 +19,6 @@ package org.elasticsearch.search; -import com.google.common.collect.Lists; - import org.elasticsearch.common.Classes; import org.elasticsearch.common.inject.AbstractModule; import org.elasticsearch.common.inject.multibindings.Multibinder; @@ -150,7 +148,7 @@ import org.elasticsearch.search.suggest.SuggestPhase; import org.elasticsearch.search.suggest.Suggester; import org.elasticsearch.search.suggest.Suggesters; -import java.util.List; +import java.util.*; /** * @@ -160,14 +158,14 @@ public class SearchModule extends AbstractModule { public static final String SEARCH_SERVICE_IMPL = "search.service_impl"; private final Settings settings; - private final List> aggParsers = Lists.newArrayList(); - private final List> pipelineAggParsers = Lists.newArrayList(); - private final List> highlighters = Lists.newArrayList(); - private final List> suggesters = Lists.newArrayList(); - private final List> functionScoreParsers = Lists.newArrayList(); - private final List> fetchSubPhases = Lists.newArrayList(); - private final List> heuristicParsers = Lists.newArrayList(); - private final List> modelParsers = Lists.newArrayList(); + private final Set> aggParsers = new HashSet<>(); + private final Set> pipelineAggParsers = new HashSet<>(); + private final Highlighters highlighters = new Highlighters(); + private final Suggesters suggesters = new Suggesters(); + private final Set> functionScoreParsers = new HashSet<>(); + private final Set> fetchSubPhases = new HashSet<>(); + private final Set> heuristicParsers = new HashSet<>(); + private final Set> modelParsers = new HashSet<>(); public SearchModule(Settings settings) { this.settings = settings; @@ -182,12 +180,12 @@ public class SearchModule extends AbstractModule { MovAvgModelStreams.registerStream(stream); } - public void registerHighlighter(Class clazz) { - highlighters.add(clazz); + public void registerHighlighter(String key, Class clazz) { + highlighters.registerExtension(key, clazz); } - public void registerSuggester(Class suggester) { - suggesters.add(suggester); + public void registerSuggester(String key, Class suggester) { + suggesters.registerExtension(key, suggester); } public void registerFunctionScoreParser(Class parser) { @@ -245,14 +243,7 @@ public class SearchModule extends AbstractModule { } protected void configureSuggesters() { - Multibinder suggesterMultibinder = Multibinder.newSetBinder(binder(), Suggester.class); - for (Class clazz : suggesters) { - suggesterMultibinder.addBinding().to(clazz); - } - - bind(SuggestParseElement.class).asEagerSingleton(); - bind(SuggestPhase.class).asEagerSingleton(); - bind(Suggesters.class).asEagerSingleton(); + suggesters.bind(binder()); } protected void configureFunctionScore() { @@ -264,11 +255,7 @@ public class SearchModule extends AbstractModule { } protected void configureHighlighters() { - Multibinder multibinder = Multibinder.newSetBinder(binder(), Highlighter.class); - for (Class highlighter : highlighters) { - multibinder.addBinding().to(highlighter); - } - bind(Highlighters.class).asEagerSingleton(); + highlighters.bind(binder()); } protected void configureAggs() { @@ -346,7 +333,6 @@ public class SearchModule extends AbstractModule { bind(FetchPhase.class).asEagerSingleton(); bind(SearchServiceTransportAction.class).asEagerSingleton(); bind(MoreLikeThisFetchService.class).asEagerSingleton(); - // search service -- testing only! String impl = settings.get(SEARCH_SERVICE_IMPL); if (impl == null) { @@ -414,4 +400,5 @@ public class SearchModule extends AbstractModule { BucketSelectorPipelineAggregator.registerStreams(); SerialDiffPipelineAggregator.registerStreams(); } + } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/SignificanceHeuristicStreams.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/SignificanceHeuristicStreams.java index 51dd11c5e87..18a6f6b93dd 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/SignificanceHeuristicStreams.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/SignificanceHeuristicStreams.java @@ -64,6 +64,9 @@ public class SignificanceHeuristicStreams { * @param stream The stream to register */ public static synchronized void registerStream(Stream stream) { + if (STREAMS.containsKey(stream.getName())) { + throw new IllegalArgumentException("Can't register stream with name [" + stream.getName() + "] more than once"); + } HashMap map = new HashMap<>(); map.putAll(STREAMS); map.put(stream.getName(), stream); diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/MovAvgModelStreams.java b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/MovAvgModelStreams.java index d12b0cdfef4..faee8a9f75b 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/MovAvgModelStreams.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/MovAvgModelStreams.java @@ -64,6 +64,9 @@ public class MovAvgModelStreams { * @param stream The stream to register */ public static synchronized void registerStream(Stream stream) { + if (STREAMS.containsKey(stream.getName())) { + throw new IllegalArgumentException("Can't register stream with name [" + stream.getName() + "] more than once"); + } HashMap map = new HashMap<>(); map.putAll(STREAMS); map.put(stream.getName(), stream); diff --git a/core/src/main/java/org/elasticsearch/search/highlight/FastVectorHighlighter.java b/core/src/main/java/org/elasticsearch/search/highlight/FastVectorHighlighter.java index 67b42a5d866..de73a898334 100644 --- a/core/src/main/java/org/elasticsearch/search/highlight/FastVectorHighlighter.java +++ b/core/src/main/java/org/elasticsearch/search/highlight/FastVectorHighlighter.java @@ -49,11 +49,6 @@ public class FastVectorHighlighter implements Highlighter { this.termVectorMultiValue = settings.getAsBoolean("search.highlight.term_vector_multi_value", true); } - @Override - public String[] names() { - return new String[]{"fvh", "fast-vector-highlighter"}; - } - @Override public HighlightField highlight(HighlighterContext highlighterContext) { SearchContextHighlight.Field field = highlighterContext.field; diff --git a/core/src/main/java/org/elasticsearch/search/highlight/Highlighter.java b/core/src/main/java/org/elasticsearch/search/highlight/Highlighter.java index 26c3dc0bf21..af4801f3633 100644 --- a/core/src/main/java/org/elasticsearch/search/highlight/Highlighter.java +++ b/core/src/main/java/org/elasticsearch/search/highlight/Highlighter.java @@ -25,8 +25,6 @@ import org.elasticsearch.index.mapper.FieldMapper; */ public interface Highlighter { - String[] names(); - HighlightField highlight(HighlighterContext highlighterContext); boolean canHighlight(FieldMapper fieldMapper); diff --git a/core/src/main/java/org/elasticsearch/search/highlight/Highlighters.java b/core/src/main/java/org/elasticsearch/search/highlight/Highlighters.java index 9f14b0f7ed1..349227f5c57 100644 --- a/core/src/main/java/org/elasticsearch/search/highlight/Highlighters.java +++ b/core/src/main/java/org/elasticsearch/search/highlight/Highlighters.java @@ -18,44 +18,74 @@ */ package org.elasticsearch.search.highlight; -import com.google.common.collect.ImmutableMap; -import org.elasticsearch.common.collect.MapBuilder; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.logging.DeprecationLogger; +import org.elasticsearch.common.logging.ESLoggerFactory; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.ExtensionPoint; -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; -import java.util.Set; +import java.util.*; /** - * + * An extensions point and registry for all the highlighters a node supports. */ -public class Highlighters { +public class Highlighters extends ExtensionPoint.MapExtensionPoint { + + @Deprecated // remove in 3.0 + private static final String FAST_VECTOR_HIGHLIGHTER = "fast-vector-highlighter"; + private static final String FVH = "fvh"; + @Deprecated // remove in 3.0 + private static final String HIGHLIGHTER = "highlighter"; + private static final String PLAIN = "plain"; + @Deprecated // remove in 3.0 + private static final String POSTINGS_HIGHLIGHTER = "postings-highlighter"; + private static final String POSTINGS = "postings"; + private final Map parsers; + private final DeprecationLogger deprecationLogger = new DeprecationLogger(ESLoggerFactory.getLogger(Highlighters.class.getName())); + + public Highlighters(){ + this(Collections.EMPTY_MAP); + } + + private Highlighters(Map parsers) { + super("highlighter", Highlighter.class, new HashSet<>(Arrays.asList(FVH, FAST_VECTOR_HIGHLIGHTER, PLAIN, HIGHLIGHTER, POSTINGS, POSTINGS_HIGHLIGHTER)), + Highlighters.class); + this.parsers = Collections.unmodifiableMap(parsers); + } @Inject - public Highlighters(Settings settings, Set parsers) { + public Highlighters(Settings settings, Map parsers) { + this(addBuiltIns(settings, parsers)); + } + + private static Map addBuiltIns(Settings settings, Map parsers) { // build in highlighers Map map = new HashMap<>(); - add(map, new FastVectorHighlighter(settings)); - add(map, new PlainHighlighter()); - add(map, new PostingsHighlighter()); - for (Highlighter highlighter : parsers) { - add(map, highlighter); - } - this.parsers = Collections.unmodifiableMap(map); + map.put(FVH, new FastVectorHighlighter(settings)); + map.put(FAST_VECTOR_HIGHLIGHTER, map.get(FVH)); + map.put(PLAIN, new PlainHighlighter()); + map.put(HIGHLIGHTER, map.get(PLAIN)); + map.put(POSTINGS, new PostingsHighlighter()); + map.put(POSTINGS_HIGHLIGHTER, map.get(POSTINGS)); + map.putAll(parsers); + return map; } public Highlighter get(String type) { + switch (type) { + case FAST_VECTOR_HIGHLIGHTER: + deprecationLogger.deprecated("highlighter key [{}] is deprecated and will be removed in 3.x use [{}] instead", FAST_VECTOR_HIGHLIGHTER, FVH); + break; + case HIGHLIGHTER: + deprecationLogger.deprecated("highlighter key [{}] is deprecated and will be removed in 3.x use [{}] instead", HIGHLIGHTER, PLAIN); + break; + case POSTINGS_HIGHLIGHTER: + deprecationLogger.deprecated("highlighter key [{}] is deprecated and will be removed in 3.x use [{}] instead", POSTINGS_HIGHLIGHTER, POSTINGS); + break; + } return parsers.get(type); } - private void add(Map map, Highlighter highlighter) { - for (String type : highlighter.names()) { - map.put(type, highlighter); - } - } - } diff --git a/core/src/main/java/org/elasticsearch/search/highlight/PlainHighlighter.java b/core/src/main/java/org/elasticsearch/search/highlight/PlainHighlighter.java index a9094f9ceaf..27f439a5c33 100644 --- a/core/src/main/java/org/elasticsearch/search/highlight/PlainHighlighter.java +++ b/core/src/main/java/org/elasticsearch/search/highlight/PlainHighlighter.java @@ -47,11 +47,6 @@ public class PlainHighlighter implements Highlighter { private static final String CACHE_KEY = "highlight-plain"; - @Override - public String[] names() { - return new String[] { "plain", "highlighter" }; - } - @Override public HighlightField highlight(HighlighterContext highlighterContext) { SearchContextHighlight.Field field = highlighterContext.field; diff --git a/core/src/main/java/org/elasticsearch/search/highlight/PostingsHighlighter.java b/core/src/main/java/org/elasticsearch/search/highlight/PostingsHighlighter.java index 35f6560899e..270401a9108 100644 --- a/core/src/main/java/org/elasticsearch/search/highlight/PostingsHighlighter.java +++ b/core/src/main/java/org/elasticsearch/search/highlight/PostingsHighlighter.java @@ -40,11 +40,6 @@ public class PostingsHighlighter implements Highlighter { private static final String CACHE_KEY = "highlight-postings"; - @Override - public String[] names() { - return new String[]{"postings", "postings-highlighter"}; - } - @Override public HighlightField highlight(HighlighterContext highlighterContext) { diff --git a/core/src/main/java/org/elasticsearch/search/suggest/Suggester.java b/core/src/main/java/org/elasticsearch/search/suggest/Suggester.java index 51f5f21b460..7b3f7bdb89f 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/Suggester.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/Suggester.java @@ -29,8 +29,6 @@ public abstract class Suggester> innerExecute(String name, T suggestion, IndexSearcher searcher, CharsRefBuilder spare) throws IOException; - public abstract String[] names(); - public abstract SuggestContextParser getContextParser(); public Suggest.Suggestion> diff --git a/core/src/main/java/org/elasticsearch/search/suggest/Suggesters.java b/core/src/main/java/org/elasticsearch/search/suggest/Suggesters.java index 264720b8b90..1be80b57502 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/Suggesters.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/Suggesters.java @@ -18,45 +18,46 @@ */ package org.elasticsearch.search.suggest; -import com.google.common.collect.ImmutableMap; -import org.elasticsearch.common.collect.MapBuilder; +import org.elasticsearch.common.inject.Binder; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.util.ExtensionPoint; import org.elasticsearch.script.ScriptService; -import org.elasticsearch.search.aggregations.bucket.significant.heuristics.SignificanceHeuristicParser; import org.elasticsearch.search.suggest.completion.CompletionSuggester; import org.elasticsearch.search.suggest.phrase.PhraseSuggester; import org.elasticsearch.search.suggest.term.TermSuggester; -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; -import java.util.Set; +import java.util.*; /** * */ -public class Suggesters { +public final class Suggesters extends ExtensionPoint.MapExtensionPoint { private final Map parsers; + public Suggesters() { + this(Collections.EMPTY_MAP); + } + + public Suggesters(Map suggesters) { + super("suggester", Suggester.class, new HashSet<>(Arrays.asList("phrase", "term", "completion")), Suggesters.class, SuggestParseElement.class, SuggestPhase.class); + this.parsers = Collections.unmodifiableMap(suggesters); + } + @Inject - public Suggesters(Set suggesters, ScriptService scriptService) { + public Suggesters(Map suggesters, ScriptService scriptService) { + this(addBuildIns(suggesters, scriptService)); + } + + private static Map addBuildIns(Map suggesters, ScriptService scriptService) { final Map map = new HashMap<>(); - add(map, new PhraseSuggester(scriptService)); - add(map, new TermSuggester()); - add(map, new CompletionSuggester()); - for (Suggester suggester : suggesters) { - add(map, suggester); - } - this.parsers = Collections.unmodifiableMap(map); + map.put("phrase", new PhraseSuggester(scriptService)); + map.put("term", new TermSuggester()); + map.put("completion", new CompletionSuggester()); + map.putAll(suggesters); + return map; } public Suggester get(String type) { return parsers.get(type); } - - private void add(Map map, Suggester suggester) { - for (String type : suggester.names()) { - map.put(type, suggester); - } - } } diff --git a/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggester.java b/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggester.java index 4af360fa05f..4a1d5d1d28c 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggester.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggester.java @@ -101,11 +101,6 @@ public class CompletionSuggester extends Suggester return completionSuggestion; } - @Override - public String[] names() { - return new String[] { "completion" }; - } - @Override public SuggestContextParser getContextParser() { return new CompletionSuggestParser(this); diff --git a/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggester.java b/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggester.java index 30c1b63de21..e7d0eb378c3 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggester.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggester.java @@ -150,11 +150,6 @@ public final class PhraseSuggester extends Suggester { return scriptService; } - @Override - public String[] names() { - return new String[] {"phrase"}; - } - @Override public SuggestContextParser getContextParser() { return new PhraseSuggestParser(this); diff --git a/core/src/main/java/org/elasticsearch/search/suggest/term/TermSuggester.java b/core/src/main/java/org/elasticsearch/search/suggest/term/TermSuggester.java index 70dfefe9522..4c1b176c990 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/term/TermSuggester.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/term/TermSuggester.java @@ -65,11 +65,6 @@ public final class TermSuggester extends Suggester { return response; } - @Override - public String[] names() { - return new String[] {"term"}; - } - @Override public SuggestContextParser getContextParser() { return new TermSuggestParser(this); diff --git a/core/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationModuleTests.java b/core/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationModuleTests.java index 678bfa3b0e5..7b57ef07190 100644 --- a/core/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationModuleTests.java +++ b/core/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationModuleTests.java @@ -59,8 +59,7 @@ public class AllocationModuleTests extends ModuleTestCase { try { module.registerAllocationDecider(EnableAllocationDecider.class); } catch (IllegalArgumentException e) { - assertTrue(e.getMessage().contains("Cannot register AllocationDecider")); - assertTrue(e.getMessage().contains("twice")); + assertEquals(e.getMessage(), "Can't register the same [allocation_decider] more than once for [" + EnableAllocationDecider.class.getName() + "]"); } } @@ -82,14 +81,14 @@ public class AllocationModuleTests extends ModuleTestCase { try { module.registerShardAllocator(AllocationModule.BALANCED_ALLOCATOR, FakeShardsAllocator.class); } catch (IllegalArgumentException e) { - assertTrue(e.getMessage().contains("already registered")); + assertEquals(e.getMessage(), "Can't register the same [shards_allocator] more than once for [balanced]"); } } public void testUnknownShardsAllocator() { Settings settings = Settings.builder().put(AllocationModule.SHARDS_ALLOCATOR_TYPE_KEY, "dne").build(); AllocationModule module = new AllocationModule(settings); - assertBindingFailure(module, "Unknown ShardsAllocator"); + assertBindingFailure(module, "Unknown [shards_allocator]"); } public void testEvenShardsAllocatorBackcompat() { diff --git a/core/src/test/java/org/elasticsearch/common/inject/ModuleTestCase.java b/core/src/test/java/org/elasticsearch/common/inject/ModuleTestCase.java index d96b89d382c..60c3ca126d5 100644 --- a/core/src/test/java/org/elasticsearch/common/inject/ModuleTestCase.java +++ b/core/src/test/java/org/elasticsearch/common/inject/ModuleTestCase.java @@ -72,6 +72,37 @@ public abstract class ModuleTestCase extends ESTestCase { } } + /** + * Configures the module and checks a Map of the "to" class + * is bound to "theClas". + */ + public void assertMapMultiBinding(Module module, Class to, Class theClass) { + List elements = Elements.getElements(module); + Set bindings = new HashSet<>(); + boolean providerFound = false; + for (Element element : elements) { + if (element instanceof LinkedKeyBinding) { + LinkedKeyBinding binding = (LinkedKeyBinding)element; + if (to.equals(binding.getKey().getTypeLiteral().getType())) { + bindings.add(binding.getLinkedKey().getTypeLiteral().getType()); + } + } else if (element instanceof ProviderInstanceBinding) { + ProviderInstanceBinding binding = (ProviderInstanceBinding)element; + String setType = binding.getKey().getTypeLiteral().getType().toString(); + if (setType.equals("java.util.Map")) { + providerFound = true; + } + } + } + + if (bindings.contains(theClass) == false) { + fail("Expected to find " + theClass.getName() + " as binding to " + to.getName() + ", found these classes:\n" + bindings); + } + assertTrue("Did not find provider for map of " + to.getName(), providerFound); + } + + + /** * Configures the module and checks a Set of the "to" class * is bound to "classes". There may be more classes bound diff --git a/core/src/test/java/org/elasticsearch/search/SearchModuleTests.java b/core/src/test/java/org/elasticsearch/search/SearchModuleTests.java new file mode 100644 index 00000000000..efdcf0062c3 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/search/SearchModuleTests.java @@ -0,0 +1,69 @@ +/* + * 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.search; + +import org.elasticsearch.common.inject.ModuleTestCase; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.search.highlight.CustomHighlighter; +import org.elasticsearch.search.highlight.Highlighter; +import org.elasticsearch.search.highlight.PlainHighlighter; +import org.elasticsearch.search.suggest.CustomSuggester; +import org.elasticsearch.search.suggest.Suggester; +import org.elasticsearch.search.suggest.phrase.PhraseSuggester; +/** + */ +public class SearchModuleTests extends ModuleTestCase { + + public void testDoubleRegister() { + SearchModule module = new SearchModule(Settings.EMPTY); + try { + module.registerHighlighter("fvh", PlainHighlighter.class); + } catch (IllegalArgumentException e) { + assertEquals(e.getMessage(), "Can't register the same [highlighter] more than once for [fvh]"); + } + + try { + module.registerSuggester("term", PhraseSuggester.class); + } catch (IllegalArgumentException e) { + assertEquals(e.getMessage(), "Can't register the same [suggester] more than once for [term]"); + } + } + + public void testRegisterSuggester() { + SearchModule module = new SearchModule(Settings.EMPTY); + module.registerSuggester("custom", CustomSuggester.class); + try { + module.registerSuggester("custom", CustomSuggester.class); + } catch (IllegalArgumentException e) { + assertEquals(e.getMessage(), "Can't register the same [suggester] more than once for [custom]"); + } + assertMapMultiBinding(module, Suggester.class, CustomSuggester.class); + } + + public void testRegisterHighlighter() { + SearchModule module = new SearchModule(Settings.EMPTY); + module.registerHighlighter("custom", CustomHighlighter.class); + try { + module.registerHighlighter("custom", CustomHighlighter.class); + } catch (IllegalArgumentException e) { + assertEquals(e.getMessage(), "Can't register the same [highlighter] more than once for [custom]"); + } + assertMapMultiBinding(module, Highlighter.class, CustomHighlighter.class); + } +} diff --git a/core/src/test/java/org/elasticsearch/search/highlight/CustomHighlighter.java b/core/src/test/java/org/elasticsearch/search/highlight/CustomHighlighter.java index 3a9135cb731..e193d2ad69b 100644 --- a/core/src/test/java/org/elasticsearch/search/highlight/CustomHighlighter.java +++ b/core/src/test/java/org/elasticsearch/search/highlight/CustomHighlighter.java @@ -32,11 +32,6 @@ import java.util.Map; */ public class CustomHighlighter implements Highlighter { - @Override - public String[] names() { - return new String[] { "test-custom" }; - } - @Override public HighlightField highlight(HighlighterContext highlighterContext) { SearchContextHighlight.Field field = highlighterContext.field; diff --git a/core/src/test/java/org/elasticsearch/search/highlight/CustomHighlighterPlugin.java b/core/src/test/java/org/elasticsearch/search/highlight/CustomHighlighterPlugin.java index e7c69793c2e..705265ea5f6 100644 --- a/core/src/test/java/org/elasticsearch/search/highlight/CustomHighlighterPlugin.java +++ b/core/src/test/java/org/elasticsearch/search/highlight/CustomHighlighterPlugin.java @@ -35,6 +35,6 @@ public class CustomHighlighterPlugin extends AbstractPlugin { } public void onModule(SearchModule highlightModule) { - highlightModule.registerHighlighter(CustomHighlighter.class); + highlightModule.registerHighlighter("test-custom", CustomHighlighter.class); } } diff --git a/core/src/test/java/org/elasticsearch/search/suggest/CustomSuggester.java b/core/src/test/java/org/elasticsearch/search/suggest/CustomSuggester.java index 6e57390a165..e3dfe3b96d3 100644 --- a/core/src/test/java/org/elasticsearch/search/suggest/CustomSuggester.java +++ b/core/src/test/java/org/elasticsearch/search/suggest/CustomSuggester.java @@ -55,11 +55,6 @@ public class CustomSuggester extends Suggester Date: Thu, 13 Aug 2015 14:39:10 +0200 Subject: [PATCH 08/10] Tests: Fix SimpleIcuAnalysisTests to not load a non-existent configuration file. --- .../elasticsearch/index/analysis/SimpleIcuAnalysisTests.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/analysis-icu/src/test/java/org/elasticsearch/index/analysis/SimpleIcuAnalysisTests.java b/plugins/analysis-icu/src/test/java/org/elasticsearch/index/analysis/SimpleIcuAnalysisTests.java index 3516a1342d9..8369809aee2 100644 --- a/plugins/analysis-icu/src/test/java/org/elasticsearch/index/analysis/SimpleIcuAnalysisTests.java +++ b/plugins/analysis-icu/src/test/java/org/elasticsearch/index/analysis/SimpleIcuAnalysisTests.java @@ -33,8 +33,7 @@ public class SimpleIcuAnalysisTests extends ESTestCase { @Test public void testDefaultsIcuAnalysis() { Settings settings = settingsBuilder() - .put("path.home", createTempDir()) - .loadFromClasspath("org/elasticsearch/index/analysis/phonetic-1.yml").build(); + .put("path.home", createTempDir()).build(); AnalysisService analysisService = createAnalysisService(settings); TokenizerFactory tokenizerFactory = analysisService.tokenizer("icu_tokenizer"); From 4e23fe265748f3e86aed0439e1b0313af559267a Mon Sep 17 00:00:00 2001 From: David Pilato Date: Thu, 13 Aug 2015 15:21:06 +0200 Subject: [PATCH 09/10] [build] revert maven 3.3.x fix We need to revert #12803 as it creates some troubles in IntelliJ and also might fail when using older elasticsearch versions. --- core/pom.xml | 42 +++++++++++++++++++++--- distribution/shaded/pom.xml | 1 - lucene/pom.xml | 62 ------------------------------------ pom.xml | 1 - qa/smoke-test-shaded/pom.xml | 12 ------- 5 files changed, 38 insertions(+), 80 deletions(-) delete mode 100644 lucene/pom.xml diff --git a/core/pom.xml b/core/pom.xml index 81b6498a8d7..8782a9ee95e 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -44,10 +44,44 @@ - org.elasticsearch - elasticsearch-lucene - ${elasticsearch.version} - pom + org.apache.lucene + lucene-core + + + org.apache.lucene + lucene-backward-codecs + + + org.apache.lucene + lucene-analyzers-common + + + org.apache.lucene + lucene-queries + + + org.apache.lucene + lucene-memory + + + org.apache.lucene + lucene-highlighter + + + org.apache.lucene + lucene-queryparser + + + org.apache.lucene + lucene-suggest + + + org.apache.lucene + lucene-join + + + org.apache.lucene + lucene-spatial org.apache.lucene diff --git a/distribution/shaded/pom.xml b/distribution/shaded/pom.xml index 2302d1db2a6..7024504a7ec 100644 --- a/distribution/shaded/pom.xml +++ b/distribution/shaded/pom.xml @@ -77,7 +77,6 @@ ${project.build.directory}/dependency-reduced-pom.xml - org.elasticsearch:elasticsearch-lucene org.apache.lucene:* com.spatial4j:* diff --git a/lucene/pom.xml b/lucene/pom.xml deleted file mode 100644 index c2e4b8975d1..00000000000 --- a/lucene/pom.xml +++ /dev/null @@ -1,62 +0,0 @@ - - - - elasticsearch-parent - org.elasticsearch - 2.0.0-SNAPSHOT - - 4.0.0 - - elasticsearch-lucene - Lucene dependencies for elasticsearch - pom - - - true - - - - - org.apache.lucene - lucene-core - - - org.apache.lucene - lucene-backward-codecs - - - org.apache.lucene - lucene-analyzers-common - - - org.apache.lucene - lucene-queries - - - org.apache.lucene - lucene-memory - - - org.apache.lucene - lucene-highlighter - - - org.apache.lucene - lucene-queryparser - - - org.apache.lucene - lucene-suggest - - - org.apache.lucene - lucene-join - - - org.apache.lucene - lucene-spatial - - - diff --git a/pom.xml b/pom.xml index 8fc826bc7e5..a66f5ba5b43 100644 --- a/pom.xml +++ b/pom.xml @@ -1487,7 +1487,6 @@ org.eclipse.jdt.ui.text.custom_code_templates= dev-tools rest-api-spec - lucene core distribution plugins diff --git a/qa/smoke-test-shaded/pom.xml b/qa/smoke-test-shaded/pom.xml index d0bb0ede85f..711259e6dba 100644 --- a/qa/smoke-test-shaded/pom.xml +++ b/qa/smoke-test-shaded/pom.xml @@ -24,18 +24,6 @@ org.elasticsearch.distribution.shaded elasticsearch ${elasticsearch.version} - - - org.elasticsearch - elasticsearch - - - - - org.elasticsearch - elasticsearch-lucene - ${elasticsearch.version} - pom org.hamcrest From 1f41a8c682fffee8b7ef56018c47e7ef247343da Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Thu, 13 Aug 2015 22:21:46 +0200 Subject: [PATCH 10/10] Don't cache percolator query on loading percolators No need to load catch this query since it's cheap and not reused. If we cache it, it can cause assertions to be tripped since this method is executed during postRecovery phase and might still run while nodes are shutdown in tests. --- .../index/percolator/PercolatorQueriesRegistry.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/elasticsearch/index/percolator/PercolatorQueriesRegistry.java b/core/src/main/java/org/elasticsearch/index/percolator/PercolatorQueriesRegistry.java index 3a230d1267f..91ff1de78e7 100644 --- a/core/src/main/java/org/elasticsearch/index/percolator/PercolatorQueriesRegistry.java +++ b/core/src/main/java/org/elasticsearch/index/percolator/PercolatorQueriesRegistry.java @@ -20,6 +20,7 @@ package org.elasticsearch.index.percolator; import org.apache.lucene.index.Term; +import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.Query; import org.apache.lucene.search.TermQuery; import org.apache.lucene.util.BytesRef; @@ -260,7 +261,9 @@ public class PercolatorQueriesRegistry extends AbstractIndexShardComponent imple try (Engine.Searcher searcher = shard.engine().acquireSearcher("percolator_load_queries")) { Query query = new TermQuery(new Term(TypeFieldMapper.NAME, PercolatorService.TYPE_NAME)); QueriesLoaderCollector queryCollector = new QueriesLoaderCollector(PercolatorQueriesRegistry.this, logger, mapperService, indexFieldDataService); - searcher.searcher().search(query, queryCollector); + IndexSearcher indexSearcher = new IndexSearcher(searcher.reader()); + indexSearcher.setQueryCache(null); + indexSearcher.search(query, queryCollector); Map queries = queryCollector.queries(); for (Map.Entry entry : queries.entrySet()) { Query previousQuery = percolateQueries.put(entry.getKey(), entry.getValue());