Skip to content
This repository was archived by the owner on Jun 24, 2022. It is now read-only.

heap.dump.path is set as mandatory variable. Setting it to optional i… #668

Closed

Conversation

ozonsozons
Copy link

heap.dump.path is set as mandatory variable.
Currently, if heap.dump.path is not defined then after deployment in linux it appears as "-XX:HeapDumpPath=${heap.dump.path}" in elastic+ process

Setting it to optional if defined.

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

Copy link
Member

@jmlrt jmlrt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ozonsozons,
Thanks for this PR.
${heap.dump.path} is not an ansible variable but a shell variable that can be used by Elasticsearch if defined.
According to jvm.options source, we should replace -XX:HeapDumpPath=${heap.dump.path} wich was the old way by ${heap.dump.path}. This way nothing appears in Elasticsearch process if this variable is empty.

Comment on lines +113 to +115
{% if heap.dump.path is defined %}
-XX:HeapDumpPath=${heap.dump.path}
{% endif %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{% if heap.dump.path is defined %}
-XX:HeapDumpPath=${heap.dump.path}
{% endif %}
${heap.dump.path}

@jmlrt
Copy link
Member

jmlrt commented May 6, 2020

We will finally add a new Ansible variable es_heap_dump_path in #691.
Closing this PR.

@jmlrt jmlrt closed this May 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants