atelet: preserve holes when copying a local checkpoint - #769
Conversation
copyFile used a plain io.Copy, which reads a hole as zeroes and writes it back as data. The biggest thing it copies is a guest memory image, which is mostly unallocated: staging a local checkpoint for a restore inflated a 164MiB snapshot into its full 2GiB logical size. That costs disk and I/O on every local checkpoint restore, and it compounds, because cloud-hypervisor then loads the whole image and the next checkpoint it writes is dense in turn. Five pause cycles on one actor left 8.2GB of local checkpoints where a few hundred MiB would do. Copy only the populated extents, located with SEEK_DATA and SEEK_HOLE, and fall back to the dense copy when the filesystem cannot report holes.
The dense io.Copy this replaced could hand the whole file to copy_file_range, since os.File implements ReadFrom. Copying extents through a userspace buffer gave that up, which would make a fully populated checkpoint — a guest that really did touch all its RAM — slower to stage than before. Copy each extent with copy_file_range where the destination exposes a descriptor, falling back to the buffered copy when the kernel or filesystem refuses (EXDEV across filesystems, ENOSYS on old kernels) or when the destination is not a file.
|
Dmitry Berkovich (@dberkov) this would've cost a lot for local snapshots with uVM specifically, not sure what you were testing. |
| // It reports errKernelCopyUnsupported when the kernel or filesystem cannot do the | ||
| // copy — most commonly EXDEV, when source and destination are on different | ||
| // filesystems — so the caller can fall back to a userspace copy. | ||
| func kernelCopyRange(srcFd, dstFd int, off, length int64) (int64, error) { |
There was a problem hiding this comment.
maxKernelCopy is set to 1 << 30 (1 GiB). Performing a 1 GiB copy_file_range syscall can take hundreds of milliseconds to seconds. In Go 1.14+, the Go runtime sends SIGURG signals for asynchronous goroutine preemption. If a signal is caught while copy_file_range is in progress before any bytes are transferred, unix.CopyFileRange returns unix.EINTR.
Currently, unix.EINTR is not handled in kernelCopyRange nor caught as unsupported in copySparse. As a result, copySparse returns copying ...: interrupted system call, aborting the entire checkpoint restore on a transient signal.
func kernelCopyRange(srcFd, dstFd int, off, length int64) (int64, error) {
if length > maxKernelCopy {
length = maxKernelCopy
}
roff, woff := off, off
for {
n, err := unix.CopyFileRange(srcFd, &roff, dstFd, &woff, int(length), 0)
if errors.Is(err, unix.EINTR) {
continue
}
if err != nil {
switch {
case errors.Is(err, unix.ENOSYS),
errors.Is(err, unix.EXDEV),
errors.Is(err, unix.EOPNOTSUPP),
errors.Is(err, unix.EPERM),
errors.Is(err, unix.EINVAL),
errors.Is(err, unix.EBADF):
return 0, errKernelCopyUnsupported
}
return 0, err
}
if n == 0 {
return 0, errKernelCopyUnsupported
}
return int64(n), nil
}
}
There was a problem hiding this comment.
fixed
|
|
||
| // allocatedBytes reports how much disk a file actually occupies, which is less than its | ||
| // size when it has holes. | ||
| func allocatedBytes(t *testing.T, path string) int64 { |
There was a problem hiding this comment.
nit:
copyrange_other.go was introduced so non-Linux platforms (e.g. macOS / Windows) can build cmd/atelet during local development. However, main_test.go lacks build tags, and syscall.Stat / syscall.Stat_t are not available on Windows (GOOS=windows), breaking go test ./cmd/atelet on Windows environments.
Suggested Fix: Move allocatedBytes and sparse assertions into main_linux_test.go (with //go:build linux), or wrap allocatedBytes with OS-specific helpers.
There was a problem hiding this comment.
cmd/atelet doesn't build for windows as-is before this PR, I'm not sure if we care about windows.
There was a problem hiding this comment.
I'd go so far as to say we actively don't care about windows, except perhaps client tools/libraries. In-cluster tools it is useful to support macOS developers where mostly things are the same anyhow, but windows is so different and I doubt we will target windows workloads.
|
Is new behavior applied for both gVisor & microVMs ? I think gVisor already does all the optimization and the new code might make gVisor work slower. |
Two robustness gaps from review: The extent loop trusted SEEK_HOLE to advance. A filesystem reporting a hole at or before where the iteration started would spin forever, and data reported past the size being copied would seek beyond EOF. Bail out on the latter and fail loudly on the former rather than hang. copy_file_range is interruptible and the Go runtime signals goroutines to preempt them, so EINTR is expected on a copy this large. It was treated as fatal, which would abort a restore on a transient signal; retry instead.
That's a fair concern, but the old routine wasn't actually faster, it was a naive io.Copy loop which is generic around readers/writers. This is an optimized file copy routine. It micro-benchmarks faster even for dense files. |
285232a
into
agent-substrate:main
2 participants
copyFile used a plain
io.Copy, which reads a hole as zeroes and writes it back as data. The biggest thing it copies is a guest memory image, which is mostly unallocated: staging a local checkpoint for a restore inflated a 164MiB snapshot into its full 2GiB logical size. (Note: That itself is a bug, see #679)That costs disk and I/O on every local checkpoint restore, and it compounds, because cloud-hypervisor then loads the whole image and the next checkpoint it writes is dense in turn. Five pause cycles on one actor left 8.2GB of local checkpoints where a few hundred MiB would do.
Copy only the populated extents, located with
SEEK_DATAandSEEK_HOLE, and fall back to the dense copy when the filesystem cannot report holes.