Basekick-Labs / Basekick-Labs/arc

backup/restore: the source→temp copy lost its zero-copy fast path to the tracking writer; get it back without losing failure attribution

Open
#791 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
677
Forks
53
Avg merge
9h 14m
Merged PRs (30d)
164

Description

Follow-up to #778 (restore) and #784 (backup). Both wrap the per-file temp file in `trackingWriter` so a `ReadTo` failure can be attributed to the destination (temp filesystem full) rather than the source. The wrapper is deliberate and documented, but it has a cost: `io.Copy` only takes the `copy_file_range` path on Linux when the destination is an `*os.File`, so the source→temp hop now runs through a 32 KiB userspace loop. Restore is rare and disk-bound; backup runs on a schedule over the whole store, so on Linux with local data storage this is a measurable CPU cost per backup.

## Options

1. **Implement `io.ReaderFrom` on `trackingWriter`** delegating to the underlying `*os.File`'s `ReadFrom`, which restores `copy_file_range` when the source is also an `*os.File` (the local backend). The catch: a kernel copy reports one error for either side, so attribution has to come from somewhere else.
2. **Attribute on failure by probing the destination:** after a failed `ReadTo`, attempt a small write to the temp file. If the probe fails (ENOSPC, EDQUOT, EBADF, EIO on the temp volume) the destination is at fault; otherwise treat it as a source read. Heuristic, but it only runs on the failure path, and a transient condition that clears between failure and probe degrades to today's pre-#778 behaviour, not worse.
3. Keep the wrapper only for backends whose `ReadTo` is not a local file copy (S3/Azure stream into the temp file, where `copy_file_range` never applied anyway) and use the bare `*os.File` for `LocalBackend`. Simplest, but it means two code paths.

Measure before choosing: a 1 GiB file, `io.Copy(dst *os.File, src *os.File)` vs `io.Copy(&trackingWriter{dst}, src)` on Linux (macOS has no zero-copy path, so the difference cannot be seen there). The existing seam tests (`TestCopyDataFiles_TempWriteFailureIsFatalNotSkipped`, `TestRestore_TempWriteFailureIsFatalNotSkipped`) are the contract any option must keep green.

Contributor guide

Open the contributing guide

Research direction

Start by benchmarking a 1 GiB local-file copy with io.Copy to an *os.File versus through trackingWriter on Linux, as described in the issue. Then inspect trackingWriter, LocalBackend, and the existing seam tests TestCopyDataFiles_TempWriteFailureIsFatalNotSkipped and TestRestore_TempWriteFailureIsFatalNotSkipped. Done means the chosen approach restores the measurable fast path while keeping both failure-attribution tests green.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, linux
Domain
backend, databases, performance
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.