From b9f89bd546ee188f8a03f16db79f97ae471464ec Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Tue, 14 Jan 2025 13:56:17 -0800 Subject: [PATCH 1/4] Suggest FlakeHub Cache when hit by 429 --- gha-cache/src/api.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/gha-cache/src/api.rs b/gha-cache/src/api.rs index 1f0b545..52a7a83 100644 --- a/gha-cache/src/api.rs +++ b/gha-cache/src/api.rs @@ -624,10 +624,16 @@ impl AtomicCircuitBreaker for AtomicBool { 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"); + let msg = "\ + Magic Nix Cache has exceeded GitHub Actions Cache's rate limit. \ + Save more time and seamlessly share the builds across your team with FlakeHub Cache. \ + See: https://flakehub.com/pricing\ + "; + println!("::notice::{msg}"); self.store(true, Ordering::Relaxed); } } From 003f106338209253110db29f44e6f5521092c605 Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Thu, 16 Jan 2025 07:32:20 -0800 Subject: [PATCH 2/4] Update GHA 429 notice wording Co-authored-by: Graham Christensen --- gha-cache/src/api.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/gha-cache/src/api.rs b/gha-cache/src/api.rs index 52a7a83..7d56626 100644 --- a/gha-cache/src/api.rs +++ b/gha-cache/src/api.rs @@ -628,12 +628,13 @@ impl AtomicCircuitBreaker for AtomicBool { } = e { tracing::info!("Disabling GitHub Actions Cache due to 429: Too Many Requests"); + let title = "Magic Nix Cache was rate-limited by GitHub Actions."; let msg = "\ - Magic Nix Cache has exceeded GitHub Actions Cache's rate limit. \ - Save more time and seamlessly share the builds across your team with FlakeHub Cache. \ - See: https://flakehub.com/pricing\ + Turn Magic Nix Cache into a cache for your whole team. \ + Move beyond GitHub's limits, save time, and share builds outside CI with FlakeHub Cache. \ + See: https://dtr.mn/github-cache-limits\ "; - println!("::notice::{msg}"); + println!("::notice title={title}::{msg}"); self.store(true, Ordering::Relaxed); } } From 322a99d45eb58f011847d0d868e1cf5a522704f3 Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Thu, 16 Jan 2025 07:34:14 -0800 Subject: [PATCH 3/4] fixup: update upload-artifact, download-artifact action versions --- .github/workflows/build.yaml | 2 +- .github/workflows/check-and-test.yaml | 2 +- .github/workflows/release-prs.yml | 8 ++++---- .github/workflows/release-tags.yml | 8 ++++---- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 2bc09fd..936bce5 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -45,7 +45,7 @@ jobs: nix-store --export $(nix-store -qR ./result) | xz -9 > "${{ env.ARCHIVE_NAME }}" - name: Upload magic-nix-cache closure for ${{ matrix.systems.system }} - uses: actions/upload-artifact@v3.1.2 + uses: actions/upload-artifact@v4.6.0 with: # Artifact name name: ${{ env.ARTIFACT_KEY }} diff --git a/.github/workflows/check-and-test.yaml b/.github/workflows/check-and-test.yaml index 116a6d4..0951dc5 100644 --- a/.github/workflows/check-and-test.yaml +++ b/.github/workflows/check-and-test.yaml @@ -63,7 +63,7 @@ jobs: - uses: actions/checkout@v4 - name: Download closure for ${{ matrix.systems.system }} - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4.1.8 with: name: ${{ env.ARTIFACT_KEY }} path: ${{ env.ARTIFACT_KEY }} diff --git a/.github/workflows/release-prs.yml b/.github/workflows/release-prs.yml index 501e20c..0a6475c 100644 --- a/.github/workflows/release-prs.yml +++ b/.github/workflows/release-prs.yml @@ -36,28 +36,28 @@ jobs: - name: Create the artifacts directory run: rm -rf ./artifacts && mkdir ./artifacts - - uses: actions/download-artifact@v3 + - uses: actions/download-artifact@v4.1.8 with: name: magic-nix-cache-ARM64-macOS path: cache-binary-ARM64-macOS - name: Persist the cache binary run: cp ./cache-binary-ARM64-macOS/magic-nix-cache.closure.xz ./artifacts/magic-nix-cache-ARM64-macOS - - uses: actions/download-artifact@v3 + - uses: actions/download-artifact@v4.1.8 with: name: magic-nix-cache-X64-macOS path: cache-binary-X64-macOS - name: Persist the cache binary run: cp ./cache-binary-X64-macOS/magic-nix-cache.closure.xz ./artifacts/magic-nix-cache-X64-macOS - - uses: actions/download-artifact@v3 + - uses: actions/download-artifact@v4.1.8 with: name: magic-nix-cache-X64-Linux path: cache-binary-X64-Linux - name: Persist the cache binary run: cp ./cache-binary-X64-Linux/magic-nix-cache.closure.xz ./artifacts/magic-nix-cache-X64-Linux - - uses: actions/download-artifact@v3 + - uses: actions/download-artifact@v4.1.8 with: name: magic-nix-cache-ARM64-Linux path: cache-binary-ARM64-Linux diff --git a/.github/workflows/release-tags.yml b/.github/workflows/release-tags.yml index 8f0bda7..7d9d38b 100644 --- a/.github/workflows/release-tags.yml +++ b/.github/workflows/release-tags.yml @@ -24,28 +24,28 @@ jobs: - name: Create the artifacts directory run: rm -rf ./artifacts && mkdir ./artifacts - - uses: actions/download-artifact@v3 + - uses: actions/download-artifact@v4.1.8 with: name: magic-nix-cache-ARM64-macOS path: cache-binary-ARM64-macOS - name: Persist the cache binary run: cp ./cache-binary-ARM64-macOS/magic-nix-cache.closure.xz ./artifacts/magic-nix-cache-ARM64-macOS - - uses: actions/download-artifact@v3 + - uses: actions/download-artifact@v4.1.8 with: name: magic-nix-cache-X64-macOS path: cache-binary-X64-macOS - name: Persist the cache binary run: cp ./cache-binary-X64-macOS/magic-nix-cache.closure.xz ./artifacts/magic-nix-cache-X64-macOS - - uses: actions/download-artifact@v3 + - uses: actions/download-artifact@v4.1.8 with: name: magic-nix-cache-X64-Linux path: cache-binary-X64-Linux - name: Persist the cache binary run: cp ./cache-binary-X64-Linux/magic-nix-cache.closure.xz ./artifacts/magic-nix-cache-X64-Linux - - uses: actions/download-artifact@v3 + - uses: actions/download-artifact@v4.1.8 with: name: magic-nix-cache-ARM64-Linux path: cache-binary-ARM64-Linux From 4e2b37be36557b29dd437630f2879bf8af1ab354 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Thu, 16 Jan 2025 11:13:00 -0500 Subject: [PATCH 4/4] Set a metric field for when GHA 429's --- gha-cache/src/api.rs | 44 ++++++++++++++++++++++++-------- magic-nix-cache/src/gha.rs | 10 +++++++- magic-nix-cache/src/telemetry.rs | 2 ++ 3 files changed, 44 insertions(+), 12 deletions(-) diff --git a/gha-cache/src/api.rs b/gha-cache/src/api.rs index 7d56626..3b42d66 100644 --- a/gha-cache/src/api.rs +++ b/gha-cache/src/api.rs @@ -48,6 +48,8 @@ const MAX_CONCURRENCY: usize = 4; type Result = std::result::Result; +pub type CircuitBreakerTrippedCallback = Arc>; + /// An API error. #[derive(Error, Debug)] pub enum Error { @@ -82,7 +84,6 @@ pub enum Error { TooManyCollisions, } -#[derive(Debug)] pub struct Api { /// Credentials to access the cache. credentials: Credentials, @@ -104,6 +105,8 @@ pub struct Api { circuit_breaker_429_tripped: Arc, + circuit_breaker_429_tripped_callback: CircuitBreakerTrippedCallback, + /// Backend request statistics. #[cfg(debug_assertions)] stats: RequestStats, @@ -242,7 +245,10 @@ impl fmt::Display for ApiErrorInfo { } impl Api { - pub fn new(credentials: Credentials) -> Result { + pub fn new( + credentials: Credentials, + circuit_breaker_429_tripped_callback: CircuitBreakerTrippedCallback, + ) -> Result { let mut headers = HeaderMap::new(); let auth_header = { let mut h = HeaderValue::from_str(&format!("Bearer {}", credentials.runtime_token)) @@ -273,6 +279,7 @@ impl Api { client, concurrency_limit: Arc::new(Semaphore::new(MAX_CONCURRENCY)), circuit_breaker_429_tripped: Arc::new(AtomicBool::from(false)), + circuit_breaker_429_tripped_callback, #[cfg(debug_assertions)] stats: Default::default(), }) @@ -366,6 +373,8 @@ impl Api { let client = self.client.clone(); let concurrency_limit = self.concurrency_limit.clone(); let circuit_breaker_429_tripped = self.circuit_breaker_429_tripped.clone(); + let circuit_breaker_429_tripped_callback = + self.circuit_breaker_429_tripped_callback.clone(); let url = self.construct_url(&format!("caches/{}", allocation.0 .0)); tokio::task::spawn(async move { @@ -402,7 +411,8 @@ impl Api { drop(permit); - circuit_breaker_429_tripped.check_result(&r); + circuit_breaker_429_tripped + .check_result(&r, &circuit_breaker_429_tripped_callback); r }) @@ -465,7 +475,8 @@ impl Api { .check_json() .await; - self.circuit_breaker_429_tripped.check_result(&res); + self.circuit_breaker_429_tripped + .check_result(&res, &self.circuit_breaker_429_tripped_callback); match res { Ok(entry) => Ok(Some(entry)), @@ -508,7 +519,8 @@ impl Api { .check_json() .await; - self.circuit_breaker_429_tripped.check_result(&res); + self.circuit_breaker_429_tripped + .check_result(&res, &self.circuit_breaker_429_tripped_callback); res } @@ -535,7 +547,8 @@ impl Api { .check() .await { - self.circuit_breaker_429_tripped.check_err(&e); + self.circuit_breaker_429_tripped + .check_err(&e, &self.circuit_breaker_429_tripped_callback); return Err(e); } @@ -610,18 +623,26 @@ async fn handle_error(res: reqwest::Response) -> Error { } trait AtomicCircuitBreaker { - fn check_err(&self, e: &Error); - fn check_result(&self, r: &std::result::Result); + fn check_err(&self, e: &Error, callback: &CircuitBreakerTrippedCallback); + fn check_result( + &self, + r: &std::result::Result, + callback: &CircuitBreakerTrippedCallback, + ); } impl AtomicCircuitBreaker for AtomicBool { - fn check_result(&self, r: &std::result::Result) { + fn check_result( + &self, + r: &std::result::Result, + callback: &CircuitBreakerTrippedCallback, + ) { if let Err(ref e) = r { - self.check_err(e) + self.check_err(e, callback) } } - fn check_err(&self, e: &Error) { + fn check_err(&self, e: &Error, callback: &CircuitBreakerTrippedCallback) { if let Error::ApiError { status: reqwest::StatusCode::TOO_MANY_REQUESTS, .. @@ -636,6 +657,7 @@ impl AtomicCircuitBreaker for AtomicBool { "; println!("::notice title={title}::{msg}"); self.store(true, Ordering::Relaxed); + callback(); } } } diff --git a/magic-nix-cache/src/gha.rs b/magic-nix-cache/src/gha.rs index cd4df57..086c97d 100644 --- a/magic-nix-cache/src/gha.rs +++ b/magic-nix-cache/src/gha.rs @@ -37,7 +37,15 @@ impl GhaCache { metrics: Arc, narinfo_negative_cache: Arc>>, ) -> Result { - let mut api = Api::new(credentials)?; + let cb_metrics = metrics.clone(); + let mut api = Api::new( + credentials, + Arc::new(Box::new(move || { + cb_metrics + .tripped_429 + .store(true, std::sync::atomic::Ordering::Relaxed); + })), + )?; if let Some(cache_version) = &cache_version { api.mutate_version(cache_version.as_bytes()); diff --git a/magic-nix-cache/src/telemetry.rs b/magic-nix-cache/src/telemetry.rs index c9dfc3e..59de261 100644 --- a/magic-nix-cache/src/telemetry.rs +++ b/magic-nix-cache/src/telemetry.rs @@ -28,6 +28,8 @@ pub struct TelemetryReport { pub num_original_paths: Metric, pub num_final_paths: Metric, pub num_new_paths: Metric, + + pub tripped_429: std::sync::atomic::AtomicBool, } #[derive(Debug, Default, serde::Serialize)]