Skip to content

Commit

Permalink
rafs: only decide whether to do digest validate based on configuration.
Browse files Browse the repository at this point in the history
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 <imeoer@linux.alibaba.com>
  • Loading branch information
imeoer committed Mar 17, 2023
1 parent c9d9b43 commit d27d3e7
Show file tree
Hide file tree
Showing 11 changed files with 18 additions and 21 deletions.
3 changes: 1 addition & 2 deletions rafs/src/builder/core/bootstrap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,8 +283,7 @@ impl Bootstrap {
blob_mgr: &mut BlobManager,
) -> Result<Tree> {
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"));
};
Expand Down
2 changes: 1 addition & 1 deletion rafs/src/builder/core/chunk_dict.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ impl HashChunkDict {
config: Arc<ConfigV2>,
rafs_config: &RafsSuperConfig,
) -> Result<Self> {
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(),
Expand Down
2 changes: 1 addition & 1 deletion rafs/src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand Down
6 changes: 5 additions & 1 deletion rafs/src/metadata/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -683,9 +683,13 @@ impl RafsSuper {
pub fn load_from_file<P: AsRef<Path>>(
path: P,
config: Arc<ConfigV2>,
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,
Expand Down
2 changes: 1 addition & 1 deletion service/src/blob_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ impl BlobCacheMgr {
path: PathBuf,
config: Arc<ConfigV2>,
) -> 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"));
}
Expand Down
2 changes: 1 addition & 1 deletion src/bin/nydus-image/inspect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ impl RafsInspector {
request_mode: bool,
config: Arc<ConfigV2>,
) -> Result<Self, anyhow::Error> {
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 {
Expand Down
2 changes: 1 addition & 1 deletion src/bin/nydus-image/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<String>("chunk-dict") {
None => None,
Expand Down
9 changes: 4 additions & 5 deletions src/bin/nydus-image/merge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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());
Expand All @@ -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())
Expand Down
2 changes: 1 addition & 1 deletion src/bin/nydus-image/stat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ impl ImageStat {
}

pub fn stat(&mut self, path: &Path, is_base: bool, config: Arc<ConfigV2>) -> 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 =
Expand Down
7 changes: 1 addition & 6 deletions src/bin/nydus-image/unpack/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,7 @@ impl OCIUnpacker {
}

fn load_rafs(&self, config: Arc<ConfigV2>) -> Result<RafsSuper> {
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)
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/bin/nydus-image/validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub struct Validator {

impl Validator {
pub fn new(bootstrap_path: &Path, config: Arc<ConfigV2>) -> Result<Self> {
let (sb, _) = RafsSuper::load_from_file(bootstrap_path, config, true, false)?;
let (sb, _) = RafsSuper::load_from_file(bootstrap_path, config, false)?;

Ok(Self { sb })
}
Expand Down

0 comments on commit d27d3e7

Please sign in to comment.