atelet: preserve holes when copying a local checkpoint - #769
atelet: preserve holes when copying a local checkpoint#769Benjamin Elder (BenTheElder) wants to merge 2 commits into
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. |
| } | ||
| return fmt.Errorf("seeking to data at %d: %w", off, err) | ||
| } | ||
| holeOff, err := unix.Seek(fd, dataOff, unix.SEEK_HOLE) |
There was a problem hiding this comment.
If unix.Seek(unix.SEEK_HOLE) returns holeOff <= off (for instance, if concurrent modifications or a non-standard filesystem implementation returns an offset that fails to advance past off), off = holeOff will fail to progress, causing an infinite loop. Additionally, if dataOff >= size is returned, unix.Seek(dataOff, unix.SEEK_HOLE) would query past EOF.
if dataOff >= size {
break
}
holeOff, err := unix.Seek(fd, dataOff, unix.SEEK_HOLE)
if err != nil {
return fmt.Errorf("seeking to hole at %d: %w", dataOff, err)
}
if holeOff <= off {
return fmt.Errorf("seek to hole at %d returned non-advancing offset %d", dataOff, holeOff)
}
| // 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
}
}
|
|
||
| // 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.
|
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. |
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.