[PR #136] [MERGED] refactor(snapshot): always use direct OverlayBD packaging #203

New issue

Closed

opened 2026-10-29 14:20:59 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-29 14:20:59 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/kvcache-ai/AgentENV/pull/136
Author: @Hygge-Gezelligheid
Created: 8/7/2026
Status: ✅ Merged
Merged: 8/12/2026
Merged by: @yingdi-shan

Base: main ← Head: remove-direct-overlaybd-option

📝 Commits (1)

82749b4 refactor(snapshot): always use direct OverlayBD packaging

📊 Changes

12 files changed (+46 additions, -276 deletions)

View changed files

📝 CLAUDE.md (+1 -1)
📝 config/default.toml (+2 -7)
📝 docs/src/configuration/reference.md (+1 -2)
📝 docs/src/internals/architecture.md (+1 -1)
📝 docs/src/internals/persistence-artifact-inventory.md (+2 -3)
📝 docs/src/internals/sandbox-testing.md (+4 -3)
📝 src/cfg.rs (+3 -19)
📝 src/sandbox/firecracker/instance.rs (+4 -19)
📝 src/sandbox/firecracker/overlaybd_snapshot.rs (+0 -136)
📝 src/sandbox/firecracker/sandbox.rs (+17 -40)
📝 src/setup/deps.rs (+0 -1)
📝 tests/integration/fc.rs (+11 -44)

📄 Description

What

Remove the memory_snapshot.direct_overlaybd option and always package memory snapshots through the direct OverlayBD path.

Remove the configuration field and environment variable.
Delete the legacy intermediate mem.bin path and its dedicated tests.
Keep both mincore-based page selection and opt-in dirty-page tracking on the direct path.
Update the configuration and architecture documentation.

Why

The direct OverlayBD path is already the default and is the path used by dirty-page tracking. Keeping an option for the legacy path adds configuration and branching without a current need. Making the direct path unconditional simplifies the snapshot flow and removes the unused implementation.

Follow-up to #134.

Scope and non-goals

Included:

Removal of the direct_overlaybd option and legacy snapshot branch.
Cleanup of code, tests, defaults, and documentation that only supported that branch.

Not included:

Changes to track_dirty_pages behavior or its default.
PVM dirty-page tracking support.
Snapshot epoch or chain changes discussed in #120.

Design and behavior changes

Memory snapshot capture now always creates a state-only Firecracker snapshot, queries dirty or present memory ranges, reads the selected pages, and writes the OverlayBD memory layer directly. It no longer creates an intermediate mem.bin file.

With track_dirty_pages=false, page selection keeps the existing mincore-based behavior. With it enabled, Firecracker uses KVM dirty-page tracking. Both modes use the same direct packaging path.

Compatibility and operations

Public API or generated protocol: N/A; no API or generated-code changes.
Configuration or defaults: memory_snapshot.direct_overlaybd and AGENTENV_MEMORY_SNAPSHOT_DIRECT_OVERLAYBD are removed. Existing direct_overlaybd=false entries are ignored by the current configuration parser, and snapshots use the direct path.
Snapshot manifest, artifact layout, or storage format: Unchanged. The temporary legacy mem.bin artifact is no longer produced.
Upgrade and rollback: Upgrades always use direct packaging. Rolling back restores the previous optional legacy path.
Host requirements, permissions, ports, or dependencies: Unchanged.

Validation

make fmt
make clippy
make test-unit
Relevant Rust integration tests
make -C services test (required when services/ changes)
Generated clients/server regenerated with the documented make target
Documentation updated
Benchmarks or performance comparison completed

Commands and results:

make fmt
# passed

make clippy
# passed

make test-unit
# passed

cargo test -p agentenv --lib -- --nocapture
# 710 passed, 0 failed, 4 ignored

cargo test -p agentenv --test integration fc::memory_snapshot_format_matches_config_and_resumes -- --nocapture
# passed with track_dirty_pages=false
# passed with AGENTENV_MEMORY_SNAPSHOT_TRACK_DIRTY_PAGES=true

git diff --check
# passed

Skipped checks and reasons:

make -C services test: no services/ changes.
Generated clients/server regeneration: no schema or generated-code changes.
Benchmarks: this removes an unused path without changing direct-path behavior.

Risks and reviewer notes

Deployments that explicitly set direct_overlaybd=false will use direct packaging after upgrading. The most important review points are the unconditional capture flow in src/sandbox/firecracker/sandbox.rs and the removal of the legacy helpers and integration test.

Checklist

The PR contains one coherent change and no unrelated formatting or refactoring.
New behavior is covered by tests, or I explained why testing is impractical.
Logs and examples contain no credentials, tokens, or private registry information.
I did not manually edit generated code without updating its source and regenerating it.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/kvcache-ai/AgentENV/pull/136 **Author:** [@Hygge-Gezelligheid](https://github.com/Hygge-Gezelligheid) **Created:** 8/7/2026 **Status:** ✅ Merged **Merged:** 8/12/2026 **Merged by:** [@yingdi-shan](https://github.com/yingdi-shan) **Base:** `main` ← **Head:** `remove-direct-overlaybd-option` --- ### 📝 Commits (1) - [`82749b4`](https://github.com/kvcache-ai/AgentENV/commit/82749b48ab7de4fea2b4ad7a952c35187cf29674) refactor(snapshot): always use direct OverlayBD packaging ### 📊 Changes **12 files changed** (+46 additions, -276 deletions) <details> <summary>View changed files</summary> 📝 `CLAUDE.md` (+1 -1) 📝 `config/default.toml` (+2 -7) 📝 `docs/src/configuration/reference.md` (+1 -2) 📝 `docs/src/internals/architecture.md` (+1 -1) 📝 `docs/src/internals/persistence-artifact-inventory.md` (+2 -3) 📝 `docs/src/internals/sandbox-testing.md` (+4 -3) 📝 `src/cfg.rs` (+3 -19) 📝 `src/sandbox/firecracker/instance.rs` (+4 -19) 📝 `src/sandbox/firecracker/overlaybd_snapshot.rs` (+0 -136) 📝 `src/sandbox/firecracker/sandbox.rs` (+17 -40) 📝 `src/setup/deps.rs` (+0 -1) 📝 `tests/integration/fc.rs` (+11 -44) </details> ### 📄 Description ## What Remove the `memory_snapshot.direct_overlaybd` option and always package memory snapshots through the direct OverlayBD path. - Remove the configuration field and environment variable. - Delete the legacy intermediate `mem.bin` path and its dedicated tests. - Keep both mincore-based page selection and opt-in dirty-page tracking on the direct path. - Update the configuration and architecture documentation. ## Why The direct OverlayBD path is already the default and is the path used by dirty-page tracking. Keeping an option for the legacy path adds configuration and branching without a current need. Making the direct path unconditional simplifies the snapshot flow and removes the unused implementation. ## Related issue Follow-up to #134. ## Scope and non-goals Included: - Removal of the `direct_overlaybd` option and legacy snapshot branch. - Cleanup of code, tests, defaults, and documentation that only supported that branch. Not included: - Changes to `track_dirty_pages` behavior or its default. - PVM dirty-page tracking support. - Snapshot epoch or chain changes discussed in #120. ## Design and behavior changes Memory snapshot capture now always creates a state-only Firecracker snapshot, queries dirty or present memory ranges, reads the selected pages, and writes the OverlayBD memory layer directly. It no longer creates an intermediate `mem.bin` file. With `track_dirty_pages=false`, page selection keeps the existing mincore-based behavior. With it enabled, Firecracker uses KVM dirty-page tracking. Both modes use the same direct packaging path. ## Compatibility and operations - Public API or generated protocol: N/A; no API or generated-code changes. - Configuration or defaults: `memory_snapshot.direct_overlaybd` and `AGENTENV_MEMORY_SNAPSHOT_DIRECT_OVERLAYBD` are removed. Existing `direct_overlaybd=false` entries are ignored by the current configuration parser, and snapshots use the direct path. - Snapshot manifest, artifact layout, or storage format: Unchanged. The temporary legacy `mem.bin` artifact is no longer produced. - Upgrade and rollback: Upgrades always use direct packaging. Rolling back restores the previous optional legacy path. - Host requirements, permissions, ports, or dependencies: Unchanged. ## Validation - [x] `make fmt` - [x] `make clippy` - [x] `make test-unit` - [x] Relevant Rust integration tests - [ ] `make -C services test` (required when `services/` changes) - [ ] Generated clients/server regenerated with the documented `make` target - [x] Documentation updated - [ ] Benchmarks or performance comparison completed Commands and results: ```text make fmt # passed make clippy # passed make test-unit # passed cargo test -p agentenv --lib -- --nocapture # 710 passed, 0 failed, 4 ignored cargo test -p agentenv --test integration fc::memory_snapshot_format_matches_config_and_resumes -- --nocapture # passed with track_dirty_pages=false # passed with AGENTENV_MEMORY_SNAPSHOT_TRACK_DIRTY_PAGES=true git diff --check # passed ``` Skipped checks and reasons: - `make -C services test`: no `services/` changes. - Generated clients/server regeneration: no schema or generated-code changes. - Benchmarks: this removes an unused path without changing direct-path behavior. ## Risks and reviewer notes Deployments that explicitly set `direct_overlaybd=false` will use direct packaging after upgrading. The most important review points are the unconditional capture flow in `src/sandbox/firecracker/sandbox.rs` and the removal of the legacy helpers and integration test. ## Checklist - [x] The PR contains one coherent change and no unrelated formatting or refactoring. - [x] New behavior is covered by tests, or I explained why testing is impractical. - [x] Logs and examples contain no credentials, tokens, or private registry information. - [x] I did not manually edit generated code without updating its source and regenerating it. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-29 14:20:59 +00:00