Compare site: always use cached master branch commits, update cache in background

Author: miwig

site/src/comparison.rs

@@ -25,10 +25,7 @@ pub async fn handle_triage(
     log::info!("handle_triage({:?})", body);
     let start = body.start;
     let end = body.end;
-    ctxt.update_master_commits()
-        .await
-        .map_err(|e| format!("error retrieving master commit list: {}", e))?;
-    let master_commits = &ctxt.master_commits.load().commits;
+    let master_commits = &ctxt.get_master_commits().commits;
 
     let start_artifact = ctxt
         .artifact_id_for_bound(start.clone(), true)
@@ -96,10 +93,7 @@ pub async fn handle_compare(
     ctxt: &SiteCtxt,
 ) -> api::ServerResult<api::comparison::Response> {
     log::info!("handle_compare({:?})", body);
-    ctxt.update_master_commits()
-        .await
-        .map_err(|e| format!("error retrieving master commit list: {}", e))?;
-    let master_commits = &ctxt.master_commits.load().commits;
+    let master_commits = &ctxt.get_master_commits().commits;
 
     let end = body.end;
     let comparison =
@@ -458,10 +452,9 @@ pub async fn compare(
     stat: String,
     ctxt: &SiteCtxt,
 ) -> Result<Option<Comparison>, BoxedError> {
-    ctxt.update_master_commits().await?;
-    let master_commits = &ctxt.master_commits.load().commits;
+    let master_commits = &ctxt.get_master_commits().commits;
 
-    compare_given_commits(start, end, stat, ctxt, &master_commits).await
+    compare_given_commits(start, end, stat, ctxt, master_commits).await
 }
 
 /// Compare two bounds on a given stat

site/src/load.rs

@@ -1,13 +1,13 @@
-use arc_swap::ArcSwap;
+use arc_swap::{ArcSwap, Guard};
 use std::collections::HashSet;
 use std::fs;
 use std::ops::RangeInclusive;
 use std::path::Path;
 use std::sync::Arc;
 use std::time::Instant;
 
-use anyhow::Context;
 use chrono::{Duration, Utc};
+use log::error;
 use serde::{Deserialize, Serialize};
 
 use crate::db;
@@ -109,13 +109,21 @@ pub struct MasterCommitCache {
 
 impl MasterCommitCache {
     /// Download the master-branch Rust commit list
-    async fn download() -> anyhow::Result<Self> {
+    pub async fn download() -> anyhow::Result<Self> {
         let commits = collector::master_commits().await?;
         Ok(Self {
             commits,
             updated: Instant::now(),
         })
     }
+
+    /// Returns an empty cache that is marked as stale
+    pub fn empty() -> Self {
+        Self {
+            commits: Vec::new(),
+            updated: Instant::now() - std::time::Duration::from_secs(2 * 60),
+        }
+    }
 }
 
 /// Site context object that contains global data
@@ -127,7 +135,7 @@ pub struct SiteCtxt {
     /// Index of various common queries
     pub index: ArcSwap<crate::db::Index>,
     /// Cached master-branch Rust commits
-    pub master_commits: ArcSwap<MasterCommitCache>,
+    pub master_commits: Arc<ArcSwap<MasterCommitCache>>, // outer Arc enables mutation in background task
     /// Database connection pool
     pub pool: Pool,
 }
@@ -180,12 +188,14 @@ impl SiteCtxt {
             }
         };
 
-        let master_commits = MasterCommitCache::download().await?;
+        let master_commits = MasterCommitCache::download()
+            .await
+            .unwrap_or(MasterCommitCache::empty()); // still run the site if we can't get the commits right now
 
         Ok(Self {
             config,
             index: ArcSwap::new(Arc::new(index)),
-            master_commits: ArcSwap::new(Arc::new(master_commits)),
+            master_commits: Arc::new(ArcSwap::new(Arc::new(master_commits))),
             pool,
             landing_page: ArcSwap::new(Arc::new(None)),
         })
@@ -223,14 +233,27 @@ impl SiteCtxt {
         )
     }
 
-    /// Download master-branch Rust commits if the cached value is older than one minute
-    pub async fn update_master_commits(&self) -> anyhow::Result<()> {
+    /// Get cached master-branch Rust commits.
+    /// Returns cached results immediately, but if the cached value is older than one minute,
+    /// updates in a background task for next time.
+    pub fn get_master_commits(&self) -> Guard<Arc<MasterCommitCache>> {
         let commits = self.master_commits.load();
+
         if commits.updated.elapsed() > std::time::Duration::from_secs(60) {
-            self.master_commits
-                .store(Arc::new(MasterCommitCache::download().await?))
+            let master_commits = self.master_commits.clone();
+            tokio::task::spawn(async move {
+                // if another update happens before this one is done, we will download the data twice, but that's it
+                match MasterCommitCache::download().await {
+                    Ok(commits) => master_commits.store(Arc::new(commits)),
+                    Err(e) => {
+                        // couldn't get the data, keep serving cached results for now
+                        error!("error retrieving master commit list: {}", e)
+                    }
+                }
+            });
         }
-        Ok(())
+
+        commits
     }
 }
 

site/src/server.rs

@@ -277,7 +277,7 @@ impl Server {
         // Refresh the landing page
         ctxt.landing_page.store(Arc::new(None));
 
-        let _ = ctxt.update_master_commits().await;
+        let _ = ctxt.get_master_commits();
 
         // Spawn off a task to post the results of any commit results that we
         // are now aware of.