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

Memory Leak in warmer #2754

Closed
mxbossard opened this issue Sep 22, 2023 · 2 comments · Fixed by #2763
Closed

Memory Leak in warmer #2754

mxbossard opened this issue Sep 22, 2023 · 2 comments · Fixed by #2763
Labels
area/performance issues related to kaniko performance enhancement feat/warmer kind/enhancement New feature or request priority/p2 High impact feature/bug. Will get a lot of users happy

Comments

@mxbossard
Copy link
Contributor

mxbossard commented Sep 22, 2023

Actual behavior
When we warm an image it's fully loaded in RAM before it's written on disk.
The TarWriter is a bytes.Buffer here:

TarWriter: tarBuf,

Then the files are written here:
if err := writeBufsToFile(cachePath, tarBuf, manifestBuf); err != nil {

Expected behavior
The downloaded file should be streamed into a file on disk with a FileWritter.

To Reproduce
warmer -i

Triage Notes for the Maintainers

Description Yes/No
Please check if this a new feature you are proposing
  • - [No]
@aaron-prindle aaron-prindle added area/performance issues related to kaniko performance enhancement feat/warmer priority/p2 High impact feature/bug. Will get a lot of users happy kind/enhancement New feature or request area/filesystems For all bugs related to kaniko container filesystems (mounting issues etc) and removed area/filesystems For all bugs related to kaniko container filesystems (mounting issues etc) labels Sep 25, 2023
@aaron-prindle
Copy link
Collaborator

Thank you for flagging this @mxbossard! Would you want to drive a PR to fix this issue - changing the warmer to stream files vs loading them all into RAM first?

@mxbossard mxbossard mentioned this issue Sep 27, 2023
4 tasks
@mxbossard
Copy link
Contributor Author

I gave it a try and proposed a PR.
I added a shell script in scripts dir to test the warmer in a docker container within boxed memory conditions.
I could reproduce the oom error before the fix and validate the fix.

aaron-prindle pushed a commit that referenced this issue Nov 29, 2023
* Fix warmer memory leak. Write down images directly into a temp file. Add a script to test warmer in boxed memory conditions. Fixes: #2754

* Document usage of boxed_warm_in_docker.sh integration test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/performance issues related to kaniko performance enhancement feat/warmer kind/enhancement New feature or request priority/p2 High impact feature/bug. Will get a lot of users happy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants