Repository navigation
atelet: stage local checkpoints by hard link instead of copying - #935
Jeffrey Ying (Jefftree) wants to merge 1 commit into
Conversation
98184b3 to
184e1a1
Compare
| // The cost is that a checkpoint failing from here on leaves no earlier snapshot | ||
| // to fall back to. That is survivable: the guest stays paused when | ||
| // CheckpointWorkload fails, so the checkpoint can simply be retried. | ||
| pruneLocalCheckpoints(ctx, actorUID) |
There was a problem hiding this comment.
🤖 question 🟢 – The stated mitigation covers an RPC failure but not a crash, and the reordering widens the window where neither snapshot exists.
A gap between the prune and moveLocalCheckpoint was already there, but it used to be short. It now spans the whole CheckpointWorkload call — pause, write the memory image, tear down — which is the expensive part. Through all of it the node holds no local snapshot while the actor's LocalSnapshotInfo still names the pruned one.
"The checkpoint can simply be retried" holds when the RPC returns an error, because the guest is still paused. It doesn't hold if atelet or the node dies mid-snapshot: nothing retries, and the actor is left pinned by RequiredNodes to a node whose local snapshot has been deleted. Before this change the same crash left the earlier snapshot intact and the resume worked.
Keeping the old snapshot isn't free — the merge would take the copying path at roughly 130ms instead of 14ms — so trading that for a wider crash window may well be the right call. Worth saying so in the comment, though, since it currently reads as if a retry always covers the cost.
There was a problem hiding this comment.
Human: We would probably prefer to keep the disk fuller than lose reliability.
There was a problem hiding this comment.
Yeah this is a tradeoff losing reliability for improved performance. It's also a suspend only optimization and resume seems more important. Removed the early pruning.
Bowei Du (bowei)
added
kind/feature
b947030 to
4039156
Compare
While benchmarking pause actor I ran into multiple bottlenecks. I'm currently importing agent-substrate#935 into my branch but agent-substrate#1876 is another alternative to solve the first bottleneck. The 2nd bottleneck (primary target of this PR) is due to fsyncs being called in writeFileAtomic() for sandbox-assets.json and SystemInfoVolume files. Because these are new files that require ext4 filesystem metadata updates, and we have ext4 mounted in ordered mode, calling fsync on the file + directory causes all other pending writes (such as other checkpoints) to be flushed. So the file creation ends up being blocked behind other actor's checkpoint syncs. These files do not actually need to be flushed to disk. SystemInfo is recreatable. sandbox-assets.json is written during Run/Resume(), and read during Checkpoint(). But once the checkpoint is finished, the manifest is saved with the snapshot. If the node crashes between Run/Resume and Checkpoint, the actor is crashed anyway. ### Single-Node Pause/Resume Benchmark Summary **Configuration:** 17 workers, 15 actors, `glutton` (`512 MiB` RAM capacity, `32 MiB` churn per cycle), `120s` run, `1.0s` wait, `100%` pause (`0%` suspend) | Metric | Baseline (`upstream/main`) | + Commit 1 (`os.Link` local checkpoint) | + Commit 2 (remove `fsync` on ephemeral volumes) | | :--- | :---: | :---: | :---: | | **Completed Cycles** (`120s`) | `106.5` (`112` pauses / `101` resumes) | `160.0` (`165` pauses / `155` resumes) | **`212.5`** (`214` pauses / `211` resumes) | | **Cycle Throughput** | `53.3 cycles/min` (`0.89/s`) | `80.0 cycles/min` (`1.33/s`, **+50.2%**) | **`106.3 cycles/min`** (`1.77/s`, **2.00x**) | | **`ResumeActor` p50** | `6,000 ms` | `4,100 ms` | **`210 ms`** (**28.6x faster**) | | **`ResumeActor` avg** | `7,023 ms` | `4,867 ms` | **`246 ms`** (**28.5x faster**) | | **`ResumeActor` p90 / p99** | `15,000 ms` / `19,000 ms` | `9,400 ms` / `20,000 ms` | **`350 ms` / `590 ms`** | - [x] Tests pass - [x] Appropriate changes to documentation are included in the PR --------- Co-authored-by: Jefftree <[email protected]>
Resuming from a node-local pause checkpoint copied the whole snapshot into the restore dir. Both dirs sit under the same actor dir, so link instead, falling back to a copy only on EXDEV.
Staging the image on resume drops from 105 ms to under 1 ms (kind, 2 GiB guest, ~133 MiB populated).
The staged files now share an inode with the cached snapshot, so nothing may write them in place.
rewriteSnapshotSocketPathswritesconfig.jsonvia temp file and rename, andMergeDeltaIntoBasecopies instead of overlaying in place when base has a second link. With the pinned CH v53 the merge is skipped, so suspend is unchanged. On OnDemand restore (CH < 53) the merge takes the copying path.