From 6742c6a85e893543b078d36e2ee2e0191aecb4fd Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Wed, 12 Jun 2024 15:36:07 -0400 Subject: [PATCH 01/14] Fixup dev shell on macos --- flake.nix | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/flake.nix b/flake.nix index 560d87c..0638acb 100644 --- a/flake.nix +++ b/flake.nix @@ -59,7 +59,11 @@ bacon age - ]; + ] ++ lib.optionals pkgs.stdenv.isDarwin (with pkgs.darwin.apple_sdk.frameworks; [ + SystemConfiguration + ]); + + NIX_CFLAGS_LINK = lib.optionalString pkgs.stdenv.isDarwin "-lc++abi"; }; /* From 9c7b8e3fc99011bf191be3fe71daab1f3c969d8f Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Wed, 12 Jun 2024 15:44:26 -0400 Subject: [PATCH 02/14] Trip a circuit breaker when we get a 429 so we don't keep doing useless work --- flake.nix | 1 + gha-cache/src/api.rs | 41 +++++++++++++++++++++++++++++++++++- magic-nix-cache/src/error.rs | 4 ++-- magic-nix-cache/src/gha.rs | 11 ++++++++-- 4 files changed, 52 insertions(+), 5 deletions(-) diff --git a/flake.nix b/flake.nix index 0638acb..704d321 100644 --- a/flake.nix +++ b/flake.nix @@ -56,6 +56,7 @@ cargo-bloat cargo-edit cargo-udeps + cargo-watch bacon age diff --git a/gha-cache/src/api.rs b/gha-cache/src/api.rs index a04a10b..31e1508 100644 --- a/gha-cache/src/api.rs +++ b/gha-cache/src/api.rs @@ -4,7 +4,7 @@ use std::fmt; #[cfg(debug_assertions)] -use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use std::sync::Arc; use async_trait::async_trait; @@ -53,6 +53,9 @@ pub enum Error { #[error("Failed to initialize the client: {0}")] InitError(Box), + #[error("Circuit breaker tripped.")] + CircuitBreakerTripped, + #[error("Request error: {0}")] RequestError(#[from] reqwest::Error), // TODO: Better errors @@ -96,6 +99,8 @@ pub struct Api { /// The concurrent upload limit. concurrency_limit: Arc, + circuit_breaker_429_tripped: Arc, + /// Backend request statistics. #[cfg(debug_assertions)] stats: RequestStats, @@ -264,11 +269,16 @@ impl Api { version_hasher, client, concurrency_limit: Arc::new(Semaphore::new(MAX_CONCURRENCY)), + circuit_breaker_429_tripped: Arc::new(AtomicBool::from(false)), #[cfg(debug_assertions)] stats: Default::default(), }) } + pub fn circuit_breaker_tripped(&self) -> bool { + self.circuit_breaker_429_tripped.load(Ordering::Relaxed) + } + /// Mutates the cache version/namespace. pub fn mutate_version(&mut self, data: &[u8]) { self.version_hasher.update(data); @@ -324,6 +334,10 @@ impl Api { where S: AsyncRead + Unpin + Send, { + if self.circuit_breaker_429_tripped.load(Ordering::Relaxed) { + return Err(Error::CircuitBreakerTripped); + } + let mut offset = 0; let mut futures = Vec::new(); loop { @@ -347,6 +361,7 @@ impl Api { futures.push({ let client = self.client.clone(); let concurrency_limit = self.concurrency_limit.clone(); + let circuit_breaker_429_tripped = self.circuit_breaker_429_tripped.clone(); let url = self.construct_url(&format!("caches/{}", allocation.0 .0)); tokio::task::spawn(async move { @@ -380,6 +395,11 @@ impl Api { drop(permit); + + if let Err(Error::ApiError{ status: reqwest::StatusCode::TOO_MANY_REQUESTS, info: ref _info }) = r { + circuit_breaker_429_tripped.store(true, Ordering::Relaxed); + } + r }) }); @@ -401,6 +421,11 @@ impl Api { /// Downloads a file based on a list of key prefixes. pub async fn get_file_url(&self, keys: &[&str]) -> Result> { + + if self.circuit_breaker_429_tripped.load(Ordering::Relaxed) { + return Err(Error::CircuitBreakerTripped); + } + Ok(self .get_cache_entry(keys) .await? @@ -419,6 +444,10 @@ impl Api { /// Retrieves a cache based on a list of key prefixes. async fn get_cache_entry(&self, keys: &[&str]) -> Result> { + if self.circuit_breaker_429_tripped.load(Ordering::Relaxed) { + return Err(Error::CircuitBreakerTripped); + } + #[cfg(debug_assertions)] self.stats.get.fetch_add(1, Ordering::SeqCst); @@ -448,6 +477,11 @@ impl Api { key: &str, cache_size: Option, ) -> Result { + + if self.circuit_breaker_429_tripped.load(Ordering::Relaxed) { + return Err(Error::CircuitBreakerTripped); + } + tracing::debug!("Reserving cache for {}", key); let req = ReserveCacheRequest { @@ -473,6 +507,11 @@ impl Api { /// Finalizes uploading to a cache. async fn commit_cache(&self, cache_id: CacheId, size: usize) -> Result<()> { + + if self.circuit_breaker_429_tripped.load(Ordering::Relaxed) { + return Err(Error::CircuitBreakerTripped); + } + tracing::debug!("Commiting cache {:?}", cache_id); let req = CommitCacheRequest { size }; diff --git a/magic-nix-cache/src/error.rs b/magic-nix-cache/src/error.rs index ec1b8d3..e80ea70 100644 --- a/magic-nix-cache/src/error.rs +++ b/magic-nix-cache/src/error.rs @@ -10,8 +10,8 @@ pub type Result = std::result::Result; #[derive(Error, Debug)] pub enum Error { - #[error("GitHub API error: {0}")] - Api(#[from] gha_cache::api::Error), + #[error("GitHub API error: {0}")] + Api(#[from] gha_cache::api::Error), #[error("Not Found")] NotFound, diff --git a/magic-nix-cache/src/gha.rs b/magic-nix-cache/src/gha.rs index 756b96b..e1f0aa6 100644 --- a/magic-nix-cache/src/gha.rs +++ b/magic-nix-cache/src/gha.rs @@ -119,6 +119,13 @@ async fn worker( break; } Request::Upload(path) => { + if (api.circuit_breaker_tripped()) { + tracing::trace!( + "GitHub Actions gave us a 429, so we're done.", + ); + continue; + } + if !done.insert(path.clone()) { continue; } @@ -188,8 +195,8 @@ async fn upload_path( tracing::debug!("Uploading '{}'", narinfo_path); - api.upload_file(narinfo_allocation, narinfo.as_bytes()) - .await?; + api.upload_file(narinfo_allocation, narinfo.as_bytes()).await?; + metrics.narinfos_uploaded.incr(); narinfo_negative_cache From 4dd3242a1413a9c4c22dbb93fab225e7ba512218 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Wed, 12 Jun 2024 15:46:52 -0400 Subject: [PATCH 03/14] cleanup --- gha-cache/src/api.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/gha-cache/src/api.rs b/gha-cache/src/api.rs index 31e1508..a3b3a6f 100644 --- a/gha-cache/src/api.rs +++ b/gha-cache/src/api.rs @@ -334,7 +334,7 @@ impl Api { where S: AsyncRead + Unpin + Send, { - if self.circuit_breaker_429_tripped.load(Ordering::Relaxed) { + if self.circuit_breaker_tripped() { return Err(Error::CircuitBreakerTripped); } @@ -422,7 +422,7 @@ impl Api { /// Downloads a file based on a list of key prefixes. pub async fn get_file_url(&self, keys: &[&str]) -> Result> { - if self.circuit_breaker_429_tripped.load(Ordering::Relaxed) { + if self.circuit_breaker_tripped() { return Err(Error::CircuitBreakerTripped); } @@ -444,7 +444,7 @@ impl Api { /// Retrieves a cache based on a list of key prefixes. async fn get_cache_entry(&self, keys: &[&str]) -> Result> { - if self.circuit_breaker_429_tripped.load(Ordering::Relaxed) { + if self.circuit_breaker_tripped() { return Err(Error::CircuitBreakerTripped); } @@ -478,7 +478,7 @@ impl Api { cache_size: Option, ) -> Result { - if self.circuit_breaker_429_tripped.load(Ordering::Relaxed) { + if self.circuit_breaker_tripped() { return Err(Error::CircuitBreakerTripped); } @@ -508,7 +508,7 @@ impl Api { /// Finalizes uploading to a cache. async fn commit_cache(&self, cache_id: CacheId, size: usize) -> Result<()> { - if self.circuit_breaker_429_tripped.load(Ordering::Relaxed) { + if self.circuit_breaker_tripped() { return Err(Error::CircuitBreakerTripped); } From 805d2cfc156489cf0bf5643466528dc64c67ae42 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Wed, 12 Jun 2024 15:50:22 -0400 Subject: [PATCH 04/14] fixup --- gha-cache/src/api.rs | 44 ++++++++++++++++----------------- magic-nix-cache/src/error.rs | 4 +-- magic-nix-cache/src/flakehub.rs | 2 +- magic-nix-cache/src/gha.rs | 11 ++++----- 4 files changed, 30 insertions(+), 31 deletions(-) diff --git a/gha-cache/src/api.rs b/gha-cache/src/api.rs index a3b3a6f..838d889 100644 --- a/gha-cache/src/api.rs +++ b/gha-cache/src/api.rs @@ -276,7 +276,7 @@ impl Api { } pub fn circuit_breaker_tripped(&self) -> bool { - self.circuit_breaker_429_tripped.load(Ordering::Relaxed) + self.circuit_breaker_429_tripped.load(Ordering::Relaxed) } /// Mutates the cache version/namespace. @@ -334,9 +334,9 @@ impl Api { where S: AsyncRead + Unpin + Send, { - if self.circuit_breaker_tripped() { - return Err(Error::CircuitBreakerTripped); - } + if self.circuit_breaker_tripped() { + return Err(Error::CircuitBreakerTripped); + } let mut offset = 0; let mut futures = Vec::new(); @@ -395,9 +395,12 @@ impl Api { drop(permit); - - if let Err(Error::ApiError{ status: reqwest::StatusCode::TOO_MANY_REQUESTS, info: ref _info }) = r { - circuit_breaker_429_tripped.store(true, Ordering::Relaxed); + if let Err(Error::ApiError { + status: reqwest::StatusCode::TOO_MANY_REQUESTS, + info: ref _info, + }) = r + { + circuit_breaker_429_tripped.store(true, Ordering::Relaxed); } r @@ -421,10 +424,9 @@ impl Api { /// Downloads a file based on a list of key prefixes. pub async fn get_file_url(&self, keys: &[&str]) -> Result> { - - if self.circuit_breaker_tripped() { - return Err(Error::CircuitBreakerTripped); - } + if self.circuit_breaker_tripped() { + return Err(Error::CircuitBreakerTripped); + } Ok(self .get_cache_entry(keys) @@ -444,9 +446,9 @@ impl Api { /// Retrieves a cache based on a list of key prefixes. async fn get_cache_entry(&self, keys: &[&str]) -> Result> { - if self.circuit_breaker_tripped() { - return Err(Error::CircuitBreakerTripped); - } + if self.circuit_breaker_tripped() { + return Err(Error::CircuitBreakerTripped); + } #[cfg(debug_assertions)] self.stats.get.fetch_add(1, Ordering::SeqCst); @@ -477,10 +479,9 @@ impl Api { key: &str, cache_size: Option, ) -> Result { - - if self.circuit_breaker_tripped() { - return Err(Error::CircuitBreakerTripped); - } + if self.circuit_breaker_tripped() { + return Err(Error::CircuitBreakerTripped); + } tracing::debug!("Reserving cache for {}", key); @@ -507,10 +508,9 @@ impl Api { /// Finalizes uploading to a cache. async fn commit_cache(&self, cache_id: CacheId, size: usize) -> Result<()> { - - if self.circuit_breaker_tripped() { - return Err(Error::CircuitBreakerTripped); - } + if self.circuit_breaker_tripped() { + return Err(Error::CircuitBreakerTripped); + } tracing::debug!("Commiting cache {:?}", cache_id); diff --git a/magic-nix-cache/src/error.rs b/magic-nix-cache/src/error.rs index e80ea70..ec1b8d3 100644 --- a/magic-nix-cache/src/error.rs +++ b/magic-nix-cache/src/error.rs @@ -10,8 +10,8 @@ pub type Result = std::result::Result; #[derive(Error, Debug)] pub enum Error { - #[error("GitHub API error: {0}")] - Api(#[from] gha_cache::api::Error), + #[error("GitHub API error: {0}")] + Api(#[from] gha_cache::api::Error), #[error("Not Found")] NotFound, diff --git a/magic-nix-cache/src/flakehub.rs b/magic-nix-cache/src/flakehub.rs index 958f8e4..1336d51 100644 --- a/magic-nix-cache/src/flakehub.rs +++ b/magic-nix-cache/src/flakehub.rs @@ -345,7 +345,7 @@ async fn rewrite_github_actions_token( let token_response: TokenResponse = token_response .json() .await - .with_context(|| format!("converting response into json"))?; + .with_context(|| "converting response into json")?; let new_github_jwt_string = token_response.value; let netrc_contents = tokio::fs::read_to_string(netrc_path) diff --git a/magic-nix-cache/src/gha.rs b/magic-nix-cache/src/gha.rs index e1f0aa6..00e1ba2 100644 --- a/magic-nix-cache/src/gha.rs +++ b/magic-nix-cache/src/gha.rs @@ -119,11 +119,9 @@ async fn worker( break; } Request::Upload(path) => { - if (api.circuit_breaker_tripped()) { - tracing::trace!( - "GitHub Actions gave us a 429, so we're done.", - ); - continue; + if api.circuit_breaker_tripped() { + tracing::trace!("GitHub Actions gave us a 429, so we're done.",); + continue; } if !done.insert(path.clone()) { @@ -195,7 +193,8 @@ async fn upload_path( tracing::debug!("Uploading '{}'", narinfo_path); - api.upload_file(narinfo_allocation, narinfo.as_bytes()).await?; + api.upload_file(narinfo_allocation, narinfo.as_bytes()) + .await?; metrics.narinfos_uploaded.incr(); From 0ad1f17858d6baea4a20147ae432b7f30001b13a Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Wed, 12 Jun 2024 16:01:21 -0400 Subject: [PATCH 05/14] ? --- gha-cache/src/api.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gha-cache/src/api.rs b/gha-cache/src/api.rs index 838d889..dd99b7b 100644 --- a/gha-cache/src/api.rs +++ b/gha-cache/src/api.rs @@ -4,7 +4,8 @@ use std::fmt; #[cfg(debug_assertions)] -use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; +use std::sync::atomic::AtomicUsize; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use async_trait::async_trait; From 22f76db21567ef94304bc7dcda99b130e01182c7 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Wed, 12 Jun 2024 16:30:27 -0400 Subject: [PATCH 06/14] Catch 429s in more places --- gha-cache/src/api.rs | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/gha-cache/src/api.rs b/gha-cache/src/api.rs index dd99b7b..3b26341 100644 --- a/gha-cache/src/api.rs +++ b/gha-cache/src/api.rs @@ -280,6 +280,18 @@ impl Api { self.circuit_breaker_429_tripped.load(Ordering::Relaxed) } + fn circuit_breaker_trip_on_429(&self, e: &Error) { + if let Error::ApiError { + status: reqwest::StatusCode::TOO_MANY_REQUESTS, + info: ref _info, + } = e + { + tracing::info!("Disabling GitHub Actions Cache due to 429: Too Many Requests"); + self.circuit_breaker_429_tripped + .store(true, Ordering::Relaxed); + } + } + /// Mutates the cache version/namespace. pub fn mutate_version(&mut self, data: &[u8]) { self.version_hasher.update(data); @@ -461,7 +473,8 @@ impl Api { .send() .await? .check_json() - .await; + .await + .inspect_err(|e| self.circuit_breaker_trip_on_429(e)); match res { Ok(entry) => Ok(Some(entry)), @@ -502,7 +515,8 @@ impl Api { .send() .await? .check_json() - .await?; + .await + .inspect_err(|e| self.circuit_breaker_trip_on_429(e))?; Ok(res) } @@ -526,7 +540,8 @@ impl Api { .send() .await? .check() - .await?; + .await + .inspect_err(|e| self.circuit_breaker_trip_on_429(e))?; Ok(()) } From 56d600d74b87130405f7901175d739242cf690b3 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Wed, 12 Jun 2024 16:33:58 -0400 Subject: [PATCH 07/14] I hatethis --- gha-cache/src/api.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/gha-cache/src/api.rs b/gha-cache/src/api.rs index 3b26341..31a6f2d 100644 --- a/gha-cache/src/api.rs +++ b/gha-cache/src/api.rs @@ -281,15 +281,15 @@ impl Api { } fn circuit_breaker_trip_on_429(&self, e: &Error) { - if let Error::ApiError { - status: reqwest::StatusCode::TOO_MANY_REQUESTS, - info: ref _info, - } = e - { - tracing::info!("Disabling GitHub Actions Cache due to 429: Too Many Requests"); - self.circuit_breaker_429_tripped - .store(true, Ordering::Relaxed); - } + if let Error::ApiError { + status: reqwest::StatusCode::TOO_MANY_REQUESTS, + info: ref _info, + } = e + { + tracing::info!("Disabling GitHub Actions Cache due to 429: Too Many Requests"); + self.circuit_breaker_429_tripped + .store(true, Ordering::Relaxed); + } } /// Mutates the cache version/namespace. From 76db34a53faee283a5afae0986443b50ca89bc9d Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Wed, 12 Jun 2024 16:59:38 -0400 Subject: [PATCH 08/14] fixup for older rust --- gha-cache/src/api.rs | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/gha-cache/src/api.rs b/gha-cache/src/api.rs index 31a6f2d..52208c9 100644 --- a/gha-cache/src/api.rs +++ b/gha-cache/src/api.rs @@ -473,8 +473,11 @@ impl Api { .send() .await? .check_json() - .await - .inspect_err(|e| self.circuit_breaker_trip_on_429(e)); + .await; + + if let Err(ref e) = res { + self.circuit_breaker_trip_on_429(e); + } match res { Ok(entry) => Ok(Some(entry)), @@ -515,10 +518,13 @@ impl Api { .send() .await? .check_json() - .await - .inspect_err(|e| self.circuit_breaker_trip_on_429(e))?; + .await; - Ok(res) + if let Err(ref e) = res { + self.circuit_breaker_trip_on_429(e); + } + + res } /// Finalizes uploading to a cache. @@ -534,14 +540,18 @@ impl Api { #[cfg(debug_assertions)] self.stats.post.fetch_add(1, Ordering::SeqCst); - self.client + if let Err(e) = self + .client .post(self.construct_url(&format!("caches/{}", cache_id.0))) .json(&req) .send() .await? .check() .await - .inspect_err(|e| self.circuit_breaker_trip_on_429(e))?; + { + self.circuit_breaker_trip_on_429(&e); + return Err(e); + } Ok(()) } From 1e5e79a3c88fdb77083ccda20ac3ab0f065b3f40 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Wed, 12 Jun 2024 17:55:39 -0400 Subject: [PATCH 09/14] lol --- flake.nix | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/flake.nix b/flake.nix index 704d321..febbd5c 100644 --- a/flake.nix +++ b/flake.nix @@ -43,6 +43,34 @@ magic-nix-cache = pkgs.callPackage ./package.nix { }; #inherit (cranePkgs) magic-nix-cache; default = magic-nix-cache; + + veryLongChain = + let + # Function to write the current date to a file + startFile = + pkgs.stdenv.mkDerivation { + name = "start-file"; + buildCommand = '' + echo ${magic-nix-cache} > $out + ''; + }; + + # Recursive function to create a chain of derivations + createChain = n: startFile: + pkgs.stdenv.mkDerivation { + name = "chain-${toString n}"; + src = + if n == 0 then + startFile + else createChain (n - 1) startFile; + buildCommand = '' + echo $src > $out + ''; + }; + + in + # Starting point of the chain + createChain 1000 startFile; }); devShells = forEachSupportedSystem ({ pkgs, cranePkgs, lib }: { From 3d21d10cae188c83e075d74e31cc231aa11e3fee Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Wed, 12 Jun 2024 18:03:34 -0400 Subject: [PATCH 10/14] lol --- .github/workflows/check-and-test.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/check-and-test.yaml b/.github/workflows/check-and-test.yaml index 93d17f4..861338b 100644 --- a/.github/workflows/check-and-test.yaml +++ b/.github/workflows/check-and-test.yaml @@ -78,3 +78,6 @@ jobs: - name: Run nix to test magic-nix-cache-action run: | nix develop --command echo "just testing" + - name: Exhaust our GitHub Actions Cache tokens + run: | + nix build .#veryLongChain -v From 3c54557810a29e29801d7ceac585d79d744d82b4 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Wed, 12 Jun 2024 21:20:43 -0400 Subject: [PATCH 11/14] Shrink the chain, rebuild every time --- .github/workflows/check-and-test.yaml | 1 + flake.nix | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/check-and-test.yaml b/.github/workflows/check-and-test.yaml index 861338b..3674523 100644 --- a/.github/workflows/check-and-test.yaml +++ b/.github/workflows/check-and-test.yaml @@ -80,4 +80,5 @@ jobs: nix develop --command echo "just testing" - name: Exhaust our GitHub Actions Cache tokens run: | + date >> README.md nix build .#veryLongChain -v diff --git a/flake.nix b/flake.nix index febbd5c..0eb1d20 100644 --- a/flake.nix +++ b/flake.nix @@ -46,12 +46,14 @@ veryLongChain = let + ctx = ./README.md; + # Function to write the current date to a file startFile = pkgs.stdenv.mkDerivation { name = "start-file"; buildCommand = '' - echo ${magic-nix-cache} > $out + cat ${ctx} > $out ''; }; @@ -70,7 +72,7 @@ in # Starting point of the chain - createChain 1000 startFile; + createChain 200 startFile; }); devShells = forEachSupportedSystem ({ pkgs, cranePkgs, lib }: { From 6996f0029f9c57a6fc9434aa7ef913415eb1dba0 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Wed, 12 Jun 2024 21:53:11 -0400 Subject: [PATCH 12/14] Don't run the GHA exhaustion test generally --- .github/workflows/check-and-test.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/check-and-test.yaml b/.github/workflows/check-and-test.yaml index 3674523..1e68cbc 100644 --- a/.github/workflows/check-and-test.yaml +++ b/.github/workflows/check-and-test.yaml @@ -79,6 +79,8 @@ jobs: run: | nix develop --command echo "just testing" - name: Exhaust our GitHub Actions Cache tokens + # Generally skip this step since it is so intensive + if: ${{ false }} run: | date >> README.md nix build .#veryLongChain -v From 25359d9b17ad5ecf7c6248875cdae8ecf3dfe764 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Thu, 13 Jun 2024 12:57:41 -0400 Subject: [PATCH 13/14] nit on error text --- gha-cache/src/api.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gha-cache/src/api.rs b/gha-cache/src/api.rs index 52208c9..c8e05c6 100644 --- a/gha-cache/src/api.rs +++ b/gha-cache/src/api.rs @@ -54,7 +54,7 @@ pub enum Error { #[error("Failed to initialize the client: {0}")] InitError(Box), - #[error("Circuit breaker tripped.")] + #[error("GitHub Actions Cache throttled Magic Nix Cache. Not trying to use it again on this run.")] CircuitBreakerTripped, #[error("Request error: {0}")] From 51bb9f972d9de7094f8d6ee96047b46f73e2817e Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Thu, 13 Jun 2024 13:07:00 -0400 Subject: [PATCH 14/14] Move the tripping to a separate impl on the atomicbool --- gha-cache/src/api.rs | 58 +++++++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 27 deletions(-) diff --git a/gha-cache/src/api.rs b/gha-cache/src/api.rs index c8e05c6..c1852df 100644 --- a/gha-cache/src/api.rs +++ b/gha-cache/src/api.rs @@ -54,7 +54,9 @@ pub enum Error { #[error("Failed to initialize the client: {0}")] InitError(Box), - #[error("GitHub Actions Cache throttled Magic Nix Cache. Not trying to use it again on this run.")] + #[error( + "GitHub Actions Cache throttled Magic Nix Cache. Not trying to use it again on this run." + )] CircuitBreakerTripped, #[error("Request error: {0}")] @@ -280,18 +282,6 @@ impl Api { self.circuit_breaker_429_tripped.load(Ordering::Relaxed) } - fn circuit_breaker_trip_on_429(&self, e: &Error) { - if let Error::ApiError { - status: reqwest::StatusCode::TOO_MANY_REQUESTS, - info: ref _info, - } = e - { - tracing::info!("Disabling GitHub Actions Cache due to 429: Too Many Requests"); - self.circuit_breaker_429_tripped - .store(true, Ordering::Relaxed); - } - } - /// Mutates the cache version/namespace. pub fn mutate_version(&mut self, data: &[u8]) { self.version_hasher.update(data); @@ -408,13 +398,7 @@ impl Api { drop(permit); - if let Err(Error::ApiError { - status: reqwest::StatusCode::TOO_MANY_REQUESTS, - info: ref _info, - }) = r - { - circuit_breaker_429_tripped.store(true, Ordering::Relaxed); - } + circuit_breaker_429_tripped.check_result(&r); r }) @@ -475,9 +459,7 @@ impl Api { .check_json() .await; - if let Err(ref e) = res { - self.circuit_breaker_trip_on_429(e); - } + self.circuit_breaker_429_tripped.check_result(&res); match res { Ok(entry) => Ok(Some(entry)), @@ -520,9 +502,7 @@ impl Api { .check_json() .await; - if let Err(ref e) = res { - self.circuit_breaker_trip_on_429(e); - } + self.circuit_breaker_429_tripped.check_result(&res); res } @@ -549,7 +529,7 @@ impl Api { .check() .await { - self.circuit_breaker_trip_on_429(&e); + self.circuit_breaker_429_tripped.check_err(&e); return Err(e); } @@ -619,3 +599,27 @@ async fn handle_error(res: reqwest::Response) -> Error { Error::ApiError { status, info } } + +trait AtomicCircuitBreaker { + fn check_err(&self, e: &Error); + fn check_result(&self, r: &std::result::Result); +} + +impl AtomicCircuitBreaker for AtomicBool { + fn check_result(&self, r: &std::result::Result) { + if let Err(ref e) = r { + self.check_err(e) + } + } + + fn check_err(&self, e: &Error) { + if let Error::ApiError { + status: reqwest::StatusCode::TOO_MANY_REQUESTS, + info: ref _info, + } = e + { + tracing::info!("Disabling GitHub Actions Cache due to 429: Too Many Requests"); + self.store(true, Ordering::Relaxed); + } + } +}