continuity icon indicating copy to clipboard operation
continuity copied to clipboard

[WIP|RFC] fs: add DiffDirChanges function to get changeset fast

Open fuweid opened this issue 6 years ago • 2 comments

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 [email protected]

fuweid avatar Nov 23 '19 15:11 fuweid

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 avatar Nov 23 '19 15:11 fuweid

@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!

fuweid avatar Jun 15 '21 02:06 fuweid

Why was this never merged?

pankajkumar229 avatar Jan 23 '24 16:01 pankajkumar229

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

AkihiroSuda avatar Jan 23 '24 16:01 AkihiroSuda

oops. Will update it later. Thanks for remider

fuweid avatar Jan 24 '24 01:01 fuweid

Many Thanks for the quick turnaround.

pankajkumar229 avatar Jan 24 '24 16:01 pankajkumar229

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

pankajkumar229 avatar Jan 25 '24 08:01 pankajkumar229

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

Unlikely, due to potential risk of regression.

AkihiroSuda avatar Jan 25 '24 08:01 AkihiroSuda

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 avatar Jan 25 '24 08:01 pankajkumar229

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 avatar Jan 27 '24 10:01 pankajkumar229

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.

pankajkumar229 avatar Jan 27 '24 13:01 pankajkumar229