Skip to content
This repository has been archived by the owner on May 16, 2023. It is now read-only.

[elasticsearch] only configure ES_JAVA_OPTS when value is set #1089

Merged
merged 2 commits into from Mar 18, 2021

Conversation

jmlrt
Copy link
Member

@jmlrt jmlrt commented Mar 3, 2021

This commit update Elasticsearch Statefulset template to set
ES_JAVA_OPTS variable only when esJavaOpts value is defined.

This allow to benefit of JVM heap automatic sizing in Elasticsearch
7.11.

Fix #1083

This commit update Elasticsearch Statefulset template to set
`ES_JAVA_OPTS` variable only when `esJavaOpts` value is defined.

This allow to benefit of JVM heap automatic sizing in Elasticsearch
7.11.

Fix elastic#1083
@jmlrt jmlrt requested a review from a team March 3, 2021 11:27
mehta-ankit
mehta-ankit previously approved these changes Mar 3, 2021
@@ -315,8 +315,10 @@ spec:
value: "{{ .Values.clusterName }}"
- name: network.host
value: "{{ .Values.networkHost }}"
{{- if .Values.esJavaOpts }}
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like there is a test asserting the default ES_JAVA_OPTS option that will need updated https://devops-ci.elastic.co/job/elastic+helm-charts+pull-request+template-testing/1218/CHART=elasticsearch,label=docker&&virtual/console

Copy link
Contributor

@Conky5 Conky5 left a comment

Choose a reason for hiding this comment

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

LGTM

@jmlrt jmlrt merged commit 0ff3a3c into elastic:master Mar 18, 2021
@jmlrt jmlrt deleted the fix-1083 branch March 18, 2021 14:24
@jmlrt jmlrt added enhancement New feature or request need-backport labels Mar 18, 2021
AndreasChristianson added a commit to AndreasChristianson/helm-charts that referenced this pull request Apr 6, 2021
jmlrt pushed a commit to jmlrt/helm-charts that referenced this pull request Apr 23, 2021
jmlrt pushed a commit to jmlrt/helm-charts that referenced this pull request Apr 23, 2021
jmlrt pushed a commit to jmlrt/helm-charts that referenced this pull request Apr 23, 2021
jmlrt added a commit that referenced this pull request Apr 23, 2021
…1157)

see #1089

Co-authored-by: AndreasChristianson <33329956+AndreasChristianson@users.noreply.github.com>
jmlrt added a commit that referenced this pull request Apr 23, 2021
…#1158)

see #1089

Co-authored-by: AndreasChristianson <33329956+AndreasChristianson@users.noreply.github.com>
jmlrt added a commit that referenced this pull request Apr 23, 2021
…1159)

see #1089

Co-authored-by: AndreasChristianson <33329956+AndreasChristianson@users.noreply.github.com>
jmlrt added a commit to jmlrt/helm-charts that referenced this pull request May 25, 2021
jmlrt added a commit to jmlrt/helm-charts that referenced this pull request May 25, 2021
jmlrt added a commit to jmlrt/helm-charts that referenced this pull request May 25, 2021
@viceice
Copy link

viceice commented Jul 12, 2021

This broke my upgrade from 6.8.16 to 6.8.17 with constantly crashing elastice search v6

@jmlrt
Copy link
Member Author

jmlrt commented Jul 22, 2021

Hi @viceice, the upgrade from 6.8.16 to 6.8.17 with this feature was tested successfully in our CI.
Can you open a new issue with all the details to understand what problem did you add?

@viceice
Copy link

viceice commented Aug 2, 2021

@jmlrt I'm using imageTag: docker.elastic.co/elasticsearch/elasticsearch:5.6.16, so it's probably a problem with the ancient es version. I can't currently switch to a newer es, because of some breakings i need so solve first.

@jmlrt
Copy link
Member Author

jmlrt commented Aug 2, 2021

Hi @viceice, without much investigation I think this may be related to the old Elasticsearch version.
Note that only 6.8.X and 7.X versions are supported with these Helm charts.
All previous versions were never tested by Elastic with these charts and we don't try to maintain any compatibility.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can we disable the default setting of JVM heap size in statefulset.yaml?
4 participants