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.

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

refactor: gohan elasticsearch jvm heap options file #231

Merged
merged 4 commits into from
Mar 22, 2024

Conversation

v-rocheleau
Copy link
Contributor

Replaced the Elasticsearch JVM heap size environment variable with the preffered configuration file approach.

https://www.elastic.co/guide/en/elasticsearch/reference/current/advanced-configuration.html#set-jvm-heap-size

Copy link
Member

@davidlougheed davidlougheed left a comment

Choose a reason for hiding this comment

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

small typo fixes

docs/installation.md Outdated Show resolved Hide resolved
docs/installation.md Outdated Show resolved Hide resolved
Copy link
Member

@davidlougheed davidlougheed left a comment

Choose a reason for hiding this comment

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

two more things, otherwise looks good to me

docs/installation.md Show resolved Hide resolved
@@ -47,6 +47,10 @@ lib/public/*
!lib/public/.gitkeep
!lib/public/docker-compose.public.yaml

# gohan_elasticsearch
lib/gohan/es_jvm_options/*
!lib/gohan/es_jvm_options/.gitkeep
Copy link
Member

Choose a reason for hiding this comment

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

what happens when the user runs gohan without anything in here; does it work, or should we have a default editable file here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It works with nothing in it as well. This dir binds to /usr/share/elasticsearch/config/jvm.options.d/, which is always empty by default . Only files with the .options suffix are read by ES there, so whatever is in .gitkeep is ignored.

https://www.elastic.co/guide/en/elasticsearch/reference/current/advanced-configuration.html#set-jvm-options

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regarding the content of options file, per the docs:

Blank lines are ignored. Lines beginning with # are treated as comments and ignored. Lines that aren’t commented out and aren’t recognized as valid JVM arguments are rejected and Elasticsearch will fail to start.

Copy link
Member

@davidlougheed davidlougheed left a comment

Choose a reason for hiding this comment

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

lgtm

@v-rocheleau v-rocheleau merged commit e6cae74 into releases/v15.2 Mar 22, 2024
2 checks passed
@v-rocheleau v-rocheleau deleted the refact/gohan-es-config branch March 22, 2024 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants