search

containerd / continuity

A transport-agnostic, filesystem metadata manifest system
https://containerd.io
Apache License 2.0
138 stars 66 forks source link

fs: add DiffDirChanges function to get changeset fast #145

Closed fuweid closed 5 months ago

fuweid commented 4 years ago

Since AUFS/OverlayFS can persist changeset in diff directory, DiffDirChanges function can retrieve layer changeset from diff directory without walking the whole rootfs directory.

Signed-off-by: Wei Fu fuweid89@gmail.com

fuweid commented 4 years ago

The original issue is from https://github.com/moby/buildkit/issues/1192. The docker uses graphdriver to retrieve the layer diff based on AUFS/OverlayFS and it is fast than containerd Compare API which walks the whole rootfs directory.

The github.com/containerd/continuity/fs package provides the Changes function, and it is hard to get relationship between two rootfs paths without original mount information, since the overlayfs mount info might be changed by Chdir if there are too many lowerdirs. Therefore, add new function named by DiffDirChanges and containerd Compare API can choose the efficient way to retrieve diff layer.

@dmcgowan @tonistiigi @AkihiroSuda

fuweid commented 3 years ago

@fuweid Are you still working on this?

Fixing slow diff has been gathering demand in the community (https://github.com/moby/buildkit/issues/1704, https://github.com/moby/buildkit/issues/1192).

Can we move this forward?

yeah. will update it in days～ Thanks！

pankajkumar229 commented 5 months ago

Why was this never merged?

AkihiroSuda commented 5 months ago

I think this can be just merged after rebasing and removing AUFS codes

fuweid commented 5 months ago

oops. Will update it later. Thanks for remider

pankajkumar229 commented 5 months ago

Many Thanks for the quick turnaround.

pankajkumar229 commented 5 months ago

Is it possible to get this change to a previous version of containerd when it gets released?

AkihiroSuda commented 5 months ago

Is it possible to get this change to a previous version of containerd when it gets released?

Unlikely, due to potential risk of regression.

pankajkumar229 commented 5 months ago

Is it possible to merge this soon? It will help us compile our own containerd. OTherwise I think we will need some code changes in containerd to use this branch or another fork.

Thanks again for the quick turnaround.

pankajkumar229 commented 5 months ago

We still see the slowness in commit. It is possibly because "nerdctl commit" is calling the function Compare inside containerd and it seems to create a two temporary mounts: upperRoot and lowerRoot. Both seem to have a full copy of the file system. Now when compare calls writeDiff, even if continuity can check for overlayfs, it is still going through a lot of files and taking forever to do "nerdctl commit". What would be a good way to fix this?

Screenshot 2024-01-27 at 3 50 18 PM

Although this belongs to the containerd repository , I thought I would ask here since I saw Mr. Kohei Tokunaga's name in the relevant code and it is possibly better discussed in one place.

pankajkumar229 commented 5 months ago

I created a (https://github.com/containerd/containerd/pull/9699/files) that essentially does not temp mount both upper and lower but only the lower. And then compares with the upper. It seems to work with the continuity changes in this pull request. Is this the correct approach? It is a pretty small change, just newline changes make it look large.