Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move ES_TMPDIR substitution into jvm options parser #47189

Merged
merged 5 commits into from
Oct 4, 2019
Merged

Move ES_TMPDIR substitution into jvm options parser #47189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
32f5b17
Move ES_TMPDIR substitution into jvm options parser
jasontedor Sep 27, 2019
77dd706
Fix Windows
jasontedor Sep 27, 2019
00ebd8b
Add test
jasontedor Sep 27, 2019
6150294
Merge remote-tracking branch 'elastic/master' into jvm-options-es-tmp…
jasontedor Oct 4, 2019
4844dc2
Iteration
jasontedor Oct 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions distribution/src/bin/elasticsearch
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ if [ -z "$ES_TMPDIR" ]; then
fi

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=`export ES_TMPDIR; "$JAVA" -cp "$ES_CLASSPATH" org.elasticsearch.tools.launchers.JvmOptionsParser "$ES_JVM_OPTIONS"`

# manual parsing to find out, if process should be detached
if ! echo $* | grep -E '(^-d |-d$| -d |--daemonize$|--daemonize )' > /dev/null; then
Expand Down
4 changes: 2 additions & 2 deletions distribution/src/bin/elasticsearch-service.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ set ES_JVM_OPTIONS=%ES_PATH_CONF%\jvm.options
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!%
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 ES_JAVA_OPTS=%%a
@endlocal & set "MAYBE_JVM_OPTIONS_PARSER_FAILED=%ES_JAVA_OPTS%" & set ES_JAVA_OPTS=%ES_JAVA_OPTS%

if "%MAYBE_JVM_OPTIONS_PARSER_FAILED%" == "jvm_options_parser_failed" (
exit /b 1
Expand Down
4 changes: 2 additions & 2 deletions distribution/src/bin/elasticsearch.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ if not defined ES_TMPDIR (

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!%
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 ES_JAVA_OPTS=%%a
@endlocal & set "MAYBE_JVM_OPTIONS_PARSER_FAILED=%ES_JAVA_OPTS%" & set ES_JAVA_OPTS=%ES_JAVA_OPTS%

if "%MAYBE_JVM_OPTIONS_PARSER_FAILED%" == "jvm_options_parser_failed" (
exit /b 1
Expand Down
26 changes: 23 additions & 3 deletions ...ion/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOptionsParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,11 @@ public void accept(final int lineNumber, final String line) {
.filter(Predicate.not(String::isBlank))
.collect(Collectors.toUnmodifiableList()));
}
final List<String> ergonomicJvmOptions = JvmErgonomics.choose(jvmOptions);
jvmOptions.addAll(ergonomicJvmOptions);
final String spaceDelimitedJvmOptions = spaceDelimitJvmOptions(jvmOptions);
final List<String> substitutedJvmOptions =
substitutePlaceholders(jvmOptions, Map.of("ES_TMPDIR", System.getenv("ES_TMPDIR")));
final List<String> ergonomicJvmOptions = JvmErgonomics.choose(substitutedJvmOptions);
substitutedJvmOptions.addAll(ergonomicJvmOptions);
final String spaceDelimitedJvmOptions = spaceDelimitJvmOptions(substitutedJvmOptions);
Launchers.outPrintln(spaceDelimitedJvmOptions);
Launchers.exit(0);
} else {
Expand All @@ -115,6 +117,24 @@ public void accept(final int lineNumber, final String line) {
}
}

static List<String> substitutePlaceholders(final List<String> jvmOptions, final Map<String, String> substitutions) {
final Map<String, String> placeholderSubstitutions =
substitutions.entrySet().stream().collect(Collectors.toMap(e -> "${" + e.getKey() + "}", Map.Entry::getValue));
return jvmOptions.stream()
.map(
jvmOption -> {
String actualJvmOption = jvmOption;
int start = jvmOption.indexOf("${");
if (start >= 0 && jvmOption.indexOf('}', start) > 0) {
for (final Map.Entry<String, String> placeholderSubstitution : placeholderSubstitutions.entrySet()) {
actualJvmOption = actualJvmOption.replace(placeholderSubstitution.getKey(), placeholderSubstitution.getValue());
}
}
return actualJvmOption;
})
.collect(Collectors.toList());
}

/**
* Callback for valid JVM options.
*/
Expand Down
7 changes: 7 additions & 0 deletions ...ools/launchers/src/test/java/org/elasticsearch/tools/launchers/JvmOptionsParserTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.util.Map;
import java.util.concurrent.atomic.AtomicBoolean;

import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.equalTo;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
Expand All @@ -39,6 +40,12 @@

public class JvmOptionsParserTests extends LaunchersTestCase {

public void testSubstitution() {
final List<String> jvmOptions =
JvmOptionsParser.substitutePlaceholders(List.of("-Djava.io.tmpdir=${ES_TMPDIR}"), Map.of("ES_TMPDIR", "/tmp/elasticsearch"));
assertThat(jvmOptions, contains("-Djava.io.tmpdir=/tmp/elasticsearch"));
}

public void testUnversionedOptions() throws IOException {
try (StringReader sr = new StringReader("-Xms1g\n-Xmx1g");
BufferedReader br = new BufferedReader(sr)) {
Expand Down