From e7e2419ff86e171e9e443a0d1a1d07550a51f9ec Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Thu, 14 Dec 2023 20:50:07 -0600 Subject: [PATCH] Make `publisher` yank one crate at a time (#3324) ## Motivation and Context `P109001503` ## Description When `publisher` yanks crates, it blasts `crates.io` in parallel with all the yank requests, and then awaits their results. To avoid throttling errors, this PR instead yanks one crate at a time, with a configurable time delay in between via a `--delay-millis` command line argument. Further, a call to yank is now wrapped with `run_with_retry` to detect throttle errors, wait a long period of time after, and try again. Essentially, the structure of the `yank` module is now similar to that of the `publish` module such as use of `run_with_retry` and the presence of a CLI argument `delay-millis`. ## Testing On top of changes in the PR, manually modified the following for loop in `yank_release.rs` locally ``` for (crate_name, crate_version) in crates { ``` to ``` for (crate_name, crate_version) in [("aws-sigv4", "0.55.0"); 50] { // aws-sigv4@0.55.0 has already been yanked in the past so it's safe to yank repeatedly ``` and observed the behavior of `publisher yank-release --delay-millis 2000` that it - waits 2 seconds beteen yanks - yanks in a serial manner ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- tools/ci-build/publisher/src/lib.rs | 1 + .../publisher/src/subcommand/yank_release.rs | 39 +++++++------------ tools/ci-build/publisher/src/yank.rs | 27 +++++++++++++ 3 files changed, 43 insertions(+), 24 deletions(-) create mode 100644 tools/ci-build/publisher/src/yank.rs diff --git a/tools/ci-build/publisher/src/lib.rs b/tools/ci-build/publisher/src/lib.rs index 5e677b0a2..562880c5d 100644 --- a/tools/ci-build/publisher/src/lib.rs +++ b/tools/ci-build/publisher/src/lib.rs @@ -17,3 +17,4 @@ pub mod publish; pub mod retry; pub mod sort; pub mod subcommand; +pub mod yank; diff --git a/tools/ci-build/publisher/src/subcommand/yank_release.rs b/tools/ci-build/publisher/src/subcommand/yank_release.rs index 12b769a10..0b8378c22 100644 --- a/tools/ci-build/publisher/src/subcommand/yank_release.rs +++ b/tools/ci-build/publisher/src/subcommand/yank_release.rs @@ -4,21 +4,20 @@ */ use crate::cargo; +use crate::yank::yank; use anyhow::{anyhow, bail, Context, Result}; use clap::{ArgEnum, Parser}; use dialoguer::Confirm; use smithy_rs_tool_common::package::PackageCategory; use smithy_rs_tool_common::release_tag::ReleaseTag; -use smithy_rs_tool_common::shell::ShellOperation; use smithy_rs_tool_common::versions_manifest::{Release, VersionsManifest}; use std::collections::BTreeMap; use std::path::{Path, PathBuf}; use std::str::FromStr; -use std::sync::Arc; -use tokio::sync::Semaphore; +use std::time::Duration; use tracing::info; -const MAX_CONCURRENCY: usize = 5; +const DEFAULT_DELAY_MILLIS: usize = 1000; #[derive(Copy, Clone, Debug, ArgEnum, Eq, PartialEq, Ord, PartialOrd)] pub enum CrateSet { @@ -42,6 +41,9 @@ pub struct YankReleaseArgs { versions_toml: Option, #[clap(arg_enum)] crate_set: Option, + /// Time delay between crate yanking to avoid crates.io throttling errors. + #[clap(long)] + delay_millis: Option, } pub async fn subcommand_yank_release( @@ -49,6 +51,7 @@ pub async fn subcommand_yank_release( github_release_tag, versions_toml, crate_set, + delay_millis, }: &YankReleaseArgs, ) -> Result<()> { // Make sure cargo exists @@ -75,28 +78,16 @@ pub async fn subcommand_yank_release( // Don't proceed unless the user confirms the plan confirm_plan(&tag, &crates)?; - // Use a semaphore to only allow a few concurrent yanks - let semaphore = Arc::new(Semaphore::new(MAX_CONCURRENCY)); - info!( - "Will yank {} crates in parallel where possible.", - MAX_CONCURRENCY - ); + let delay_millis = Duration::from_millis(delay_millis.unwrap_or(DEFAULT_DELAY_MILLIS) as _); - let mut tasks = Vec::new(); + // Yank one crate at a time to try avoiding throttling errors for (crate_name, crate_version) in crates { - let permit = semaphore.clone().acquire_owned().await.unwrap(); - tasks.push(tokio::spawn(async move { - info!("Yanking `{}-{}`...", crate_name, crate_version); - let result = cargo::Yank::new(&crate_name, &crate_version).spawn().await; - drop(permit); - if result.is_ok() { - info!("Successfully yanked `{}-{}`", crate_name, crate_version); - } - result - })); - } - for task in tasks { - task.await??; + yank(&crate_name, &crate_version).await?; + + // Keep things slow to avoid getting throttled by crates.io + tokio::time::sleep(delay_millis).await; + + info!("Successfully yanked `{}-{}`", crate_name, crate_version); } Ok(()) diff --git a/tools/ci-build/publisher/src/yank.rs b/tools/ci-build/publisher/src/yank.rs new file mode 100644 index 000000000..fa46919a1 --- /dev/null +++ b/tools/ci-build/publisher/src/yank.rs @@ -0,0 +1,27 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +use crate::cargo; +use crate::retry::{run_with_retry, BoxError, ErrorClass}; +use smithy_rs_tool_common::shell::ShellOperation; +use std::time::Duration; +use tracing::info; + +#[tracing::instrument] +pub async fn yank(crate_name: &str, crate_version: &str) -> anyhow::Result<()> { + info!("Yanking `{}-{}`...", crate_name, crate_version); + run_with_retry( + &format!("Yanking `{}-{}`", crate_name, crate_version), + 5, + Duration::from_secs(60), + || async { + cargo::Yank::new(crate_name, crate_version).spawn().await?; + Result::<_, BoxError>::Ok(()) + }, + |_err| ErrorClass::Retry, + ) + .await?; + Ok(()) +} -- GitLab