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

Reduce generate_dump mem usage for cores #3052

Merged

Conversation

davidm-arista
Copy link
Contributor

Add the core files to the tarball while they are been processed, this ensures that only one core file at a time will be consuming flash space inside the tarpath and the tarball.

Add the core files to the tarball while they are been processed, this ensures that
only one core file at a time will be consuming flash space inside the tarpath and the
tarball.
@davidm-arista
Copy link
Contributor Author

@yxieca can you please take a look

@yxieca
Copy link
Contributor

yxieca commented Nov 28, 2023

@dgsudharsan can you suggest a reviewer for this change? Thanks!

@dgsudharsan
Copy link
Collaborator

@vivekrnv Please review

Copy link
Contributor

@vivekrnv vivekrnv left a comment

Choose a reason for hiding this comment

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

So the assumption is saving into tarball would be efficient than saving to flash path. But we are not compressing the file while appending it to tarball so i'm not very sure how efficient this is than the current approach.

Can you provide date to signify this?

@davidm-arista
Copy link
Contributor Author

Hi @vivekrnv

save_crash_files() calls save_file() for each core file, which will copy the core files into the tar_path. If the 4th argument of save_file() is true it will add it to the tarball, and delete the core file. If the 4th argument of save_file() is false, it will not add it to the tarfile, and will not delete the core the file.

At the end of the generate_dump script save_to_tar() is called which will add all the files in tar_path to the tarball.

If we had 4 core files, before this change we would end up with:
4 core files in /var/core
4 core files /var/dump/<tar_path>/...
4 core files in the tar file

After this change we would end up with:
4 core files in /var/core
4 core files in the tar file

Resulting in 4 less core files consuming flash space.

Of course this is temporary as tar_path will get cleaned up after this has run. But during this time the savings is significant in tests with lots of core dump files.

@StormLiangMS
Copy link
Contributor

@vivekrnv @dgsudharsan could you help to comment if this is good to merge.

@StormLiangMS StormLiangMS merged commit 377e581 into sonic-net:master Dec 8, 2023
5 checks passed
@StormLiangMS
Copy link
Contributor

ADO:26118419

@StormLiangMS
Copy link
Contributor

@davidm-arista there is a cherry pick failure, could you help to create a separate PR to 202305?

@liushilongbuaa
Copy link
Contributor

Sorry. Maybe it is a bug in automation.
checking.

mssonicbld pushed a commit to mssonicbld/sonic-utilities that referenced this pull request Dec 8, 2023
Add the core files to the tarball while they are been processed, this ensures that
only one core file at a time will be consuming flash space inside the tarpath and the
tarball.
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202305: #3074

mssonicbld pushed a commit that referenced this pull request Dec 8, 2023
Add the core files to the tarball while they are been processed, this ensures that
only one core file at a time will be consuming flash space inside the tarpath and the
tarball.
mssonicbld pushed a commit to mssonicbld/sonic-utilities that referenced this pull request Feb 2, 2024
Add the core files to the tarball while they are been processed, this ensures that
only one core file at a time will be consuming flash space inside the tarpath and the
tarball.
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202311: #3140

mssonicbld pushed a commit that referenced this pull request Feb 2, 2024
Add the core files to the tarball while they are been processed, this ensures that
only one core file at a time will be consuming flash space inside the tarpath and the
tarball.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants