Auto merge of #401 - Zeegomo:separate-experiment-crates, r=pietroalbini

Separate crates from experiment

`Experiment` does not hold anymore the list of crates. Instead, relevant crates can be queried with a new method `get_crates(db: &Database) -> Vec<Crate>`.
As a consequence, `next_experiment` from agent api now returns both an `Experiment` and a `Vec<Crate>` and runner require also a `&[Crate]` argument besides the experiment.

src/actions/experiments/create.rs

@@ -131,7 +131,7 @@ mod tests {
  );
  assert_eq!(ex.mode, Mode::BuildAndTest);
  assert_eq!(
- ex.crates,
+ ex.get_crates(&ctx.db).unwrap(),
  crate::crates::lists::get_crates(CrateSelect::Local, &db, &config).unwrap()
  );
  assert_eq!(ex.cap_lints, CapLints::Forbid);

src/actions/experiments/edit.rs

@@ -80,7 +80,7 @@ impl Action for EditExperiment {
  &ctx.config,
  )?)
  } else if self.ignore_blacklist.is_some() {
- Some(ex.crates.clone())
+ Some(ex.get_crates(&ctx.db)?)
  } else {
  None
  };
@@ -102,7 +102,6 @@ impl Action for EditExperiment {
  ],
  )?;
  }
- ex.crates = crates_vec;
  }
 
  // Try to update the mode
@@ -212,7 +211,7 @@ mod tests {
  assert_eq!(ex.ignore_blacklist, true);
 
  assert_eq!(
- ex.crates,
+ ex.get_crates(&ctx.db).unwrap(),
  crate::crates::lists::get_crates(CrateSelect::Local, &db, &config).unwrap()
  );
  }

src/agent/api.rs

@@ -128,15 +128,15 @@ impl AgentApi {
  })
  }
 
- pub fn next_experiment(&self) -> Fallible<Experiment> {
+ pub fn next_experiment(&self) -> Fallible<(Experiment, Vec<Crate>)> {
  self.retry(|this| loop {
  let resp: Option<_> = this
  .build_request(Method::GET, "next-experiment")
  .send()?
  .to_api_response()?;
 
- if let Some(experiment) = resp {
- return Ok(experiment);
+ if let Some((experiment, crates)) = resp {
+ return Ok((experiment, crates));
  }
 
  ::std::thread::sleep(::std::time::Duration::from_secs(RETRY_AFTER));

src/agent/mod.rs

@@ -4,6 +4,7 @@ mod results;
 use crate::agent::api::AgentApi;
 use crate::agent::results::ResultsUploader;
 use crate::config::Config;
+use crate::crates::Crate;
 use crate::experiments::Experiment;
 use crate::prelude::*;
 use crate::utils;
@@ -31,7 +32,7 @@ impl Agent {
  })
  }
 
- fn experiment(&self) -> Fallible<Experiment> {
+ fn experiment(&self) -> Fallible<(Experiment, Vec<Crate>)> {
  info!("asking the server for a new experiment...");
  Ok(self.api.next_experiment()?)
  }
@@ -54,8 +55,8 @@ fn run_experiment(
  threads_count: usize,
  docker_env: &str,
 ) -> Fallible<()> {
- let ex = agent.experiment()?;
- crate::runner::run_ex(&ex, db, threads_count, &agent.config, docker_env)?;
+ let (ex, crates) = agent.experiment()?;
+ crate::runner::run_ex(&ex, &crates, db, threads_count, &agent.config, docker_env)?;
  agent.api.complete_experiment()?;
  Ok(())
 }

src/cli.rs

@@ -426,7 +426,14 @@ impl Crater {
  }
 
  let result_db = DatabaseDB::new(&db);
- runner::run_ex(&experiment, &result_db, threads, &config, docker_env)?;
+ runner::run_ex(
+ &experiment,
+ &experiment.get_uncompleted_crates(&db)?,
+ &result_db,
+ threads,
+ &config,
+ docker_env,
+ )?;
  experiment.set_status(&db, Status::NeedsReport)?;
  } else {
  bail!("missing experiment {}", ex.0);
@@ -457,6 +464,7 @@ impl Crater {
  let res = report::gen(
  &result_db,
  &experiment,
+ &experiment.get_crates(&db)?,
  &report::FileWriter::create(dest.0.clone())?,
  &config,
  );
@@ -498,6 +506,7 @@ impl Crater {
  let res = report::gen(
  &result_db,
  &experiment,
+ &experiment.get_crates(&db)?,
  &report::S3Writer::create(client, s3_prefix.clone())?,
  &config,
  );
@@ -533,7 +542,7 @@ impl Crater {
  let db = Database::open()?;
 
  if let Some(experiment) = Experiment::get(&db, &ex.0)? {
- runner::dump_dot(&experiment, &config, dest)?;
+ runner::dump_dot(&experiment, &experiment.get_crates(&db)?, &config, dest)?;
  } else {
  bail!("missing experiment: {}", ex.0);
  }

src/experiments.rs

@@ -101,17 +101,16 @@ impl FromStr for Assignee {
  }
 }
 
-#[derive(Serialize, Deserialize)]
+#[derive(Serialize, Deserialize, Clone)]
 pub struct GitHubIssue {
  pub api_url: String,
  pub html_url: String,
  pub number: i32,
 }
 
-#[derive(Serialize, Deserialize)]
+#[derive(Serialize, Deserialize, Clone)]
 pub struct Experiment {
  pub name: String,
- pub crates: Vec<Crate>,
  pub toolchains: [Toolchain; 2],
  pub mode: Mode,
  pub cap_lints: CapLints,
@@ -139,7 +138,7 @@ impl Experiment {
  )?;
  records
  .into_iter()
- .map(|record| record.into_experiment(db))
+ .map(|record| record.into_experiment())
  .collect::<Fallible<_>>()
  }
 
@@ -152,7 +151,7 @@ impl Experiment {
  )?;
 
  if let Some(record) = record {
- Ok(Some(record.into_experiment(db)?))
+ Ok(Some(record.into_experiment()?))
  } else {
  Ok(None)
  }
@@ -168,7 +167,7 @@ impl Experiment {
  )?;
 
  if let Some(record) = record {
- Ok(Some(record.into_experiment(db)?))
+ Ok(Some(record.into_experiment()?))
  } else {
  Ok(None)
  }
@@ -189,7 +188,7 @@ impl Experiment {
  )?;
 
  if let Some(record) = record {
- let mut experiment = record.into_experiment(db)?;
+ let mut experiment = record.into_experiment()?;
  experiment.set_status(&db, Status::Running)?;
  experiment.set_assigned_to(&db, Some(assignee))?;
  Ok(Some((true, experiment)))
@@ -206,7 +205,7 @@ impl Experiment {
  )?;
 
  if let Some(record) = record {
- Ok(Some(record.into_experiment(db)?))
+ Ok(Some(record.into_experiment()?))
  } else {
  Ok(None)
  }
@@ -296,26 +295,31 @@ impl Experiment {
  }
  }
 
- pub fn remove_completed_crates(&mut self, db: &Database) -> Fallible<()> {
- // FIXME: optimize this
- let mut new_crates = Vec::with_capacity(self.crates.len());
- for krate in self.crates.drain(..) {
- let results_len: u32 = db
- .get_row(
- "SELECT COUNT(*) AS count FROM results \
- WHERE experiment = ?1 AND crate = ?2;",
- &[&self.name.as_str(), &serde_json::to_string(&krate)?],
- |r| r.get("count"),
- )?
- .unwrap();
-
- if results_len < 2 {
- new_crates.push(krate);
- }
- }
+ pub fn get_crates(&self, db: &Database) -> Fallible<Vec<Crate>> {
+ db.query(
+ "SELECT crate FROM experiment_crates WHERE experiment = ?1;",
+ &[&self.name],
+ |r| {
+ let value: String = r.get("crate");
+ Ok(serde_json::from_str(&value)?)
+ },
+ )?
+ .into_iter()
+ .collect::<Fallible<Vec<Crate>>>()
+ }
 
- self.crates = new_crates;
- Ok(())
+ pub fn get_uncompleted_crates(&self, db: &Database) -> Fallible<Vec<Crate>> {
+ db.query(
+ "SELECT crate FROM experiment_crates WHERE experiment = ?1
+ AND (SELECT COUNT(*) AS count FROM results WHERE results.experiment = ?1 AND results.crate = experiment_crates.crate) < 2;",
+ &[&self.name],
+ |r| {
+ let value: String = r.get("crate");
+ Ok(serde_json::from_str(&value)?)
+ },
+ )?
+ .into_iter()
+ .collect::<Fallible<Vec<Crate>>>()
  }
 }
 
@@ -360,22 +364,9 @@ impl ExperimentDBRecord {
  }
  }
 
- fn into_experiment(self, db: &Database) -> Fallible<Experiment> {
- let crates = db
- .query(
- "SELECT crate FROM experiment_crates WHERE experiment = ?1",
- &[&self.name],
- |r| {
- let value: String = r.get("crate");
- Ok(serde_json::from_str(&value)?)
- },
- )?
- .into_iter()
- .collect::<Fallible<Vec<Crate>>>()?;
-
+ fn into_experiment(self) -> Fallible<Experiment> {
  Ok(Experiment {
  name: self.name,
- crates,
  toolchains: [self.toolchain_start.parse()?, self.toolchain_end.parse()?],
  cap_lints: self.cap_lints.parse()?,
  mode: self.mode.parse()?,
@@ -496,4 +487,72 @@ mod tests {
  // Test no other experiment is available for the other agents
  assert!(Experiment::next(&db, &agent3).unwrap().is_none());
  }
+
+ #[test]
+ fn test_completed_crates() {
+ use crate::prelude::*;
+ use crate::results::{DatabaseDB, EncodingType, FailureReason, TestResult, WriteResults};
+
+ let db = Database::temp().unwrap();
+ let config = Config::default();
+ let ctx = ActionsCtx::new(&db, &config);
+
+ crate::crates::lists::setup_test_lists(&db, &config).unwrap();
+
+ // Create a dummy experiment
+ CreateExperiment::dummy("dummy").apply(&ctx).unwrap();
+ let ex = Experiment::get(&db, "dummy").unwrap().unwrap();
+ let crates = ex.get_uncompleted_crates(&db).unwrap();
+ let crate1 = &crates[0];
+ let crate2 = &crates[1];
+
+ // Fill some dummy results into the database
+ let results = DatabaseDB::new(&db);
+ results
+ .record_result(
+ &ex,
+ &ex.toolchains[0],
+ &crate1,
+ None,
+ &config,
+ EncodingType::Gzip,
+ || {
+ info!("tc1 crate1");
+ Ok(TestResult::TestPass)
+ },
+ )
+ .unwrap();
+ results
+ .record_result(
+ &ex,
+ &ex.toolchains[1],
+ &crate1,
+ None,
+ &config,
+ EncodingType::Plain,
+ || {
+ info!("tc2 crate1");
+ Ok(TestResult::BuildFail(FailureReason::Unknown))
+ },
+ )
+ .unwrap();
+ results
+ .record_result(
+ &ex,
+ &ex.toolchains[1],
+ &crate2,
+ None,
+ &config,
+ EncodingType::Plain,
+ || {
+ info!("tc1 crate2");
+ Ok(TestResult::BuildFail(FailureReason::Unknown))
+ },
+ )
+ .unwrap();
+
+ // Test already completed crates does not show up again
+ let uncompleted_crates = ex.get_uncompleted_crates(&db).unwrap();
+ assert_eq!(uncompleted_crates.len(), crates.len() - 1);
+ }
 }

src/report/archives.rs

@@ -1,4 +1,5 @@
 use crate::config::Config;
+use crate::crates::Crate;
 use crate::experiments::Experiment;
 use crate::prelude::*;
 use crate::report::{compare, ReportWriter};
@@ -16,14 +17,15 @@ pub struct Archive {
 pub fn write_logs_archives<DB: ReadResults, W: ReportWriter>(
  db: &DB,
  ex: &Experiment,
+ crates: &[Crate],
  dest: &W,
  config: &Config,
 ) -> Fallible<Vec<Archive>> {
  let mut archives = Vec::new();
  let mut all = TarBuilder::new(GzEncoder::new(Vec::new(), Compression::default()));
  let mut by_comparison = HashMap::new();
 
- for krate in &ex.crates {
+ for krate in crates {
  if config.should_skip(krate) {
  continue;
  }
@@ -126,8 +128,8 @@ mod tests {
  // Create a dummy experiment
  CreateExperiment::dummy("dummy").apply(&ctx).unwrap();
  let ex = Experiment::get(&db, "dummy").unwrap().unwrap();
- let crate1 = ex.crates[0].clone();
- let crate2 = ex.crates[1].clone();
+ let crate1 = &ex.get_crates(&db).unwrap()[0];
+ let crate2 = &ex.get_crates(&db).unwrap()[1];
 
  // Fill some dummy results into the database
  let results = DatabaseDB::new(&db);
@@ -189,7 +191,14 @@ mod tests {
  .unwrap();
 
  // Generate all the archives
- let archives = write_logs_archives(&results, &ex, &writer, &config).unwrap();
+ let archives = write_logs_archives(
+ &results,
+ &ex,
+ &ex.get_crates(&db).unwrap(),
+ &writer,
+ &config,
+ )
+ .unwrap();
 
  // Ensure the correct list of archives is returned
  let mut archives_paths = archives.into_iter().map(|a| a.path).collect::<Vec<_>>();