From 6343ec3d3e2eaeff945011f3ba5f2756d9974fa3 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 8 May 2019 12:09:02 -0700 Subject: [PATCH 1/7] Update lintian overrides (#41561) (#41953) The deb package has been updated several times in the past to contain overrides in order to pass lintian inspection. However, there have never been any tests to ensure we do not fallback to failure. This commit updates the overrides file given things that have changed since 2.x like adding ML and bundling the jdk. closes #17185 --- Vagrantfile | 6 +- distribution/packages/build.gradle | 17 ++++-- .../packages/src/deb/lintian/elasticsearch | 56 ++++++++++++++++--- .../packaging/test/PackageTestCase.java | 6 ++ 4 files changed, 71 insertions(+), 14 deletions(-) diff --git a/Vagrantfile b/Vagrantfile index d5364348596..5e27e285487 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -161,13 +161,17 @@ def deb_common(config, name, extra: '') s.privileged = false s.inline = "sudo sed -i '/tty/!s/mesg n/tty -s \\&\\& mesg n/' /root/.profile" end + extra_with_lintian = <<-SHELL + install lintian + #{extra} + SHELL linux_common( config, name, update_command: 'apt-get update', update_tracking_file: '/var/cache/apt/archives/last_update', install_command: 'apt-get install -y', - extra: extra + extra: extra_with_lintian ) end diff --git a/distribution/packages/build.gradle b/distribution/packages/build.gradle index 72804c7a907..136803a5d83 100644 --- a/distribution/packages/build.gradle +++ b/distribution/packages/build.gradle @@ -270,13 +270,13 @@ apply plugin: 'nebula.ospackage-base' // this is package indepdendent configuration ospackage { maintainer 'Elasticsearch Team ' - summary ''' - Elasticsearch is a distributed RESTful search engine built for the cloud. + summary 'Distributed RESTful search engine built for the cloud' + packageDescription ''' Reference documentation can be found at https://www.elastic.co/guide/en/elasticsearch/reference/current/index.html and the 'Elasticsearch: The Definitive Guide' book can be found at https://www.elastic.co/guide/en/elasticsearch/guide/current/index.html - '''.stripIndent().replace('\n', ' ').trim() + '''.stripIndent().trim() url 'https://www.elastic.co/' // signing setup @@ -288,7 +288,8 @@ ospackage { new File(new File(System.getProperty('user.home'), '.gnupg'), 'secring.gpg') } - requires('coreutils') + // version found on oldest supported distro, centos-6 + requires('coreutils', '8.4', GREATER | EQUAL) fileMode 0644 dirMode 0755 @@ -312,12 +313,18 @@ Closure commonDebConfig(boolean oss, boolean jdk) { version = project.version.replace('-', '~') packageGroup 'web' - requires 'bash' + + // versions found on oldest supported distro, centos-6 + requires('bash', '4.1', GREATER | EQUAL) + requires('lsb-base', '4', GREATER | EQUAL) requires 'libc6' requires 'adduser' into('/usr/share/lintian/overrides') { from('src/deb/lintian/elasticsearch') + if (oss) { + rename('elasticsearch', 'elasticsearch-oss') + } } } } diff --git a/distribution/packages/src/deb/lintian/elasticsearch b/distribution/packages/src/deb/lintian/elasticsearch index 1ca52eaed28..98038177a00 100644 --- a/distribution/packages/src/deb/lintian/elasticsearch +++ b/distribution/packages/src/deb/lintian/elasticsearch @@ -1,8 +1,48 @@ -# Ignore arch dependent warnings, we chose the right libs on start -elasticsearch binary: arch-independent-package-contains-binary-or-object -# Not stripping external libraries -elasticsearch binary: unstripped-binary-or-object -# Ignore arch dependent warnings, we chose the right libs on start -elasticsearch binary: arch-dependent-file-in-usr-share -# Please check our changelog at http://www.elastic.co/downloads/elasticsearch -elasticsearch binary: changelog-file-missing-in-native-package +# we don't have a changelog, but we put our copyright file +# under /usr/share/doc/elasticsearch, which triggers this warning +changelog-file-missing-in-native-package + +# we intentionally copy our copyright file for all deb packages +copyright-file-contains-full-apache-2-license +copyright-should-refer-to-common-license-file-for-apache-2 +copyright-without-copyright-notice + +# we still put all our files under /usr/share/elasticsearch even after transition to platform dependent packages +arch-dependent-file-in-usr-share + +# we have a bundled jdk, so don't use jarwrapper +missing-dep-on-jarwrapper + +# we prefer to not make our config and log files world readable +non-standard-file-perm etc/default/elasticsearch 0660 != 0644 +non-standard-dir-perm etc/elasticsearch/ 2750 != 0755 +non-standard-file-perm etc/elasticsearch/* +non-standard-dir-perm var/lib/elasticsearch/ 2750 != 0755 +non-standard-dir-perm var/log/elasticsearch/ 2750 != 0755 +executable-is-not-world-readable etc/init.d/elasticsearch 0750 +non-standard-file-permissions-for-etc-init.d-script etc/init.d/elasticsearch 0750 != 0755 + +# this lintian tag is simply wrong; contrary to the explanation, debian systemd +# does actually look at /usr/lib/systemd/system +systemd-service-file-outside-lib usr/lib/systemd/system/elasticsearch.service + +# we do not automatically enable the service in init.d or systemd +script-in-etc-init.d-not-registered-via-update-rc.d etc/init.d/elasticsearch + +# the package scripts handle init.d/systemd directly and don't need to use deb helpers +maintainer-script-calls-systemctl +prerm-calls-updaterc.d elasticsearch + +# bundled JDK +embedded-library +arch-dependent-file-in-usr-share usr/share/elasticsearch/jdk/* +unstripped-binary-or-object usr/share/elasticsearch/jdk/* +extra-license-file usr/share/elasticsearch/jdk/legal/* +hardening-no-pie usr/share/elasticsearch/jdk/bin/* +hardening-no-pie usr/share/elasticsearch/jdk/lib/* + +# the system java version that lintian assumes is far behind what elasticsearch uses +unknown-java-class-version + +# elastic licensed modules contain elastic license +extra-license-file usr/share/elasticsearch/modules/* diff --git a/qa/vagrant/src/main/java/org/elasticsearch/packaging/test/PackageTestCase.java b/qa/vagrant/src/main/java/org/elasticsearch/packaging/test/PackageTestCase.java index 0408a03ca26..b6e26672f47 100644 --- a/qa/vagrant/src/main/java/org/elasticsearch/packaging/test/PackageTestCase.java +++ b/qa/vagrant/src/main/java/org/elasticsearch/packaging/test/PackageTestCase.java @@ -56,6 +56,7 @@ import static org.elasticsearch.packaging.util.Packages.startElasticsearch; import static org.elasticsearch.packaging.util.Packages.stopElasticsearch; import static org.elasticsearch.packaging.util.Packages.verifyPackageInstallation; import static org.elasticsearch.packaging.util.Platforms.getOsRelease; +import static org.elasticsearch.packaging.util.Platforms.isDPKG; import static org.elasticsearch.packaging.util.Platforms.isSystemd; import static org.elasticsearch.packaging.util.ServerUtils.makeRequest; import static org.elasticsearch.packaging.util.ServerUtils.runElasticsearchTests; @@ -78,6 +79,11 @@ public abstract class PackageTestCase extends PackagingTestCase { sh = newShell(); } + public void test05CheckLintian() throws Exception { + assumeTrue(isDPKG()); + sh.run("lintian --fail-on-warnings " + FileUtils.getDistributionFile(distribution())); + } + public void test10InstallPackage() throws Exception { assertRemoved(distribution()); installation = install(distribution()); From edd6438e3467e8ee2e5a0162d83982bfe10428a9 Mon Sep 17 00:00:00 2001 From: Benjamin Trent Date: Wed, 8 May 2019 15:03:28 -0500 Subject: [PATCH 2/7] mute test related to #41967 (#41968) --- .../org/elasticsearch/cluster/coordination/CoordinatorTests.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/test/java/org/elasticsearch/cluster/coordination/CoordinatorTests.java b/server/src/test/java/org/elasticsearch/cluster/coordination/CoordinatorTests.java index 3667e5c9762..d5599dfa6d6 100644 --- a/server/src/test/java/org/elasticsearch/cluster/coordination/CoordinatorTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/coordination/CoordinatorTests.java @@ -1065,6 +1065,7 @@ public class CoordinatorTests extends ESTestCase { cluster1.stabilise(); } + @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/41967") public void testDiscoveryUsesNodesFromLastClusterState() { final Cluster cluster = new Cluster(randomIntBetween(3, 5)); cluster.runRandomly(); From 4358cc6ac81793636be8f9208446a996ff0d2dd6 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Wed, 8 May 2019 16:42:50 -0600 Subject: [PATCH 3/7] Add note about ILM action ordering (#41771) Adds a note clarifying that actions are ordered automatically. --- docs/reference/ilm/policy-definitions.asciidoc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/reference/ilm/policy-definitions.asciidoc b/docs/reference/ilm/policy-definitions.asciidoc index 945f80babad..00578ce8c05 100644 --- a/docs/reference/ilm/policy-definitions.asciidoc +++ b/docs/reference/ilm/policy-definitions.asciidoc @@ -84,6 +84,10 @@ executing. The below list shows the actions which are available in each phase. +NOTE: The order that configured actions are performed in within each phase is +determined by automatically by {ilm-init}, and cannot be changed by changing the +policy definition. + * Hot - <> - <> From 83fef23fd137838f23677537dcbe386baee7fa1f Mon Sep 17 00:00:00 2001 From: Flavio Pompermaier Date: Thu, 9 May 2019 08:52:07 +0200 Subject: [PATCH 4/7] Fix wrong property name (#40636) --- docs/reference/ingest/processors/geoip.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/ingest/processors/geoip.asciidoc b/docs/reference/ingest/processors/geoip.asciidoc index f38e62806bb..4e866624309 100644 --- a/docs/reference/ingest/processors/geoip.asciidoc +++ b/docs/reference/ingest/processors/geoip.asciidoc @@ -27,7 +27,7 @@ uncompressed. The `ingest-geoip` config directory is located at `$ES_HOME/config | `ignore_missing` | no | `false` | If `true` and `field` does not exist, the processor quietly exits without modifying the document |====== -*Depends on what is available in `database_field`: +*Depends on what is available in `database_file`: * If the GeoLite2 City database is used, then the following fields may be added under the `target_field`: `ip`, `country_iso_code`, `country_name`, `continent_name`, `region_iso_code`, `region_name`, `city_name`, `timezone`, `latitude`, `longitude` From a329aaec9069d76d699dd634e555f484177c5d48 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 9 May 2019 08:46:34 +0200 Subject: [PATCH 5/7] Fix assertion error when caching the result of a search in a read-only index (#41900) The ReadOnlyEngine wraps its reader with a SoftDeletesDirectoryReaderWrapper if soft deletes are enabled. However the wrapping is done on top of the ElasticsearchDirectoryReader and that trips assertion later on since the cache key of these directories are different. This commit changes the order of the wrapping to put the ElasticsearchDirectoryReader first in order to ensure that it is always retrieved first when we unwrap the directory. Closes #41795 --- .../elasticsearch/index/engine/ReadOnlyEngine.java | 4 ++-- .../index/engine/ReadOnlyEngineTests.java | 13 ++++++++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/engine/ReadOnlyEngine.java b/server/src/main/java/org/elasticsearch/index/engine/ReadOnlyEngine.java index 5acac256dbd..b981bdb8a84 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/ReadOnlyEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/ReadOnlyEngine.java @@ -157,11 +157,11 @@ public class ReadOnlyEngine extends Engine { protected final DirectoryReader wrapReader(DirectoryReader reader, Function readerWrapperFunction) throws IOException { - reader = ElasticsearchDirectoryReader.wrap(reader, engineConfig.getShardId()); if (engineConfig.getIndexSettings().isSoftDeleteEnabled()) { reader = new SoftDeletesDirectoryReaderWrapper(reader, Lucene.SOFT_DELETES_FIELD); } - return readerWrapperFunction.apply(reader); + reader = readerWrapperFunction.apply(reader); + return ElasticsearchDirectoryReader.wrap(reader, engineConfig.getShardId()); } protected DirectoryReader open(IndexCommit commit) throws IOException { diff --git a/server/src/test/java/org/elasticsearch/index/engine/ReadOnlyEngineTests.java b/server/src/test/java/org/elasticsearch/index/engine/ReadOnlyEngineTests.java index f9437ac9251..e0ad514e6db 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/ReadOnlyEngineTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/ReadOnlyEngineTests.java @@ -18,9 +18,12 @@ */ package org.elasticsearch.index.engine; +import org.apache.lucene.index.DirectoryReader; +import org.apache.lucene.index.IndexReader; import org.apache.lucene.util.LuceneTestCase; import org.elasticsearch.Version; import org.elasticsearch.common.bytes.BytesArray; +import org.elasticsearch.common.lucene.index.ElasticsearchDirectoryReader; import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.index.mapper.ParsedDocument; import org.elasticsearch.index.seqno.SeqNoStats; @@ -32,7 +35,9 @@ import java.util.List; import java.util.concurrent.atomic.AtomicLong; import java.util.function.Function; +import static org.elasticsearch.common.lucene.index.ElasticsearchDirectoryReader.getElasticsearchDirectoryReader; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; public class ReadOnlyEngineTests extends EngineTestCase { @@ -80,6 +85,13 @@ public class ReadOnlyEngineTests extends EngineTestCase { Engine.Searcher external = readOnlyEngine.acquireSearcher("test", Engine.SearcherScope.EXTERNAL); Engine.Searcher internal = readOnlyEngine.acquireSearcher("test", Engine.SearcherScope.INTERNAL); assertSame(external.reader(), internal.reader()); + assertThat(external.reader(), instanceOf(DirectoryReader.class)); + DirectoryReader dirReader = external.getDirectoryReader(); + ElasticsearchDirectoryReader esReader = getElasticsearchDirectoryReader(dirReader); + IndexReader.CacheHelper helper = esReader.getReaderCacheHelper(); + assertNotNull(helper); + assertEquals(helper.getKey(), dirReader.getReaderCacheHelper().getKey()); + IOUtils.close(external, internal); // the locked down engine should still point to the previous commit assertThat(readOnlyEngine.getLocalCheckpoint(), equalTo(lastSeqNoStats.getLocalCheckpoint())); @@ -88,7 +100,6 @@ public class ReadOnlyEngineTests extends EngineTestCase { try (Engine.GetResult getResult = readOnlyEngine.get(get, readOnlyEngine::acquireSearcher)) { assertTrue(getResult.exists()); } - } // Close and reopen the main engine try (InternalEngine recoveringEngine = new InternalEngine(config)) { From a48b402e907e9d041adbc6b87e218731c7c65556 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Thu, 9 May 2019 10:12:30 +0300 Subject: [PATCH 6/7] Upgrade to Gradle 5.4.1 (#41750) * Upgrade to Gradle 5.4.1 https://docs.gradle.org/5.4/release-notes.html Notable: Support for JDK12 , API for incremental tasks * Use newer version of checkstyle * Increase stack size --- .../org/elasticsearch/gradle/precommit/PrecommitTasks.groovy | 5 ++++- buildSrc/src/main/resources/minimumGradleVersion | 2 +- gradle.properties | 2 +- gradle/wrapper/gradle-wrapper.properties | 4 ++-- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy index b2a9663cf7a..364617f0329 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy @@ -34,6 +34,9 @@ import org.gradle.api.plugins.quality.Checkstyle class PrecommitTasks { /** Adds a precommit task, which depends on non-test verification tasks. */ + + public static final String CHECKSTYLE_VERSION = '8.20' + public static Task create(Project project, boolean includeDependencyLicenses) { project.configurations.create("forbiddenApisCliJar") project.dependencies { @@ -214,7 +217,7 @@ class PrecommitTasks { configProperties = [ suppressions: checkstyleSuppressions ] - toolVersion = '8.10.1' + toolVersion = CHECKSTYLE_VERSION } project.tasks.withType(Checkstyle) { task -> diff --git a/buildSrc/src/main/resources/minimumGradleVersion b/buildSrc/src/main/resources/minimumGradleVersion index 11aa145248e..04edabda285 100644 --- a/buildSrc/src/main/resources/minimumGradleVersion +++ b/buildSrc/src/main/resources/minimumGradleVersion @@ -1 +1 @@ -5.3 \ No newline at end of file +5.4.1 \ No newline at end of file diff --git a/gradle.properties b/gradle.properties index ec79845d44e..491770edd7c 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,3 +1,3 @@ org.gradle.daemon=true -org.gradle.jvmargs=-Xmx2g -XX:+HeapDumpOnOutOfMemoryError +org.gradle.jvmargs=-Xmx2g -XX:+HeapDumpOnOutOfMemoryError -Xss2m options.forkOptions.memoryMaximumSize=2g diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index 8d172843af1..47216b872e4 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,6 +1,6 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-5.3-all.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-5.4.1-all.zip zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists -distributionSha256Sum=f4d820c2a9685710eba5b92f10e0e4fb20e0d6c0dd1f46971e658160f25e7147 +distributionSha256Sum=14cd15fc8cc8705bd69dcfa3c8fefb27eb7027f5de4b47a8b279218f76895a91 From 37771502aecf24c9e429d56f86260587c24519db Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 9 May 2019 06:30:46 -0400 Subject: [PATCH 7/7] Remove manual parsing of JVM options (#41962) This commit removes manual parsing of JVM options when calculating ergonomics. This is to avoid a situation that we parse values differently than the JVM would. In fact, we already have a bug along these lines today. It is possible to start the JVM with the same flag multiple times on the command line. In this case, the last value wins. For example, -Xmx1g -Xmx2g would start the JVM with a heap size of two gigabytes. Our JVM ergonomics ignores this possibility and instead the first value is winning! Our strategy to avoid manual parsing of the JVM options is to start the Java command line parser (without actually starting a JVM) by invoking java with the same command line flags as presented and request that the JVM tell us what values it would start with. This ensures that we have the correct values when making ergonomic decisions. Moreover, our strategy also is ignoring ES_JAVA_OPTS which could override the heap size as well leading to incorrect ergonomic choices. This commit address this issue too. --- distribution/src/bin/elasticsearch | 2 +- .../src/bin/elasticsearch-service.bat | 2 +- distribution/src/bin/elasticsearch.bat | 2 +- .../tools/launchers/JvmErgonomics.java | 113 +++++++++++------- .../tools/launchers/JvmOptionsParser.java | 20 +++- .../tools/launchers/JvmErgonomicsTests.java | 67 ++++++++--- 6 files changed, 139 insertions(+), 67 deletions(-) diff --git a/distribution/src/bin/elasticsearch b/distribution/src/bin/elasticsearch index 8bdea4950cb..6843607efa1 100755 --- a/distribution/src/bin/elasticsearch +++ b/distribution/src/bin/elasticsearch @@ -18,7 +18,7 @@ source "`dirname "$0"`"/elasticsearch-env ES_JVM_OPTIONS="$ES_PATH_CONF"/jvm.options JVM_OPTIONS=`"$JAVA" -cp "$ES_CLASSPATH" org.elasticsearch.tools.launchers.JvmOptionsParser "$ES_JVM_OPTIONS"` -ES_JAVA_OPTS="${JVM_OPTIONS//\$\{ES_TMPDIR\}/$ES_TMPDIR} $ES_JAVA_OPTS" +ES_JAVA_OPTS="${JVM_OPTIONS//\$\{ES_TMPDIR\}/$ES_TMPDIR}" # manual parsing to find out, if process should be detached if ! echo $* | grep -E '(^-d |-d$| -d |--daemonize$|--daemonize )' > /dev/null; then diff --git a/distribution/src/bin/elasticsearch-service.bat b/distribution/src/bin/elasticsearch-service.bat index 7a0be55c4f5..2f9c280743d 100644 --- a/distribution/src/bin/elasticsearch-service.bat +++ b/distribution/src/bin/elasticsearch-service.bat @@ -112,7 +112,7 @@ if not "%ES_JAVA_OPTS%" == "" set ES_JAVA_OPTS=%ES_JAVA_OPTS: =;% @setlocal for /F "usebackq delims=" %%a in (`"%JAVA% -cp "!ES_CLASSPATH!" "org.elasticsearch.tools.launchers.JvmOptionsParser" "!ES_JVM_OPTIONS!" || echo jvm_options_parser_failed"`) do set JVM_OPTIONS=%%a -@endlocal & set "MAYBE_JVM_OPTIONS_PARSER_FAILED=%JVM_OPTIONS%" & set ES_JAVA_OPTS=%JVM_OPTIONS:${ES_TMPDIR}=!ES_TMPDIR!% %ES_JAVA_OPTS% +@endlocal & set "MAYBE_JVM_OPTIONS_PARSER_FAILED=%JVM_OPTIONS%" & set ES_JAVA_OPTS=%JVM_OPTIONS:${ES_TMPDIR}=!ES_TMPDIR!% if "%MAYBE_JVM_OPTIONS_PARSER_FAILED%" == "jvm_options_parser_failed" ( exit /b 1 diff --git a/distribution/src/bin/elasticsearch.bat b/distribution/src/bin/elasticsearch.bat index ecbbad826e7..f14185ddc4a 100644 --- a/distribution/src/bin/elasticsearch.bat +++ b/distribution/src/bin/elasticsearch.bat @@ -44,7 +44,7 @@ IF ERRORLEVEL 1 ( set ES_JVM_OPTIONS=%ES_PATH_CONF%\jvm.options @setlocal for /F "usebackq delims=" %%a in (`CALL %JAVA% -cp "!ES_CLASSPATH!" "org.elasticsearch.tools.launchers.JvmOptionsParser" "!ES_JVM_OPTIONS!" ^|^| echo jvm_options_parser_failed`) do set JVM_OPTIONS=%%a -@endlocal & set "MAYBE_JVM_OPTIONS_PARSER_FAILED=%JVM_OPTIONS%" & set ES_JAVA_OPTS=%JVM_OPTIONS:${ES_TMPDIR}=!ES_TMPDIR!% %ES_JAVA_OPTS% +@endlocal & set "MAYBE_JVM_OPTIONS_PARSER_FAILED=%JVM_OPTIONS%" & set ES_JAVA_OPTS=%JVM_OPTIONS:${ES_TMPDIR}=!ES_TMPDIR!% if "%MAYBE_JVM_OPTIONS_PARSER_FAILED%" == "jvm_options_parser_failed" ( exit /b 1 diff --git a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmErgonomics.java b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmErgonomics.java index 761cd9e1be5..44f60ba2d9f 100644 --- a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmErgonomics.java +++ b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmErgonomics.java @@ -19,24 +19,28 @@ package org.elasticsearch.tools.launchers; +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.nio.charset.StandardCharsets; +import java.nio.file.Paths; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Optional; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; +import java.util.stream.Stream; /** * Tunes Elasticsearch JVM settings based on inspection of provided JVM options. */ final class JvmErgonomics { - private static final long KB = 1024L; - - private static final long MB = 1024L * 1024L; - - private static final long GB = 1024L * 1024L * 1024L; - private JvmErgonomics() { throw new AssertionError("No instances intended"); @@ -48,48 +52,74 @@ final class JvmErgonomics { * @param userDefinedJvmOptions A list of JVM options that have been defined by the user. * @return A list of additional JVM options to set. */ - static List choose(List userDefinedJvmOptions) { - List ergonomicChoices = new ArrayList<>(); - Long heapSize = extractHeapSize(userDefinedJvmOptions); - Map systemProperties = extractSystemProperties(userDefinedJvmOptions); - if (heapSize != null) { - if (systemProperties.containsKey("io.netty.allocator.type") == false) { - if (heapSize <= 1 * GB) { - ergonomicChoices.add("-Dio.netty.allocator.type=unpooled"); - } else { - ergonomicChoices.add("-Dio.netty.allocator.type=pooled"); - } + static List choose(final List userDefinedJvmOptions) throws InterruptedException, IOException { + final List ergonomicChoices = new ArrayList<>(); + final Map> finalJvmOptions = finalJvmOptions(userDefinedJvmOptions); + final long heapSize = extractHeapSize(finalJvmOptions); + final Map systemProperties = extractSystemProperties(userDefinedJvmOptions); + if (systemProperties.containsKey("io.netty.allocator.type") == false) { + if (heapSize <= 1 << 30) { + ergonomicChoices.add("-Dio.netty.allocator.type=unpooled"); + } else { + ergonomicChoices.add("-Dio.netty.allocator.type=pooled"); } } return ergonomicChoices; } - private static final Pattern MAX_HEAP_SIZE = Pattern.compile("^(-Xmx|-XX:MaxHeapSize=)(?\\d+)(?\\w)?$"); + private static final Pattern OPTION = + Pattern.compile("^\\s*\\S+\\s+(?\\S+)\\s+:?=\\s+(?\\S+)?\\s+\\{[^}]+?\\}\\s+\\{[^}]+}"); + + static Map> finalJvmOptions( + final List userDefinedJvmOptions) throws InterruptedException, IOException { + return Collections.unmodifiableMap(flagsFinal(userDefinedJvmOptions).stream() + .map(OPTION::matcher).filter(Matcher::matches) + .collect(Collectors.toMap(m -> m.group("flag"), m -> Optional.ofNullable(m.group("value"))))); + } + + private static List flagsFinal(final List userDefinedJvmOptions) throws InterruptedException, IOException { + /* + * To deduce the final set of JVM options that Elasticsearch is going to start with, we start a separate Java process with the JVM + * options that we would pass on the command line. For this Java process we will add two additional flags, -XX:+PrintFlagsFinal and + * -version. This causes the Java process that we start to parse the JVM options into their final values, display them on standard + * output, print the version to standard error, and then exit. The JVM itself never bootstraps, and therefore this process is + * lightweight. By doing this, we get the JVM options parsed exactly as the JVM that we are going to execute would parse them + * without having to implement our own JVM option parsing logic. + */ + final String java = Paths.get(System.getProperty("java.home"), "bin", "java").toString(); + final List command = + Collections.unmodifiableList( + Stream.of(Stream.of(java), userDefinedJvmOptions.stream(), Stream.of("-XX:+PrintFlagsFinal"), Stream.of("-version")) + .reduce(Stream::concat) + .get() + .collect(Collectors.toList())); + final Process process = new ProcessBuilder().command(command).start(); + final List output = readLinesFromInputStream(process.getInputStream()); + final List error = readLinesFromInputStream(process.getErrorStream()); + final int status = process.waitFor(); + if (status != 0) { + final String message = String.format( + Locale.ROOT, + "starting java failed with [%d]\noutput:\n%s\nerror:\n%s", + status, + String.join("\n", output), + String.join("\n", error)); + throw new RuntimeException(message); + } else { + return output; + } + } + + private static List readLinesFromInputStream(final InputStream is) throws IOException { + try (InputStreamReader isr = new InputStreamReader(is, StandardCharsets.UTF_8); + BufferedReader br = new BufferedReader(isr)) { + return Collections.unmodifiableList(br.lines().collect(Collectors.toList())); + } + } // package private for testing - static Long extractHeapSize(List userDefinedJvmOptions) { - for (String jvmOption : userDefinedJvmOptions) { - final Matcher matcher = MAX_HEAP_SIZE.matcher(jvmOption); - if (matcher.matches()) { - final long size = Long.parseLong(matcher.group("size")); - final String unit = matcher.group("unit"); - if (unit == null) { - return size; - } else { - switch (unit.toLowerCase(Locale.ROOT)) { - case "k": - return size * KB; - case "m": - return size * MB; - case "g": - return size * GB; - default: - throw new IllegalArgumentException("Unknown unit [" + unit + "] for max heap size in [" + jvmOption + "]"); - } - } - } - } - return null; + static Long extractHeapSize(final Map> finalJvmOptions) { + return Long.parseLong(finalJvmOptions.get("MaxHeapSize").get()); } private static final Pattern SYSTEM_PROPERTY = Pattern.compile("^-D(?[\\w+].*?)=(?.*)$"); @@ -105,4 +135,5 @@ final class JvmErgonomics { } return systemProperties; } + } diff --git a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOptionsParser.java b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOptionsParser.java index d74f106c50b..586be178f0d 100644 --- a/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOptionsParser.java +++ b/distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOptionsParser.java @@ -19,12 +19,14 @@ package org.elasticsearch.tools.launchers; +import org.elasticsearch.tools.java_version_checker.JavaVersion; + import java.io.BufferedReader; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.io.Reader; -import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Paths; import java.util.ArrayList; @@ -37,8 +39,7 @@ import java.util.SortedMap; import java.util.TreeMap; import java.util.regex.Matcher; import java.util.regex.Pattern; - -import org.elasticsearch.tools.java_version_checker.JavaVersion; +import java.util.stream.Collectors; /** * Parses JVM options from a file and prints a single line with all JVM options to standard output. @@ -51,14 +52,14 @@ final class JvmOptionsParser { * * @param args the args to the program which should consist of a single option, the path to the JVM options */ - public static void main(final String[] args) throws IOException { + public static void main(final String[] args) throws InterruptedException, IOException { if (args.length != 1) { throw new IllegalArgumentException("expected one argument specifying path to jvm.options but was " + Arrays.toString(args)); } final List jvmOptions = new ArrayList<>(); final SortedMap invalidLines = new TreeMap<>(); try (InputStream is = Files.newInputStream(Paths.get(args[0])); - Reader reader = new InputStreamReader(is, Charset.forName("UTF-8")); + Reader reader = new InputStreamReader(is, StandardCharsets.UTF_8); BufferedReader br = new BufferedReader(reader)) { parse( JavaVersion.majorVersion(JavaVersion.CURRENT), @@ -78,7 +79,14 @@ final class JvmOptionsParser { } if (invalidLines.isEmpty()) { - List ergonomicJvmOptions = JvmErgonomics.choose(jvmOptions); + // now append the JVM options from ES_JAVA_OPTS + final String environmentJvmOptions = System.getenv("ES_JAVA_OPTS"); + if (environmentJvmOptions != null) { + jvmOptions.addAll(Arrays.stream(environmentJvmOptions.split("\\s+")) + .filter(s -> s.trim().isEmpty() == false) + .collect(Collectors.toList())); + } + final List ergonomicJvmOptions = JvmErgonomics.choose(jvmOptions); jvmOptions.addAll(ergonomicJvmOptions); final String spaceDelimitedJvmOptions = spaceDelimitJvmOptions(jvmOptions); Launchers.outPrintln(spaceDelimitedJvmOptions); diff --git a/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/JvmErgonomicsTests.java b/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/JvmErgonomicsTests.java index 4b075d78b70..b5b6699f471 100644 --- a/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/JvmErgonomicsTests.java +++ b/distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/JvmErgonomicsTests.java @@ -19,38 +19,70 @@ package org.elasticsearch.tools.launchers; +import java.io.IOException; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.hasToString; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; public class JvmErgonomicsTests extends LaunchersTestCase { - public void testExtractValidHeapSize() { - assertEquals(Long.valueOf(1024), JvmErgonomics.extractHeapSize(Collections.singletonList("-Xmx1024"))); - assertEquals(Long.valueOf(2L * 1024 * 1024 * 1024), JvmErgonomics.extractHeapSize(Collections.singletonList("-Xmx2g"))); - assertEquals(Long.valueOf(32 * 1024 * 1024), JvmErgonomics.extractHeapSize(Collections.singletonList("-Xmx32M"))); - assertEquals(Long.valueOf(32 * 1024 * 1024), JvmErgonomics.extractHeapSize(Collections.singletonList("-XX:MaxHeapSize=32M"))); + + public void testExtractValidHeapSizeUsingXmx() throws InterruptedException, IOException { + assertThat( + JvmErgonomics.extractHeapSize(JvmErgonomics.finalJvmOptions(Collections.singletonList("-Xmx2g"))), + equalTo(2L << 30)); } - public void testExtractInvalidHeapSize() { + public void testExtractValidHeapSizeUsingMaxHeapSize() throws InterruptedException, IOException { + assertThat( + JvmErgonomics.extractHeapSize(JvmErgonomics.finalJvmOptions(Collections.singletonList("-XX:MaxHeapSize=2g"))), + equalTo(2L << 30)); + } + + public void testExtractValidHeapSizeNoOptionPresent() throws InterruptedException, IOException { + assertThat( + JvmErgonomics.extractHeapSize(JvmErgonomics.finalJvmOptions(Collections.emptyList())), + greaterThan(0L)); + } + + public void testHeapSizeInvalid() throws InterruptedException, IOException { try { - JvmErgonomics.extractHeapSize(Collections.singletonList("-Xmx2T")); - fail("Expected IllegalArgumentException to be raised"); - } catch (IllegalArgumentException expected) { - assertEquals("Unknown unit [T] for max heap size in [-Xmx2T]", expected.getMessage()); + JvmErgonomics.extractHeapSize(JvmErgonomics.finalJvmOptions(Collections.singletonList("-Xmx2Z"))); + fail("expected starting java to fail"); + } catch (final RuntimeException e) { + assertThat(e, hasToString(containsString(("starting java failed")))); + assertThat(e, hasToString(containsString(("Invalid maximum heap size: -Xmx2Z")))); } } - public void testExtractNoHeapSize() { - assertNull("No spaces allowed", JvmErgonomics.extractHeapSize(Collections.singletonList("-Xmx 1024"))); - assertNull("JVM option is not present", JvmErgonomics.extractHeapSize(Collections.singletonList(""))); - assertNull("Multiple JVM options per line", JvmErgonomics.extractHeapSize(Collections.singletonList("-Xms2g -Xmx2g"))); + public void testHeapSizeTooSmall() throws InterruptedException, IOException { + try { + JvmErgonomics.extractHeapSize(JvmErgonomics.finalJvmOptions(Collections.singletonList("-Xmx1024"))); + fail("expected starting java to fail"); + } catch (final RuntimeException e) { + assertThat(e, hasToString(containsString(("starting java failed")))); + assertThat(e, hasToString(containsString(("Too small maximum heap")))); + } + } + + public void testHeapSizeWithSpace() throws InterruptedException, IOException { + try { + JvmErgonomics.extractHeapSize(JvmErgonomics.finalJvmOptions(Collections.singletonList("-Xmx 1024"))); + fail("expected starting java to fail"); + } catch (final RuntimeException e) { + assertThat(e, hasToString(containsString(("starting java failed")))); + assertThat(e, hasToString(containsString(("Invalid maximum heap size: -Xmx 1024")))); + } } public void testExtractSystemProperties() { @@ -69,15 +101,16 @@ public class JvmErgonomicsTests extends LaunchersTestCase { assertTrue(parsedSystemProperties.isEmpty()); } - public void testLittleMemoryErgonomicChoices() { + public void testLittleMemoryErgonomicChoices() throws InterruptedException, IOException { String smallHeap = randomFrom(Arrays.asList("64M", "512M", "1024M", "1G")); List expectedChoices = Collections.singletonList("-Dio.netty.allocator.type=unpooled"); assertEquals(expectedChoices, JvmErgonomics.choose(Arrays.asList("-Xms" + smallHeap, "-Xmx" + smallHeap))); } - public void testPlentyMemoryErgonomicChoices() { + public void testPlentyMemoryErgonomicChoices() throws InterruptedException, IOException { String largeHeap = randomFrom(Arrays.asList("1025M", "2048M", "2G", "8G")); List expectedChoices = Collections.singletonList("-Dio.netty.allocator.type=pooled"); assertEquals(expectedChoices, JvmErgonomics.choose(Arrays.asList("-Xms" + largeHeap, "-Xmx" + largeHeap))); } + }