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

VS-1246 Add more disk and add memory retry handling on SelectVariants task #9096

Open
wants to merge 9 commits into
base: ah_var_store
Choose a base branch
from

Conversation

gbggrant
Copy link
Collaborator

@gbggrant gbggrant commented Feb 12, 2025

The task SelectVariants failed several times in Echo filter set creation for what appeared to be memory (and maybe disk) space issues. This PR increases memory (more to system) and disk and lets the task handle Terra's retry with more memory strategy.

Passing integration test here

@gbggrant gbggrant marked this pull request as ready for review February 13, 2025 16:30
Copy link
Collaborator

@mcovarr mcovarr left a comment

Choose a reason for hiding this comment

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

Do the AoU docs for filter set creation need to be updated to say to use retry with more memory?

scripts/variantstore/wdl/GvsUtils.wdl Outdated Show resolved Hide resolved
@gbggrant gbggrant requested a review from mcovarr February 13, 2025 20:32
@@ -1154,10 +1167,11 @@ task SelectVariants {
runtime {
docker: gatk_docker
cpu: 1
memory: "${memory_mb} MiB"
memory: memory_gib + " GB"

Choose a reason for hiding this comment

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

this may be silly, but I don't know why you are changing how we're doing the string interpolation and setting the memory field here. Was the old way broken? Or is this just to bring it in line with how we do things elsewhere?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I actually just lifted this from the other example of retry with more memory (in Extract) - really didn't think much of the change - but agree that we should stick with how we do things.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have updated this and retested with the change.

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.

3 participants