From 8b301281fff5c75d1040f13b55d1505607515169 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 28 Jun 2024 16:20:51 -0400 Subject: [PATCH 1/3] install: Use separate pull stage for progress output The goal here is to get interactive progress on pulling, as that can be slow and also I'd like to get more information there. Signed-off-by: Colin Walters --- lib/src/cli.rs | 9 +++++---- lib/src/deploy.rs | 3 +-- lib/src/install.rs | 7 +++++++ 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/lib/src/cli.rs b/lib/src/cli.rs index f4b4077f..958bb481 100644 --- a/lib/src/cli.rs +++ b/lib/src/cli.rs @@ -533,7 +533,7 @@ async fn upgrade(opts: UpgradeOpts) -> Result<()> { } } } else { - let fetched = crate::deploy::pull(sysroot, imgref, opts.quiet).await?; + let fetched = crate::deploy::pull(repo, imgref, opts.quiet).await?; let kargs = crate::kargs::get_kargs(repo, &booted_deployment, fetched.as_ref())?; let staged_digest = staged_image.as_ref().map(|s| s.image_digest.as_str()); let fetched_digest = fetched.manifest_digest.as_str(); @@ -630,7 +630,7 @@ async fn switch(opts: SwitchOpts) -> Result<()> { } let new_spec = RequiredHostSpec::from_spec(&new_spec)?; - let fetched = crate::deploy::pull(sysroot, &target, opts.quiet).await?; + let fetched = crate::deploy::pull(repo, &target, opts.quiet).await?; let kargs = crate::kargs::get_kargs(repo, &booted_deployment, fetched.as_ref())?; if !opts.retain { @@ -664,6 +664,8 @@ async fn rollback(_opts: RollbackOpts) -> Result<()> { #[context("Editing spec")] async fn edit(opts: EditOpts) -> Result<()> { let sysroot = &get_locked_sysroot().await?; + let repo = &sysroot.repo(); + let (booted_deployment, _deployments, host) = crate::status::get_status_require_booted(sysroot)?; let new_host: Host = if let Some(filename) = opts.filename { @@ -690,8 +692,7 @@ async fn edit(opts: EditOpts) -> Result<()> { return crate::deploy::rollback(sysroot).await; } - let fetched = crate::deploy::pull(sysroot, new_spec.image, opts.quiet).await?; - let repo = &sysroot.repo(); + let fetched = crate::deploy::pull(repo, new_spec.image, opts.quiet).await?; let kargs = crate::kargs::get_kargs(repo, &booted_deployment, fetched.as_ref())?; // TODO gc old layers here diff --git a/lib/src/deploy.rs b/lib/src/deploy.rs index 690550e2..7a3f4a6f 100644 --- a/lib/src/deploy.rs +++ b/lib/src/deploy.rs @@ -164,11 +164,10 @@ async fn handle_layer_progress_print( /// Wrapper for pulling a container image, wiring up status output. #[context("Pulling")] pub(crate) async fn pull( - sysroot: &SysrootLock, + repo: &ostree::Repo, imgref: &ImageReference, quiet: bool, ) -> Result> { - let repo = &sysroot.repo(); let ostree_imgref = &OstreeImageReference::from(imgref.clone()); let mut imp = new_importer(repo, ostree_imgref).await?; let prep = match imp.prepare().await? { diff --git a/lib/src/install.rs b/lib/src/install.rs index b9d6a653..07f179cf 100644 --- a/lib/src/install.rs +++ b/lib/src/install.rs @@ -40,6 +40,7 @@ use serde::{Deserialize, Serialize}; use self::baseline::InstallBlockDeviceOpts; use crate::containerenv::ContainerExecutionInfo; use crate::mount::Filesystem; +use crate::spec::ImageReference; use crate::task::Task; use crate::utils::sigpolicy_from_opts; @@ -651,6 +652,12 @@ async fn initialize_ostree_root_from_self( imgref: src_imageref, }; + // Pull the container image into the target root filesystem. + { + let spec_imgref = ImageReference::from(src_imageref.clone()); + crate::deploy::pull(&sysroot.repo(), &spec_imgref, false).await?; + } + // Load the kargs from the /usr/lib/bootc/kargs.d from the running root, // which should be the same as the filesystem we'll deploy. let kargsd = crate::kargs::get_kargs_in_root(container_rootfs, std::env::consts::ARCH)?; From 6c81a40def256b5eb793c2aafc6d77699861740b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sat, 29 Jun 2024 18:31:04 +0000 Subject: [PATCH 2/3] install: Disable fsync() in repo when pulling This more than doubles the copy phase speed for me. We rely on a final fsync/flush of the disks when unmounting. Signed-off-by: Colin Walters --- lib/src/install.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/src/install.rs b/lib/src/install.rs index 07f179cf..4f3fa66d 100644 --- a/lib/src/install.rs +++ b/lib/src/install.rs @@ -652,10 +652,14 @@ async fn initialize_ostree_root_from_self( imgref: src_imageref, }; - // Pull the container image into the target root filesystem. + // Pull the container image into the target root filesystem. Since this is + // an install path, we don't need to fsync() individual layers. { let spec_imgref = ImageReference::from(src_imageref.clone()); - crate::deploy::pull(&sysroot.repo(), &spec_imgref, false).await?; + let repo = &sysroot.repo(); + repo.set_disable_fsync(true); + crate::deploy::pull(repo, &spec_imgref, false).await?; + repo.set_disable_fsync(false); } // Load the kargs from the /usr/lib/bootc/kargs.d from the running root, From d8b5df2f378b54c3a60ccaa673a60a7bbca5d653 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sat, 29 Jun 2024 08:41:32 -0400 Subject: [PATCH 3/3] pull: Improved progress output indicatif has nice support for multiple bars. Instead of hand-rolling a `[nn/NN]` for the overall layer count, change things so that we have: ``` Fetching layers [bar...] 8/65 ostree chunk sha256:29fc11ff03e4b3 [bar] (0 B/s) ``` Signed-off-by: Colin Walters --- lib/src/deploy.rs | 101 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 76 insertions(+), 25 deletions(-) diff --git a/lib/src/deploy.rs b/lib/src/deploy.rs index 7a3f4a6f..f5fbbfb1 100644 --- a/lib/src/deploy.rs +++ b/lib/src/deploy.rs @@ -13,7 +13,8 @@ use fn_error_context::context; use ostree::{gio, glib}; use ostree_container::OstreeImageReference; use ostree_ext::container as ostree_container; -use ostree_ext::container::store::PrepareResult; +use ostree_ext::container::store::{ImportProgress, PrepareResult}; +use ostree_ext::oci_spec::image::Descriptor; use ostree_ext::ostree; use ostree_ext::ostree::Deployment; use ostree_ext::sysroot::SysrootLock; @@ -112,34 +113,76 @@ pub(crate) fn check_bootc_label(config: &ostree_ext::oci_spec::image::ImageConfi } } +fn descriptor_of_progress(p: &ImportProgress) -> &Descriptor { + match p { + ImportProgress::OstreeChunkStarted(l) => l, + ImportProgress::OstreeChunkCompleted(l) => l, + ImportProgress::DerivedLayerStarted(l) => l, + ImportProgress::DerivedLayerCompleted(l) => l, + } +} + +fn prefix_of_progress(p: &ImportProgress) -> &'static str { + match p { + ImportProgress::OstreeChunkStarted(_) | ImportProgress::OstreeChunkCompleted(_) => { + "ostree chunk" + } + ImportProgress::DerivedLayerStarted(_) | ImportProgress::DerivedLayerCompleted(_) => { + "layer" + } + } +} + /// Write container fetch progress to standard output. async fn handle_layer_progress_print( mut layers: tokio::sync::mpsc::Receiver, mut layer_bytes: tokio::sync::watch::Receiver>, - total_layers: usize, - n_layers_fetched: &mut usize, + n_layers_to_fetch: usize, ) { - let style = indicatif::ProgressStyle::default_bar(); - let pb = indicatif::ProgressBar::new(100); - pb.set_style( - style - .template("{prefix} {bytes} [{bar:20}] ({eta}) {msg}") + let start = std::time::Instant::now(); + let mut total_read = 0u64; + let bar = indicatif::MultiProgress::new(); + let layers_bar = bar.add(indicatif::ProgressBar::new( + n_layers_to_fetch.try_into().unwrap(), + )); + let byte_bar = bar.add(indicatif::ProgressBar::new(0)); + // let byte_bar = indicatif::ProgressBar::new(0); + // byte_bar.set_draw_target(indicatif::ProgressDrawTarget::hidden()); + layers_bar.set_style( + indicatif::ProgressStyle::default_bar() + .template("{prefix} {bar} {pos}/{len} {wide_msg}") .unwrap(), ); + layers_bar.set_prefix("Fetching layers"); + layers_bar.set_message(""); + byte_bar.set_prefix("Fetching"); + byte_bar.set_style( + indicatif::ProgressStyle::default_bar() + .template( + " └ {prefix} {bar} {binary_bytes}/{binary_total_bytes} ({binary_bytes_per_sec}) {wide_msg}", + ) + .unwrap() + ); loop { tokio::select! { // Always handle layer changes first. biased; layer = layers.recv() => { if let Some(l) = layer { + let layer = descriptor_of_progress(&l); + let layer_size = u64::try_from(layer.size()).unwrap(); if l.is_starting() { - pb.set_position(0); + byte_bar.reset_elapsed(); + byte_bar.reset_eta(); + byte_bar.set_length(layer_size); + let layer_type = prefix_of_progress(&l); + let short_digest = &layer.digest()[0..21]; + byte_bar.set_message(format!("{layer_type} {short_digest}")); } else { - pb.finish(); - *n_layers_fetched += 1; + byte_bar.set_position(layer_size); + layers_bar.inc(1); + total_read = total_read.saturating_add(layer_size); } - pb.set_prefix(format!("[{}/{}]", *n_layers_fetched, total_layers)); - pb.set_message(ostree_ext::cli::layer_progress_format(&l)); } else { // If the receiver is disconnected, then we're done break @@ -152,13 +195,26 @@ async fn handle_layer_progress_print( } let bytes = layer_bytes.borrow(); if let Some(bytes) = &*bytes { - pb.set_length(bytes.total); - pb.set_position(bytes.fetched); + byte_bar.set_position(bytes.fetched); } } - } } + byte_bar.finish_and_clear(); + layers_bar.finish_and_clear(); + if let Err(e) = bar.clear() { + tracing::warn!("clearing bar: {e}"); + } + let end = std::time::Instant::now(); + let elapsed = end.duration_since(start); + let persec = total_read as f64 / elapsed.as_secs_f64(); + let persec = indicatif::HumanBytes(persec as u64); + println!( + "Fetched layers: {} in {} ({}/s)", + indicatif::HumanBytes(total_read), + indicatif::HumanDuration(elapsed), + persec, + ); } /// Wrapper for pulling a container image, wiring up status output. @@ -182,19 +238,14 @@ pub(crate) async fn pull( ostree_ext::cli::print_deprecated_warning(warning).await; } ostree_ext::cli::print_layer_status(&prep); + let layers_to_fetch = prep.layers_to_fetch().collect::>>()?; + let n_layers_to_fetch = layers_to_fetch.len(); let printer = (!quiet).then(|| { let layer_progress = imp.request_progress(); let layer_byte_progress = imp.request_layer_progress(); - let total_layers = prep.layers_to_fetch().count(); - let mut n_fetched = 0usize; tokio::task::spawn(async move { - handle_layer_progress_print( - layer_progress, - layer_byte_progress, - total_layers, - &mut n_fetched, - ) - .await + handle_layer_progress_print(layer_progress, layer_byte_progress, n_layers_to_fetch) + .await }) }); let import = imp.import(prep).await;