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

Update Java params and Docker image to expand JVM memory beyond 50% #300

Closed
jordanpadams opened this issue Mar 23, 2023 · 8 comments · Fixed by #303
Closed

Update Java params and Docker image to expand JVM memory beyond 50% #300

jordanpadams opened this issue Mar 23, 2023 · 8 comments · Fixed by #303

Comments

@jordanpadams
Copy link
Member

💡 Description

Default Java 11 JVM config is to use 50% of available memory. Since registry-api is running on it's own ECS cluster, we want to run this with more available memory (75%?)

@jordanpadams
Copy link
Member Author

@tloubrieu-jpl @jimmie @alexdunnjpl not sure who wants to do this, so assigned to you all. should hopefully be a quick-ish PR, review, merge.

@jordanpadams
Copy link
Member Author

@jimmie do we want to fudge this into the existing tagged docker image to be able to use in ECS right now?

@github-project-automation github-project-automation bot moved this to Release Backlog in B14.0 Mar 23, 2023
@jordanpadams
Copy link
Member Author

assigning to @alexdunnjpl . ping @jimmie or @tloubrieu-jpl if you have questions about what is going on here, or me for questions in priority

@jimmie
Copy link
Member

jimmie commented Mar 23, 2023

@alexdunnjpl - let me know if you'd like for me to go over where things are and what they (at least intend) to do...

@alexdunnjpl
Copy link
Contributor

@jimmie @jordanpadams I get the feeling this is one of those "takes longer to explain than to do" but at least I'll learn 😅

  • Does this amount to adding -Xmx6144m to the CMD in ./service/docker/Dockerfile.aws ?
  • Presumably we want to bring it in as a terraform variable specifying a proportion of total memory rather than hardcoding the value... Is there a better way of getting the memory than awk '/MemTotal/ { printf "%.3f \n", $2/1024 }' /proc/meminfo in this context? I've not worked with ECS before.

@jimmie
Copy link
Member

jimmie commented Mar 27, 2023

@alexdunnjpl - I agree that most of the time, hardcoding is the root of most evils. However, the terraform script likewise specifies the memory of the ECS instance (i.e. container) - so including explicit memory setting as part of the same script wouldn't be as terrible (and it is also more accountable that way since it is clearly specified in the file). That said however, the docker build is currently not yet part of the terraform. It wouldn't be difficult, but I would just want to make sure that terraform verifies if a new build is needed based on combination of name/tag and checksum (that is why I was holding off on including that in terraform). Perhaps we can work this in parallel...

@alexdunnjpl
Copy link
Contributor

@jimmie in the immediate-term should I just go ahead and hardcode it into the Dockerfile, then, and we can worry about the single-source-of-truth approach later on?

@jimmie
Copy link
Member

jimmie commented Mar 29, 2023

@alexdunnjpl - yes, I would think that's the best solution for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: 🏁 Done
Development

Successfully merging a pull request may close this issue.

4 participants