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

CROM-6917 Optionally export MEM_SIZE and MEM_UNIT for all standard backends. #6766

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

kshakir
Copy link
Contributor

@kshakir kshakir commented May 21, 2022

No description provided.

@kshakir
Copy link
Contributor Author

kshakir commented May 21, 2022

To effectively use "Retry with More Memory" for JVM jobs running on Papi one has to use MEM_SIZE and MEM_UNIT to run with the correct memory settings. But those variables were not available on any other backends. Therefore one ended up having to use other creative options for authoring multi-backend WDL such as using free.

This PR exposes those two environment variables to the command block on any Cromwell "standard" backend that supports the memory runtime attribute.

Using the environment variables also helps with call caching java jobs. One can use something along the lines of -Xmx${MEM_SIZE%.*}${MEM_UNIT%?} in a version 1.0+ WDL and the command block will stay the same even if the memory needs to be increased.


Side note: If anyone comes across this PR and wonders why the default Local backend doesn't support MEM_SIZE and MEM_UNIT it's because the Local backend does not use memory (nor cpu at the moment). The memory runtime attribute would need to be added into the runtime attributes with something like:

runtime-attributes = """
  String? docker
  String? docker_user
  Int memory_mb = 2048
"""

And then inside submit-docker use --memory=${memory_mb}m.

Then the changes in this PR will generate MEM_SIZE and MEM_UNIT for the Local backend too.

@kshakir kshakir requested a review from a team as a code owner May 9, 2023 15:34
@kshakir kshakir changed the title Optionally export MEM_SIZE and MEM_UNIT for all standard backends. CROM-6917 Optionally export MEM_SIZE and MEM_UNIT for all standard backends. May 11, 2024
@kshakir kshakir marked this pull request as draft May 11, 2024 01:01
@kshakir kshakir force-pushed the ks_standard_mem_env branch from b9fc1c3 to 61063cb Compare May 11, 2024 01:05
@kshakir kshakir marked this pull request as ready for review May 11, 2024 06:17
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.

1 participant