Allow to use the bundled JDK in Elasticsearch #853
Conversation
With this commit we add special handling for Elasticsearch versions that bundle the JDK when the command line parameter `--runtime-jdk=bundled` is set. In that case, Rally will use the bundled JDK (i.e. not set `JAVA_HOME` when starting Elasticsearch).
danielmitterdorfer
added
enhancement
docs/command_line_reference.rst
Outdated
| @@ -510,6 +510,8 @@ Example:: | |||
| # Force to run with JDK 7 | |||
| esrally --distribution-version=2.4.0 --runtime-jdk=7 | |||
|
|
|||
| There is also limited support to specify the JDK that is bundled with Elasticsearch with the special value ``bundled``. At the moment this only works on Linux. The JDK is bundled from Elasticsearch 7.0.0 onwards. | |||
There was a problem hiding this comment.
Does it make sense to link: https://www.elastic.co/blog/elasticsearch-7-0-0-released re: The JDK is bundled from Elasticsearch 7.0.0 onwards.?
There was a problem hiding this comment.
I'd consider a reference documentation more stable than a blog and would thus rather avoid pointing to a blog post?
There was a problem hiding this comment.
There was a problem hiding this comment.
Addessed in 6662420
tests/mechanic/java_resolver_test.py
Outdated
|
|
||
| # assumes most recent JDK | ||
| self.assertEqual(major, 12) | ||
| # does not extract JAVA_HOME for the bundled JDK |
There was a problem hiding this comment.
nit: does not set JAVA_HOME?
There was a problem hiding this comment.
Good point. I'll push a change
With this commit we use the proper Elasticsearch artifact per platform (if there is one). Therefore, it is possible to use the bundled JDK not only on Linux but also on MacOS.
|
Together with elastic/rally-teams#38, Rally can now:
@dliappis can you please have another look? As we already rely on new properties that will be introduced in elastic/rally-teams#38, that PR needs to be merged first. |
|
|
||
| def render(self, template): | ||
| substitutions = { | ||
| "{{VERSION}}": self.version, |
There was a problem hiding this comment.
Is there a reason -- apart from simplicity -- we can't use j2 template rendering to substitute those (and at the same time be more lenient with spaces like {{ VERSION }} and allow filters)? Many other files in rally-teams are treated as j2 hence the question.
There was a problem hiding this comment.
For other templates (like elasticsearch.yml or jvm.options) I think Jinja makes sense because their purpose is to serve as templates. config.ini is a high-level file meant to control how Rally behaves and I would not expect much templating happening here. As discussed elsewhere, I'll raise a separate enhancement issue.
There was a problem hiding this comment.
I've raised #856.
| @@ -859,6 +870,9 @@ def main(): | |||
| console.println("--target-hosts and --client-options must define the same keys for multi cluster setups.") | |||
| exit(1) | |||
| # split by component? | |||
| if sub_command == "download": | |||
There was a problem hiding this comment.
Don't we need to support --target-arch and --target-os also for the the install subcommand?
There was a problem hiding this comment.
I would like to restrict this to the download subcommand for the moment because we should not support installing an artifact for a different platform IMHO as the successive startup is likely to fail in some scenarios and I also don't see why one would use this approach.
|
@elasticmachine test this please |
hub-cap
added
the
highlight
With this commit we add special handling for Elasticsearch versions that
bundle the JDK when the command line parameter
--runtime-jdk=bundledis set. In that case, Rally will use the bundled JDK (i.e. not set
JAVA_HOMEwhen starting Elasticsearch).