From 61f32e85554571b5cbb13f31ab1bc801f562c1f7 Mon Sep 17 00:00:00 2001 From: Yan Song Date: Fri, 17 Mar 2023 09:35:33 +0000 Subject: [PATCH] rafs: only enable digest validate based on configuration We found that when using the "nydus-image check --bootstrap /path/to/bootstrap" command, it takes about 15s to check a 35MB bootstrap file (rafs v5) due to the default digest validation. This is very slow and disabling it can reduce the time to 3s. We should only allow this option to be configurable at runtime. Signed-off-by: Yan Song --- rafs/src/builder/core/bootstrap.rs | 3 +-- rafs/src/builder/core/chunk_dict.rs | 2 +- rafs/src/fs.rs | 2 +- rafs/src/metadata/mod.rs | 6 +++++- service/src/blob_cache.rs | 2 +- src/bin/nydus-image/inspect.rs | 2 +- src/bin/nydus-image/main.rs | 2 +- src/bin/nydus-image/merge.rs | 9 ++++----- src/bin/nydus-image/stat.rs | 2 +- src/bin/nydus-image/unpack/mod.rs | 7 +------ src/bin/nydus-image/validator.rs | 2 +- 11 files changed, 18 insertions(+), 21 deletions(-) diff --git a/rafs/src/builder/core/bootstrap.rs b/rafs/src/builder/core/bootstrap.rs index f889a812fc7..7d0048ea20b 100644 --- a/rafs/src/builder/core/bootstrap.rs +++ b/rafs/src/builder/core/bootstrap.rs @@ -283,8 +283,7 @@ impl Bootstrap { blob_mgr: &mut BlobManager, ) -> Result { let rs = if let Some(path) = bootstrap_mgr.f_parent_path.as_ref() { - RafsSuper::load_from_file(path, ctx.configuration.clone(), true, false) - .map(|(rs, _)| rs)? + RafsSuper::load_from_file(path, ctx.configuration.clone(), false).map(|(rs, _)| rs)? } else { return Err(Error::msg("bootstrap context's parent bootstrap is null")); }; diff --git a/rafs/src/builder/core/chunk_dict.rs b/rafs/src/builder/core/chunk_dict.rs index 06f9e1a3a17..250262687e0 100644 --- a/rafs/src/builder/core/chunk_dict.rs +++ b/rafs/src/builder/core/chunk_dict.rs @@ -160,7 +160,7 @@ impl HashChunkDict { config: Arc, rafs_config: &RafsSuperConfig, ) -> Result { - let (rs, _) = RafsSuper::load_from_file(path, config, true, true) + let (rs, _) = RafsSuper::load_from_file(path, config, true) .with_context(|| format!("failed to open bootstrap file {:?}", path))?; let mut d = HashChunkDict { m: HashMap::new(), diff --git a/rafs/src/fs.rs b/rafs/src/fs.rs index d4ccf3b9619..1ecb6ae2723 100644 --- a/rafs/src/fs.rs +++ b/rafs/src/fs.rs @@ -84,7 +84,7 @@ impl Rafs { let cache_cfg = cfg.get_cache_config().map_err(RafsError::LoadConfig)?; let rafs_cfg = cfg.get_rafs_config().map_err(RafsError::LoadConfig)?; - let (sb, reader) = RafsSuper::load_from_file(path, cfg.clone(), false, false) + let (sb, reader) = RafsSuper::load_from_file(path, cfg.clone(), false) .map_err(RafsError::FillSuperblock)?; let blob_infos = sb.superblock.get_blob_infos(); let device = BlobDevice::new(cfg, &blob_infos).map_err(RafsError::CreateDevice)?; diff --git a/rafs/src/metadata/mod.rs b/rafs/src/metadata/mod.rs index a8ffb02bba1..f8e2a979f0f 100644 --- a/rafs/src/metadata/mod.rs +++ b/rafs/src/metadata/mod.rs @@ -683,9 +683,13 @@ impl RafsSuper { pub fn load_from_file>( path: P, config: Arc, - validate_digest: bool, is_chunk_dict: bool, ) -> Result<(Self, RafsIoReader)> { + let validate_digest = config + .rafs + .as_ref() + .map(|rafs| rafs.validate) + .unwrap_or_default(); let mut rs = RafsSuper { mode: RafsMode::Direct, validate_digest, diff --git a/service/src/blob_cache.rs b/service/src/blob_cache.rs index d38a59b00e5..20808cb5440 100644 --- a/service/src/blob_cache.rs +++ b/service/src/blob_cache.rs @@ -390,7 +390,7 @@ impl BlobCacheMgr { path: PathBuf, config: Arc, ) -> Result<()> { - let (rs, _) = RafsSuper::load_from_file(&path, config.clone(), true, false)?; + let (rs, _) = RafsSuper::load_from_file(&path, config.clone(), false)?; if rs.meta.is_v5() { return Err(einval!("blob_cache: RAFSv5 image is not supported")); } diff --git a/src/bin/nydus-image/inspect.rs b/src/bin/nydus-image/inspect.rs index d4ab7462b5e..85a444b1400 100644 --- a/src/bin/nydus-image/inspect.rs +++ b/src/bin/nydus-image/inspect.rs @@ -40,7 +40,7 @@ impl RafsInspector { request_mode: bool, config: Arc, ) -> Result { - let (rafs_meta, f) = RafsSuper::load_from_file(bootstrap_path, config, false, false)?; + let (rafs_meta, f) = RafsSuper::load_from_file(bootstrap_path, config, false)?; let root_ino = rafs_meta.superblock.root_ino(); Ok(RafsInspector { diff --git a/src/bin/nydus-image/main.rs b/src/bin/nydus-image/main.rs index 053fad79277..31a75886cbb 100644 --- a/src/bin/nydus-image/main.rs +++ b/src/bin/nydus-image/main.rs @@ -1028,7 +1028,7 @@ impl Command { Some(s) => PathBuf::from(s), }; - let (rs, _) = RafsSuper::load_from_file(&bootstrap_path, config.clone(), true, false)?; + let (rs, _) = RafsSuper::load_from_file(&bootstrap_path, config.clone(), false)?; info!("load bootstrap {:?} successfully", bootstrap_path); let chunk_dict = match matches.get_one::("chunk-dict") { None => None, diff --git a/src/bin/nydus-image/merge.rs b/src/bin/nydus-image/merge.rs index 96def7d27c5..b605bc3c945 100644 --- a/src/bin/nydus-image/merge.rs +++ b/src/bin/nydus-image/merge.rs @@ -111,7 +111,7 @@ impl Merger { let mut parent_layers = 0; if let Some(parent_bootstrap_path) = &parent_bootstrap_path { let (rs, _) = - RafsSuper::load_from_file(parent_bootstrap_path, config_v2.clone(), false, false) + RafsSuper::load_from_file(parent_bootstrap_path, config_v2.clone(), false) .context(format!("load parent bootstrap {:?}", parent_bootstrap_path))?; tree = Some(Tree::from_bootstrap(&rs, &mut ())?); let blobs = rs.superblock.get_blob_infos(); @@ -127,9 +127,8 @@ impl Merger { let mut chunk_dict_blobs = HashSet::new(); let mut config = None; if let Some(chunk_dict_path) = &chunk_dict { - let (rs, _) = - RafsSuper::load_from_file(chunk_dict_path, config_v2.clone(), true, false) - .context(format!("load chunk dict bootstrap {:?}", chunk_dict_path))?; + let (rs, _) = RafsSuper::load_from_file(chunk_dict_path, config_v2.clone(), false) + .context(format!("load chunk dict bootstrap {:?}", chunk_dict_path))?; config = Some(rs.meta.get_config()); for blob in rs.superblock.get_blob_infos() { chunk_dict_blobs.insert(blob.blob_id().to_string()); @@ -140,7 +139,7 @@ impl Merger { let mut chunk_size = None; for (layer_idx, bootstrap_path) in sources.iter().enumerate() { - let (rs, _) = RafsSuper::load_from_file(bootstrap_path, config_v2.clone(), true, false) + let (rs, _) = RafsSuper::load_from_file(bootstrap_path, config_v2.clone(), false) .context(format!("load bootstrap {:?}", bootstrap_path))?; config .get_or_insert_with(|| rs.meta.get_config()) diff --git a/src/bin/nydus-image/stat.rs b/src/bin/nydus-image/stat.rs index fe1eb2d2002..ce0ded45663 100644 --- a/src/bin/nydus-image/stat.rs +++ b/src/bin/nydus-image/stat.rs @@ -159,7 +159,7 @@ impl ImageStat { } pub fn stat(&mut self, path: &Path, is_base: bool, config: Arc) -> Result<()> { - let (rs, _) = RafsSuper::load_from_file(path, config, false, false)?; + let (rs, _) = RafsSuper::load_from_file(path, config, false)?; let mut dict = HashChunkDict::new(rs.meta.get_digester()); let mut hardlinks = HashSet::new(); let tree = diff --git a/src/bin/nydus-image/unpack/mod.rs b/src/bin/nydus-image/unpack/mod.rs index 4f1136331aa..0dad0a810d4 100644 --- a/src/bin/nydus-image/unpack/mod.rs +++ b/src/bin/nydus-image/unpack/mod.rs @@ -60,12 +60,7 @@ impl OCIUnpacker { } fn load_rafs(&self, config: Arc) -> Result { - let (rs, _) = RafsSuper::load_from_file( - self.bootstrap.as_path(), - config.clone(), - config.is_chunk_validation_enabled(), - false, - )?; + let (rs, _) = RafsSuper::load_from_file(self.bootstrap.as_path(), config, false)?; Ok(rs) } } diff --git a/src/bin/nydus-image/validator.rs b/src/bin/nydus-image/validator.rs index f97420e219b..b25ae8b43b1 100644 --- a/src/bin/nydus-image/validator.rs +++ b/src/bin/nydus-image/validator.rs @@ -19,7 +19,7 @@ pub struct Validator { impl Validator { pub fn new(bootstrap_path: &Path, config: Arc) -> Result { - let (sb, _) = RafsSuper::load_from_file(bootstrap_path, config, true, false)?; + let (sb, _) = RafsSuper::load_from_file(bootstrap_path, config, false)?; Ok(Self { sb }) }