fix(sandbox): shard the persistent serial log directory

[makhov]

What

Insert one shard directory level between the persistent serial-output root and each sandbox's own log directory, so the layout becomes {serial_dir}/{shard}/{sandbox_id}/, where the shard is the last two hex digits of the sandbox id.

Why

Serial output is deliberately persistent: {serial_dir}/{sandbox_id}/ outlives the sandbox so its boot log can be read after the fact. Nothing prunes it, so
on a busy node the root accumulates one child per sandbox ever created, at ~470 entries/sec under sustained create load. Nearly every child holds a single zero-byte firecracker-stderr.log — a guest resumed from a snapshot writes nothing to the console.

That cost is paid by every create, not by whoever reads a log: each create links a new entry into the root, under an exclusive lock on the parent inode.
At ~594k entries a node sustained 197–280 creates/sec; with the root emptied and nothing else changed, the same node sustained 460–475 creates/sec at 76–78% CPU — about 1.9× on create throughput, on a node that had simply been up a while. The inverse correlation between root size and peak create rate held across all four nodes measured.

The effect is invisible on a fresh node and grows silently with uptime, which is why it seems worth fixing in the layout rather than in an operator runbook.

Related issue

Closes #260

Scope and non-goals

Included:

• the path computation in FirecrackerSandbox::default_log_dir;
• a serial_shard helper with the rationale for the shard key;
• unit tests for the shard key, its spread across ids minted together, and the
  nil-id edge case;
• the two doc comments that stated the old layout.

Non-goals, deliberately:

• no pruning, rotation, or retention policy. This bounds the fan-out of the root, not the total number of directories. Retention is a separate discussion, and this change does not preclude one.
• no migration or cleanup of existing directories. A node that ran unsharded keeps its flat root; the change stops it regrowing. Operators who want the old entries gone still remove them once, and that is best done after deploying this — before it, the root just refills.
• no change to when serial files are created, what is written, or where the root itself lives.

Design and behavior changes

default_log_dir gains one join(serial_shard(self.id)) before the per-sandbox component. All three writers of that path — firecracker_stdout_path, firecracker_stderr_path, and firecracker_log_path — already route through default_log_dir, so one edit covers every producer, and the directory is created by the same code that created it before.

The shard is the id tail, not its head. Sandbox ids are UUIDv7, whose leading bytes are a millisecond timestamp: every sandbox created in the same period shares a prefix, so head-sharding would put an entire burst — exactly the load that causes the problem — into one bucket. The trailing bytes are random. Minting 411,340 ids inside a single millisecond and sharding them both ways gives 256 buckets of at most ~1.7k entries on the tail, against one bucket holding all 411,340 on the head. serial_shard_spreads_ids_created_together is
the regression test for that property.

Two hex digits (256 buckets) is sized for the observed regime: it holds the measured ~600k directories at a few thousand children each, well inside the range where directory insertion is not the bottleneck.

Compatibility and operations

• Public API or generated protocol: N/A — the path is internal and is not reported over the API.
• Configuration or defaults: no key added, removed, or re-interpreted. serial_dir / AENV_FIRECRACKER_SERIAL_DIR keep their meaning; only the layout beneath the root changes. Explicit stdout_path / stderr_path overrides bypass this path entirely and are unaffected.
• Snapshot manifest, artifact layout, or storage format: the on-disk layout of the serial log directory changes. Nothing in the tree reads it back by reconstructing {serial_dir}/{sandbox_id} — every use of serial_output_base_dir and serial_dir is either this computation or config plumbing — so the change is self-contained in-tree. Out-of-tree tooling that globs one level deep is the one thing that breaks; see risks.
• Host requirements, permissions, ports, or dependencies: none.

Validation

• make fmt
• make clippy — ran the narrower equivalent for the touched crate, see below
• make test-unit — ran the touched tests only, see below
• 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:

$ cargo fmt --all -- --check
  (no output: clean)

$ cargo test -p agentenv --lib serial_shard
running 3 tests
test sandbox::firecracker::sandbox::tests::serial_shard_is_the_id_tail ... ok
test sandbox::firecracker::sandbox::tests::serial_shard_handles_the_nil_id ... ok
test sandbox::firecracker::sandbox::tests::serial_shard_spreads_ids_created_together ... ok
test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 815 filtered out

$ cargo clippy -p agentenv --lib --all-features -- -D warnings
    Finished `dev` profile [unoptimized + debuginfo] target(s)
  (no warnings)

rustc 1.98.1 (48a229cea 2026-09-01), rustfmt 1.9.0-stable
Run in the project's own build environment (rust:1-bookworm with clang,
libclang-dev, libprotobuf-dev, protobuf-compiler), aarch64-unknown-linux-gnu.

Skipped checks and reasons:

• make clippy and make test-unit in full: not run. I ran clippy for the crate I touched (-p agentenv --lib --all-features -- -D warnings, clean) and the three new tests, rather than the whole workspace and the full unit-test target — the latter also drives uvm-ublk and the ignored capability tests, which want a configured ublk host and root. Happy to post a full run if you would rather see one before reviewing; CI will cover it in any case.
• Integration tests: not run — they need root, /dev/kvm, and network namespaces, which I do not have available for this change. The touched code is a pure path computation with no privileged behavior, and the three writers that consume it are unchanged.
• make -C services test: nothing under services/ changes.
• Generated code: none touched.
• Documentation: no prose docs describe this layout; the two doc comments that did are updated in the diff.
• Benchmarks: the throughput numbers above are production measurements of the pathological state and of the same node with the root emptied, not a repeatable in-tree benchmark — reproducing the 1.9× requires ~600k directories first. The shard-key property is covered by a unit test instead.

Happy to run something more specific if you want it.

Risks and reviewer notes

• The one real compatibility risk is out-of-tree tooling that walks {serial_dir}/{sandbox_id}/. Anything globbing one level deep silently finds nothing after this change rather than erroring. If you would rather this be opt-in behind a config key, or want it called out in release notes, I am happy to add either.
• This spreads, it does not bound. The root's fan-out is fixed and each bucket now grows instead — 256× the headroom, but without retention the problem returns much further out. I think that is the right split (small, safe change now; retention is a policy decision), but if you would prefer them together, say so.
• Shard width is a judgment call. 256 buckets fits the measured ~600k. If you expect an order of magnitude more per node, three digits is the safer pick — trivial to change, and the tests are width-agnostic apart from the length assertion.
• serial_shard falls back to "00" for an id whose string form has fewer than two hex digits, which is unreachable for a parsed UUID; the alternative was a panic on a logging path.
• Most interesting file: src/sandbox/firecracker/sandbox.rs — the one-line change in default_log_dir and the serial_shard helper below it.

[github-actions]

🔍 OpenCodeReview found 1 issue(s) in this PR.

• ✅ Successfully posted inline: 1 comment(s)

[github-actions]

The user-facing documentation still describes the old {serial_dir}/{sandbox_id}/ layout (notably docs/src/configuration/env-vars.md and docs/src/internals/persistence-artifact-inventory.md). Because this changes the externally observable location of persistent logs, please update those references to include {shard} so operators and log tooling do not look in the wrong path.

[makhov]

Docs updated

[github-actions]

serial_shard currently formats the UUID, scans and collects all 32 hexadecimal digits, and then allocates the two-character tail; default_log_dir subsequently formats the same ID again. This runs on every sandbox launch in the high-rate path being optimized. Since SandboxId::into_inner() exposes the UUID, derive the shard directly from its final byte (for example, format!("{:02x}", id.into_inner().as_bytes()[15])) to remove the full-string formatting/filter allocation while preserving the final-two-hex-digits layout.

Suggestion:

Suggested change

	fn serial_shard(id: SandboxId) -> String {
	let hex: String = id
	.to_string()
	.chars()
	.filter(|c| c.is_ascii_hexdigit())
	.collect();
	// Hex digits are ASCII, so a byte range is also a character boundary.
	match hex.get(hex.len().saturating_sub(2)..) {
	Some(tail) if tail.len() == 2 => tail.to_string(),
	// Unreachable for a parsed UUID; a directory named "00" beats a panic.
	_ => "00".to_string(),
	}
	}
	fn serial_shard(id: SandboxId) -> String {
	format!("{:02x}", id.into_inner().as_bytes()[15])
	}

[makhov]

Fixed!

[github-actions]

This unconditionally redirects a persisted sandbox's logs to the new sharded path. After an upgrade, resuming an existing sandbox whose logs are under {root}/{sandbox_id} will create a second log directory, so its stdout/stderr history is split and readers resolving through these methods can no longer find the earlier files. Please preserve rollout compatibility, for example by using the legacy sandbox directory when it already exists and the sharded directory otherwise (or by explicitly migrating it before use). Existing IDs are bounded, so such a fallback still avoids adding new entries to the root.

[makhov]

That's worth discussing with a human.

The legacy directory can only exist for ids restored from persistence, so when a persisted sandbox is loaded we can check if {root}/{id} exists, and point its overrides at it ignoring default log location.
If this is a good idea, I can implement it.