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.

Sign up for GitHub

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

Jump to bottom

[Dask] Enhance extending env vars to avoid memory leak [1.6.x] #5486

Merged
merged 4 commits into from
May 2, 2024
Merged

[Dask] Enhance extending env vars to avoid memory leak [1.6.x] #5486

merged 4 commits into from
May 2, 2024

Conversation

liranbg
Copy link
Member

@liranbg liranbg commented May 1, 2024 •
edited
Loading

Function spec may already include some of the envvars generated by generate_runtime_k8s_env causing the extend list items to include them again. every scheduled cycle of such dask function would extend its env var list to infinity

https://iguazio.atlassian.net/browse/ML-6317

@liranbg liranbg changed the title [Dask] Enhance extending env vars to avoid memory leak [Dask] Enhance extending env vars to avoid memory leak [1.6.x] May 1, 2024
@liranbg liranbg requested a review from quaark May 1, 2024 20:58
TomerShor
TomerShor requested changes May 2, 2024
View reviewed changes
server/api/runtime_handlers/daskjob.py Outdated Show resolved Hide resolved
tests/api/runtimes/test_dask.py Show resolved Hide resolved
quaark
quaark requested changes May 2, 2024
View reviewed changes
Copy link
Member

@quaark quaark left a comment

Choose a reason for hiding this comment

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

Awesome catch!
I do however suggest making the logic slightly simpler and the code comment is really confusing actually, so I suggest making it clearer or aligning the code to it.

server/api/runtime_handlers/daskjob.py Outdated Show resolved Hide resolved
quaark
quaark requested changes May 2, 2024
View reviewed changes
server/api/runtime_handlers/daskjob.py Outdated Show resolved Hide resolved
@liranbg liranbg merged commit dbb53dc into mlrun:1.6.x May 2, 2024
10 checks passed
@liranbg liranbg deleted the dask-envvar-recreation branch May 2, 2024 08:56
liranbg added a commit to liranbg/mlrun that referenced this pull request May 2, 2024
@liranbg
[Dask] Enhance extending env vars to avoid memory leak [1.6.x] (mlrun…
2332385 
…#5486)
@liranbg liranbg mentioned this pull request May 2, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@quaark quaark quaark requested changes

@TomerShor TomerShor TomerShor requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@liranbg @quaark @TomerShor