erofs-differ: support zstd-wrapped EROFS layers#13185
Conversation
dmcgowan
left a comment
There was a problem hiding this comment.
We can figure out chain logic later, if necessary
| if err != nil { | ||
| return emptyDesc, err | ||
| } | ||
| _, err = io.Copy(f, content.NewReader(ra)) |
There was a problem hiding this comment.
The replacement does not allow the optimization that this code does, this allows copy file range when there is no decompresssion. The underlying reader supports directly copying from the content store file descriptor to another file descriptor. We should keep in a special path for uncompressed as this is the only case where content to snapshotter can be done like this.
There was a problem hiding this comment.
ok, let me update it to allow copy file range
There was a problem hiding this comment.
I added a fastcopy boolean: I admit it's not pretty clean, but I don't have much better ideas; I think we could clean it up later with chain logic changes...
There was a problem hiding this comment.
Pull request overview
Updates the EROFS differ to recognize and apply EROFS layers that are wrapped with a +zstd transport compression suffix, enabling faster pulls while keeping uncompressed EROFS blobs on disk.
Changes:
- Relax EROFS media type detection to allow
+suffixvariants. - In
Apply, map EROFS+zstdlayers onto the existing diff stream-processor chain to transparently decompress before writinglayer.erofs. - Return an updated descriptor for processed native EROFS layers (base media type, digest/size of the unwrapped blob), and add debug logging for native apply paths.
Comments suppressed due to low confidence (1)
plugins/diff/erofs/differ.go:188
diff.GetProcessormay return processors (notablybinaryProcessor) that only surface failures via anErr() errormethod; without checking that, stream-processor failures can be silently ignored and a truncated/invalid layer can be treated as successful. Consider tracking the processors in the chain (likecore/diff/apply/apply.go) and checkingErr()on any processor that implements it before returning.
processor := diff.NewProcessorChain(diffLayerType, content.NewReader(ra))
for {
if processor, err = diff.GetProcessor(ctx, processor, config.ProcessorPayloads); err != nil {
return emptyDesc, fmt.Errorf("failed to get stream processor for %s: %w", desc.MediaType, err)
}
if processor.MediaType() == ocispec.MediaTypeImageLayer {
break
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Although EROFS has native compression support (and each filesystem can contain multiple compression algorithms), in many cases, people only consider using zstd compression when transporting on the wire in order to reduce the pulling time but maintain the optimal runtime performance. Only `+zstd` is considered: it has skippable frames which will be used for the seekable EROFS implementation in future containerd versions. Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
|
@AkihiroSuda @dmcgowan thanks for approving this, let's merge this? |
Although EROFS has builtin native compression support (and each filesystem can contain multiple compression algorithms), in many cases, people only consider using zstd compression when transporting on the wire (see #12506) in order to reduce the pulling time but maintain the optimal runtime performance with uncompressed filesystems.
Only
+zstdis considered (IOWs,+gzipis never considered at all): it has skippable frames which will be used for the seekable EROFS implementation (#12703) in future containerd versions.Proposed mediatype: MediaTypeErofsLayer + "+zstd" ("application/vnd.erofs.layer.v1+zstd")
Test steps:
ctr i pull --snapshotter=erofs docker.io/hsiangkao/ubuntu:22.04-erofs-zstd ctr run --tty --snapshotter=erofs docker.io/hsiangkao/ubuntu:22.04-erofs-zstd testCc @anniecherk