Skip to content

Commit 6988cb0

Browse files
huajqyingdi-shan
huajq
authored and
yingdi-shan
committed
fix(snapshot): revert digest tracking on compaction output paths
5afd9d1 wrapped every compact_to output in DigestTrackingFile + OrderedWriter so OSS/ACR publish could skip re-hashing sealed layers. On all snapshot packing paths this: - forces ordered writes, dropping compaction write concurrency from 32 to 1 on the raw output path (the default for memory snapshots and the only mode for rootfs lower rewriting), and - adds synchronous software SHA256 over every written byte on the capture hot path. The descriptor it produces is only consumed by the OSS repository backend, the ACR source-registry publisher, and (opportunistically) P2P content-addressed layer advertisement; posix_fs and local-only snapshots pay the cost with zero benefit. Even for OSS the trade is negative in the standard flow: publish runs in the same synchronous request as capture with a warm page cache, so the saved re-read costs nothing, while the serial-write penalty is real. Inherited layers already carry descriptors from the parent manifest via OSS runtime resolution, so there is no per-publish re-hash multiplier either. Revert the descriptor plumbing (compact_layers, publish_memory_overlaybd_layer, build_mem_snapshot_image_config and their callers) and remove DigestTrackingFile/OrderedWriter. Keep the OSS multipart upload parallelization from the same commit (64 MiB parts x 8 concurrent), which is unrelated and beneficial. Publish paths without descriptors already fall back to explicit hashing (import_managed_layer_by_hash / dense export), and P2P falls back to uuid-keyed layer artifacts. Fixes kvcache-ai#184.
1 parent e4ae61d commit 6988cb0

6 files changed

Lines changed: 23 additions & 231 deletions

File tree

‎src/sandbox/firecracker/overlaybd_snapshot.rs‎

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ use super::sandbox::managed_snapshot_base;
2828
use crate::cfg::ConfigManager;
2929
use crate::sandbox::ublk::UblkDevice;
3030
use crate::sandbox::ublk::{
31-
compact_layers, create_commit_args_with_digest, OverlaybdCompactOutput, UblkDeviceManager,
31+
compact_layers, create_commit_args, OverlaybdCompactOutput, UblkDeviceManager,
3232
};
3333
use crate::sandbox::SandboxCaptureError;
3434

@@ -66,7 +66,6 @@ fn into_terminal_snapshot_failure(err: anyhow::Error) -> anyhow::Error {
6666
SandboxCaptureError::terminal(err).into()
6767
}
6868

69-
#[cfg(test)]
7069
fn local_layer_config(path: &Path) -> LayerConfig {
7170
local_layer_config_with_descriptor(path, None)
7271
}
@@ -433,17 +432,14 @@ async fn rewrite_lowers_with_runtime_roots(
433432
if runtime_suffix.iter().any(|lower| lower.file.is_empty()) {
434433
anyhow::bail!("cannot compact runtime-owned overlaybd suffix with remote lowers");
435434
}
436-
if let Some((compacted_path, compacted_descriptor)) = compact_layers(
435+
if let Some(compacted_path) = compact_layers(
437436
&runtime_suffix,
438437
&output_dir.join(compaction_output_name),
439438
compaction_output,
440439
)
441440
.await?
442441
{
443-
lowers.push(local_layer_config_with_descriptor(
444-
&compacted_path,
445-
compacted_descriptor.as_ref(),
446-
));
442+
lowers.push(local_layer_config(&compacted_path));
447443
}
448444
return Ok(lowers);
449445
}
@@ -564,13 +560,12 @@ async fn capture_live_overlaybd_snapshot(
564560
pub(super) async fn build_mem_snapshot_image_config(
565561
resume_mem_image_config_path: Option<&Path>,
566562
new_layer_path: &Path,
567-
new_layer_descriptor: Option<&overlaybd::LayerDescriptor>,
568563
output_dir: &Path,
569564
memory_output: OverlaybdCompactOutput,
570565
) -> Result<ImageConfig> {
571566
let inherited_image_config =
572567
load_existing_image_config(resume_mem_image_config_path, "memory snapshot")?;
573-
let new_layer = local_layer_config_with_descriptor(new_layer_path, new_layer_descriptor);
568+
let new_layer = local_layer_config(new_layer_path);
574569
let lowers = rewrite_lowers_with_owned_runtime_suffix(
575570
inherited_image_config.lowers,
576571
output_dir,
@@ -645,7 +640,7 @@ async fn publish_memory_overlaybd_layer(
645640
output_path: &Path,
646641
mode: OverlaybdCompactOutput,
647642
concurrency: usize,
648-
) -> Result<Option<overlaybd::LayerDescriptor>> {
643+
) -> Result<()> {
649644
let lower_tmp = output_path.with_extension("commit.tmp");
650645
let build_result = async {
651646
let io_ring = shared_transient_io_ring();
@@ -654,8 +649,7 @@ async fn publish_memory_overlaybd_layer(
654649
.await
655650
.with_context(|| format!("create temp lower failed: {}", lower_tmp.display()))?,
656651
);
657-
let (commit_args, digest_tracker) =
658-
create_commit_args_with_digest(output_file, mode, concurrency).await?;
652+
let commit_args = create_commit_args(output_file, mode, concurrency).await?;
659653
compact_to(src_layers, mappings, virtual_size, commit_args)
660654
.await
661655
.context("compact memory layer")?;
@@ -667,7 +661,7 @@ async fn publish_memory_overlaybd_layer(
667661
output_path.display()
668662
)
669663
})?;
670-
Ok(digest_tracker.descriptor())
664+
Ok(())
671665
}
672666
.await;
673667

@@ -767,7 +761,7 @@ pub(crate) async fn convert_dirty_memory_to_overlaybd(
767761
dirty_ranges: &DirtyMemoryRanges,
768762
output_dir: &Path,
769763
mode: OverlaybdCompactOutput,
770-
) -> Result<(PathBuf, u64, Option<overlaybd::LayerDescriptor>)> {
764+
) -> Result<(PathBuf, u64)> {
771765
tokio::fs::create_dir_all(output_dir)
772766
.await
773767
.with_context(|| format!("create mem overlaybd dir: {}", output_dir.display()))?;
@@ -776,7 +770,7 @@ pub(crate) async fn convert_dirty_memory_to_overlaybd(
776770
let (mappings, memory_size) = dirty_ranges_to_segment_mappings(dirty_ranges)?;
777771
let source_file: Arc<dyn VirtualFile> = Arc::new(ProcessVmReader::new(firecracker_pid));
778772
let src_layers = vec![source_file];
779-
let descriptor = publish_memory_overlaybd_layer(
773+
publish_memory_overlaybd_layer(
780774
&src_layers,
781775
&mappings,
782776
memory_size,
@@ -787,7 +781,7 @@ pub(crate) async fn convert_dirty_memory_to_overlaybd(
787781
.await
788782
.context("compact dirty memory ranges as overlaybd layer")?;
789783

790-
Ok((data_path, memory_size, descriptor))
784+
Ok((data_path, memory_size))
791785
}
792786

793787
#[cfg(test)]
@@ -1081,7 +1075,6 @@ mod tests {
10811075
let image_config = build_mem_snapshot_image_config(
10821076
Some(&parent_config_path),
10831077
&new_layer,
1084-
None,
10851078
temp.path(),
10861079
OverlaybdCompactOutput::Raw,
10871080
)

‎src/sandbox/firecracker/sandbox.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -681,7 +681,7 @@ impl FirecrackerSandbox {
681681
let memory_output = OverlaybdCompactOutput::from_memory_snapshot_config(
682682
&ConfigManager::global_config().memory_snapshot,
683683
);
684-
let (mem_layer_path, mem_virtual_size, mem_layer_descriptor) = self
684+
let (mem_layer_path, mem_virtual_size) = self
685685
.snapshot_memory_to_overlaybd(&vm_state_path, snapshot_dir, memory_output)
686686
.await?;
687687

@@ -697,7 +697,6 @@ impl FirecrackerSandbox {
697697
let mem_image_config = build_mem_snapshot_image_config(
698698
resume_mem_image_config_path,
699699
&mem_layer_path,
700-
mem_layer_descriptor.as_ref(),
701700
snapshot_dir,
702701
memory_output,
703702
)
@@ -821,7 +820,7 @@ impl FirecrackerSandbox {
821820
vm_state_path: &Path,
822821
snapshot_dir: &Path,
823822
memory_output: OverlaybdCompactOutput,
824-
) -> Result<(PathBuf, u64, Option<overlaybd::LayerDescriptor>)> {
823+
) -> Result<(PathBuf, u64)> {
825824
let mem_overlaybd_dir = snapshot_dir.join("mem_overlaybd");
826825
let firecracker_pid = self.fc_instance.pid()?;
827826
self.fc_instance

‎src/sandbox/ublk/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,5 @@ pub(crate) use device::{SharedMemDevice, UblkCreateSpec, UblkDevice};
55
pub use device::{UblkBackend, UblkConfig, UblkDaemonConfig, UblkDeviceManager};
66
pub use overlaybd::OverlaybdConfig;
77
pub(crate) use overlaybd::{
8-
compact_layers, create_commit_args_with_digest, OverlaybdCompactOutput, OverlaybdRuntimeHandle,
8+
compact_layers, create_commit_args, OverlaybdCompactOutput, OverlaybdRuntimeHandle,
99
};

‎src/sandbox/ublk/overlaybd.rs‎

Lines changed: 10 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,9 @@ use std::sync::Arc;
44
use anyhow::{Context, Result};
55
use overlaybd::backend::local::LocalFile;
66
use overlaybd::config::{LayerConfig, UpperMode};
7-
use overlaybd::index_file::{merge_files_ro, CommitArgs, DigestTrackingFile, OrderedWriter};
7+
use overlaybd::index_file::{merge_files_ro, CommitArgs};
88
use overlaybd::virtual_file::VirtualFile;
99
use overlaybd::zfile::{CompressArgs, CompressOptions, ZFileCompactWriter};
10-
use overlaybd::LayerDescriptor;
1110
use serde::{Deserialize, Serialize};
1211
use tracing::debug;
1312
use uuid::Uuid;
@@ -70,20 +69,6 @@ pub(crate) async fn create_commit_args(
7069
Ok(args)
7170
}
7271

73-
/// Like [`create_commit_args`], but tracks a content descriptor (streaming
74-
/// sha256 + size) of the sealed output at no extra I/O cost. The descriptor
75-
/// is `None` when writes were not strictly ordered.
76-
pub(crate) async fn create_commit_args_with_digest(
77-
output: Arc<dyn VirtualFile>,
78-
mode: OverlaybdCompactOutput,
79-
concurrency: usize,
80-
) -> Result<(CommitArgs, Arc<DigestTrackingFile>)> {
81-
let tracker = Arc::new(DigestTrackingFile::new(output));
82-
let mut args = create_commit_args(tracker.clone(), mode, concurrency).await?;
83-
args.writer = Arc::new(OrderedWriter::new(args.writer.clone()));
84-
Ok((args, tracker))
85-
}
86-
8772
#[derive(Clone, Debug, Serialize, Deserialize)]
8873
pub struct OverlaybdConfig {
8974
pub image_config_path: PathBuf,
@@ -114,7 +99,7 @@ pub(crate) async fn compact_layers(
11499
layers: &[LayerConfig],
115100
output_path: &Path,
116101
mode: OverlaybdCompactOutput,
117-
) -> Result<Option<(PathBuf, Option<LayerDescriptor>)>> {
102+
) -> Result<Option<PathBuf>> {
118103
if layers.is_empty() {
119104
return Ok(None);
120105
}
@@ -141,14 +126,13 @@ pub(crate) async fn compact_layers(
141126
}
142127

143128
let lower_tmp = output_path.with_extension(format!("commit.{}.tmp", Uuid::now_v7()));
144-
let build_result: Result<Option<LayerDescriptor>> = async {
129+
let build_result: Result<()> = async {
145130
let output_file: Arc<dyn VirtualFile> = Arc::new(
146131
LocalFile::new(&lower_tmp, io_ring)
147132
.await
148133
.context("create compacted layer output file")?,
149134
);
150-
let (commit_args, digest_tracker) =
151-
create_commit_args_with_digest(output_file, mode, 32).await?;
135+
let commit_args = create_commit_args(output_file, mode, 32).await?;
152136
merge_files_ro(&src_files, commit_args)
153137
.await
154138
.context("merge layers")?;
@@ -160,21 +144,21 @@ pub(crate) async fn compact_layers(
160144
output_path.display()
161145
)
162146
})?;
163-
Ok(digest_tracker.descriptor())
147+
Ok(())
164148
}
165149
.await;
166150

167151
if build_result.is_err() {
168152
let _ = tokio::fs::remove_file(&lower_tmp).await;
169153
}
170-
let descriptor = build_result?;
154+
build_result?;
171155

172156
debug!(
173157
output = %output_path.display(),
174158
input_layers = layers.len(),
175159
"compacted overlaybd layers"
176160
);
177-
Ok(Some((output_path.to_path_buf(), descriptor)))
161+
Ok(Some(output_path.to_path_buf()))
178162
}
179163

180164
#[cfg(test)]
@@ -332,23 +316,12 @@ mod tests {
332316
),
333317
] {
334318
let output_path = temp.path().join(format!("compacted-{name}.commit"));
335-
let (published, descriptor) = compact_layers(&layers, &output_path, mode)
319+
let published = compact_layers(&layers, &output_path, mode)
336320
.await
337-
.expect("compact mixed raw/lz4/zstd layers")
338-
.expect("compaction produces an output");
339-
assert_eq!(published, output_path);
321+
.expect("compact mixed raw/lz4/zstd layers");
322+
assert_eq!(published.as_deref(), Some(output_path.as_path()));
340323
assert!(!output_path.with_extension("commit.tmp").exists());
341324

342-
// The streamed digest must match a plain sha256 of the output file.
343-
let descriptor = descriptor.expect("digest descriptor for compacted layer");
344-
let file_bytes = std::fs::read(&output_path).expect("read compacted output");
345-
assert_eq!(descriptor.size, file_bytes.len() as u64, "mode {name}");
346-
assert_eq!(
347-
descriptor.digest,
348-
crate::digest::sha256_digest(&file_bytes),
349-
"mode {name}"
350-
);
351-
352325
let (zfile_flag, data) = read_sealed_layer(&output_path, vsize as usize).await;
353326
assert_eq!(zfile_flag, expected_zfile, "mode {name}");
354327
assert_eq!(data, expected, "mode {name}");

0 commit comments

Comments
 (0)