walking differ: use DiffDirChanges for overlay fast path#12842
walking differ: use DiffDirChanges for overlay fast path#12842gogasca wants to merge 2 commits intocontainerd:mainfrom
Conversation
gogasca
force-pushed
the
overlay-fast-diff
branch
2 times, most recently
from
59bcbbe to
aed1ff0
Compare
gogasca
force-pushed
the
overlay-fast-diff
branch
from
807f421 to
65d2a0a
Compare
|
There is a CI failure, the node-e2e.yml workflow needs fetch-depth: 0 added to the |
gogasca
force-pushed
the
overlay-fast-diff
branch
from
65d2a0a to
a5c8525
Compare
|
Thank you, which release will have the fixes? |
gogasca
force-pushed
the
overlay-fast-diff
branch
from
a5c8525 to
82accc3
Compare
|
@pankajkumar229 thanks for responding, I'm waiting for a repo owner to take a look at this PR and provide this info (I see versions: 2.x or 1.7x ) . Do you know who may be able to help so I can tag them here? Thanks! |
gogasca
force-pushed
the
overlay-fast-diff
branch
from
82accc3 to
f16a445
Compare
|
Hi, @AkihiroSuda Could you please review when you have time ? |
gogasca
force-pushed
the
overlay-fast-diff
branch
from
f16a445 to
7b28a32
Compare
Any benchmark results? |
Added three benchmarks to plugins/diff/overlay/differ_linux_test.go: With 200 files in the lower layer and 10 changed files in the upper layer, the overlay single-walk approach is ~9x faster with ~11x fewer allocations — and the gap grows with larger |
gogasca
force-pushed
the
overlay-fast-diff
branch
2 times, most recently
from
42a5e9a to
0fe5bc4
Compare
gogasca
force-pushed
the
overlay-fast-diff
branch
4 times, most recently
from
a926f50 to
903dea3
Compare
gogasca
force-pushed
the
overlay-fast-diff
branch
from
903dea3 to
9aec0cd
Compare
|
Please squash the commits |
gogasca
force-pushed
the
overlay-fast-diff
branch
from
9aec0cd to
92bc582
Compare
done. Thanks for looking into it. |
dmcgowan
left a comment
dmcgowan
left a comment
There was a problem hiding this comment.
The single walk differ code is pretty old and won't correctly handle overlay's metacopy or redirect options. I worry this may have some lurking issues we need to fully address, either in containerd to skip in those cases or in continuity to properly handle those xattrs. I'm curious if this is likely to hit in build cases or if redirect/metacopy is not really used by builders.
Looks like the same is true of the erofs differ, but that snapshotter is newer and hasn't been as thoroughly used.
| cw := archive.NewChangeWriter(io.Discard, upperDir) | ||
| // Walk only the upperdir; lowerDir is referenced but not traversed here. | ||
| // In production this uses fs.DiffDirChanges + DiffSourceOverlayFS which | ||
| // reads overlay xattrs. Here we use filepath.Walk for pure I/O comparison. |
There was a problem hiding this comment.
Why does this need to be benchmarked then? Its just benchmarking walking a directory.
There was a problem hiding this comment.
You're right. The benchmarks use filepath. Walk as a proxy rather than the actual DiffDirChanges + DiffSourceOverlayFS path (which requires real overlay mounts and is not runnable in unit test environments). They don't accurately reflect the production code path and are essentially pure IO benchmarks. I'll remove them.
Move the overlay fast-diff logic from the walking differ into a dedicated differ package. Register the overlay differ with higher priority so it is used by default when overlay is available. - Introduce diff/overlay package with OverlayDiffer - Walking differ delegates to DiffDirChanges for overlay fast path - Integration tests updated to reflect new differ registration order - Fix migration tests for 1.6/1.7 configs without overlay differ Signed-off-by: Gonzalo Gasca Meza <[email protected]>
Fuzz inputs with randomly generated mount structures could cause Compare and Apply to hang indefinitely when accessing non-existent paths. Add a 25s context timeout to guard against this. Signed-off-by: Gonzalo Gasca Meza <[email protected]>
gogasca
force-pushed
the
overlay-fast-diff
branch
from
92bc582 to
331206f
Compare
|
dmcgowan@ can you please take another look? Thanks! |
5 participants
Summary
Enables fast overlay diff using continuity's DiffDirChanges function for active snapshots with overlay mounts. This avoids walking the entire rootfs and instead leverages
overlayfs's native diff directory.
Changes
Fixes containerd/continuity#236
Related #9663