Discover experiments in need of reporting on reports thread

Mark-Simulacrum · Mark-Simulacrum · commit 61d761931971 · 2022-03-04T20:48:55.000-05:00
This avoids each new result needing to scan the results and experiment_crates
tables in order to discover the number of in-progress results. Instead, that
work will happen every ~10 minutes (by current setting) in the background.

It's not clear that this is going to help with our performance problems (though
it should not hurt!). Regardless, this is also a good step towards avoiding the need to
retry report generation, since on server restarts we will automatically discover
the need to generate a fresh report (rather than needing to manually move an
experiment back to the needs-report state).

Future commits will clean up some of the states that we no longer need as a
result of this change.
diff --git a/src/experiments.rs b/src/experiments.rs
@@ -298,20 +298,20 @@ impl Experiment {
         }
     }
 
-    pub fn first_by_status(db: &Database, status: Status) -> Fallible<Option<Experiment>> {
-        let record = db.get_row(
-            "SELECT * FROM experiments \
-             WHERE status = ?1 \
-             ORDER BY priority DESC, created_at;",
-            &[&status.to_str()],
-            |r| ExperimentDBRecord::from_row(r),
-        )?;
-
-        if let Some(record) = record {
-            Ok(Some(record.into_experiment()?))
-        } else {
-            Ok(None)
+    // Returns the first experiment which has all results ready (and so can
+    // produce a complete report). However, the experiment should not be
+    // *completed* yet. Note that this may return an experiment which has had
+    // report generation already start.
+    pub fn ready_for_report(db: &Database) -> Fallible<Option<Experiment>> {
+        let unfinished = Self::unfinished(db)?;
+        for ex in unfinished {
+            let (completed, all) = ex.raw_progress(db)?;
+            if completed == all {
+                return Ok(Some(ex));
+            }
         }
+
+        Ok(None)
     }
 
     pub fn find_next(db: &Database, assignee: &Assignee) -> Fallible<Option<Experiment>> {
diff --git a/src/server/reports.rs b/src/server/reports.rs
@@ -37,7 +37,7 @@ fn reports_thread(data: &Data, github_data: Option<&GithubData>) -> Fallible<()>
     let results = DatabaseDB::new(&data.db);
 
     loop {
-        let mut ex = match Experiment::first_by_status(&data.db, Status::NeedsReport)? {
+        let mut ex = match Experiment::ready_for_report(&data.db)? {
             Some(ex) => ex,
             None => {
                 // This will sleep AUTOMATIC_THREAD_WAKEUP seconds *or* until a wake is received
diff --git a/src/server/routes/agent.rs b/src/server/routes/agent.rs
@@ -1,5 +1,5 @@
 use crate::agent::Capabilities;
-use crate::experiments::{Assignee, Experiment, Status};
+use crate::experiments::{Assignee, Experiment};
 use crate::prelude::*;
 use crate::results::{DatabaseDB, EncodingType, ProgressData};
 use crate::server::api_types::{AgentConfig, ApiResponse};
@@ -160,7 +160,7 @@ fn endpoint_record_progress(
     auth: AuthDetails,
 ) -> Fallible<Response<Body>> {
     let data = mutex.lock().unwrap();
-    let mut ex = Experiment::get(&data.db, &result.experiment_name)?
+    let ex = Experiment::get(&data.db, &result.experiment_name)?
         .ok_or_else(|| err_msg("no experiment run by this agent"))?;
 
     data.metrics
@@ -169,13 +169,6 @@ fn endpoint_record_progress(
     let db = DatabaseDB::new(&data.db);
     db.store(&ex, &result.data, EncodingType::Gzip)?;
 
-    let (completed, all) = ex.raw_progress(&data.db)?;
-    if completed == all {
-        ex.set_status(&data.db, Status::NeedsReport)?;
-        info!("experiment {} completed, marked as needs-report", ex.name);
-        data.reports_worker.wake(); // Ensure the reports worker is awake
-    }
-
     Ok(ApiResponse::Success { result: true }.into_response()?)
 }
 
