libimage: rework DiskUsage() to count layers #2355
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Luap99 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
cc @mtrmac I will make a PR against podman to see if any tests blow up with it and also add a test based on the actual reproducer that was given. |
|
podman side containers/podman#25506 |
libimage/disk_usage.go
Outdated
| // does try to lookup this map as well but also falls back to reading | ||
| // the full big data files into memory for no apparent reason. | ||
| // Can we trust that the map always has the data we care about? | ||
| // Is there ever a case where this matters if the map does not have all the data? |
There was a problem hiding this comment.
There was a time where BigDataSizes did not exist.
… for about 3 months in 2016. Nominally, the code does not have any mechanism to refuse working on such old stores.
I don’t have a strong opinion on caring about this.
There was a problem hiding this comment.
ack thanks, I guess we don't worry about it then.
The old implementation only counted full images when sharing content between them. That is wrong, the store is layer based. We can have two images with no parent image that shares layers. As such get rid of the image tree that only is able to walk child/parent images. Instead we actually walk all layers now and correctly notice when they are shared. To this this correctly, first convert all layers to map so we can look them up by ID. And add missing size information if needed. Then we walk all images layers and count how often each layers is used. Then walk again but this time we know if the layer size must be shared or not so we can actually acount things correctly. Fixes: containers/podman#24452 Fixes: https://issues.redhat.com/browse/RHEL-29641 Signed-off-by: Paul Holzinger <pholzing@redhat.com>
There was a problem hiding this comment.
/lgtm
Failures in containers/podman#25506 seem to be the usual registry flakes only.
The old implementation only counted full images when sharing content between them. That is wrong, the store is layer based. We can have two images with no parent image that shares layers.
As such get rid of the image tree that only is able to walk child/parent images. Instead we actually walk all layers now and correctly notice when they are shared.
To this this correctly, first convert all layers to map so we can look them up by ID. And add missing size information if needed. Then we walk all images layers and count how often each layers is used. Then walk again but this time we know if the layer size must be shared or not so we can actually acount things correctly.
Fixes: containers/podman#24452
Fixes: https://issues.redhat.com/browse/RHEL-29641