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

defaulting to unbuffered reader for dmverity hashing #1887

Merged

Conversation

SethHollandsworth
Copy link
Contributor

In the confcom CLI tooling that uses the dmverity-vhd executable, we were seeing an issue where large OCI images would cause the dmverity hashing process to get killed because the machine ran out of memory: example.

This unbuffered option makes go-containerregistry stream in the image as needed instead of loading it all at once, saving memory with the tradeoff of being slower for large images. The typical use case we've seen is to use the confcom tooling on a local machine or VM with ~16GB of memory so memory efficiency is more important than time efficiency.

Discussed with Maksim that making this the default should be fine. thanks!

Co-authored-by: ksayid <khalilsayid@microsoft.com>
Signed-off-by: Seth Hollandsworth <sethho@microsoft.com>
@SethHollandsworth SethHollandsworth requested a review from a team as a code owner September 6, 2023 14:33
Copy link
Collaborator

@KenGordon KenGordon left a comment

Choose a reason for hiding this comment

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

I doubt it is slower.

@anmaxvl anmaxvl merged commit e7509cc into microsoft:main Sep 13, 2023
16 checks passed
anmaxvl pushed a commit to anmaxvl/hcsshim that referenced this pull request Sep 14, 2023
Signed-off-by: Seth Hollandsworth <sethho@microsoft.com>
Co-authored-by: ksayid <khalilsayid@microsoft.com>
(cherry picked from commit e7509cc)
anmaxvl pushed a commit to anmaxvl/hcsshim that referenced this pull request Sep 14, 2023
Signed-off-by: Seth Hollandsworth <sethho@microsoft.com>
Co-authored-by: ksayid <khalilsayid@microsoft.com>
(cherry picked from commit e7509cc)
Signed-off-by: Maksim An <maksiman@microsoft.com>
@anmaxvl anmaxvl mentioned this pull request Sep 14, 2023
anmaxvl pushed a commit to anmaxvl/hcsshim that referenced this pull request Sep 20, 2023
Signed-off-by: Seth Hollandsworth <sethho@microsoft.com>
Co-authored-by: ksayid <khalilsayid@microsoft.com>
(cherry picked from commit e7509cc)
Signed-off-by: Maksim An <maksiman@microsoft.com>
anmaxvl pushed a commit to anmaxvl/hcsshim that referenced this pull request Sep 20, 2023
Signed-off-by: Seth Hollandsworth <sethho@microsoft.com>
Co-authored-by: ksayid <khalilsayid@microsoft.com>
(cherry picked from commit e7509cc)
Signed-off-by: Maksim An <maksiman@microsoft.com>
anmaxvl pushed a commit that referenced this pull request Sep 20, 2023
Signed-off-by: Seth Hollandsworth <sethho@microsoft.com>
Co-authored-by: ksayid <khalilsayid@microsoft.com>
(cherry picked from commit e7509cc)
Signed-off-by: Maksim An <maksiman@microsoft.com>
helsaawy pushed a commit to helsaawy/hcsshim that referenced this pull request Sep 27, 2023
Signed-off-by: Seth Hollandsworth <sethho@microsoft.com>
Co-authored-by: ksayid <khalilsayid@microsoft.com>
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.

5 participants