Skip to content

Commit

Permalink
fix: prevent processor from attempting to run an empty select_all (#211)
Browse files Browse the repository at this point in the history
* feature: check that replicas is non-empty

* bug: filter subsidized remotes

* fix: prevent select_all from being called with empty specified subsidized

* chore: update changelog with bugfixes
  • Loading branch information
prestwich authored Jul 14, 2022
1 parent f797020 commit 7e1dc8b
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 8 deletions.
1 change: 1 addition & 0 deletions agents/processor/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

### Unreleased

- bug: add check for empty intersection of specified and subsidized
- refactor: processor now uses global AWS client when proof pushing is enabled
- prevent processor from retrying messages it has previously attempted to
process
Expand Down
23 changes: 15 additions & 8 deletions agents/processor/src/processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,12 +337,21 @@ impl NomadAgent for Processor {
where
Self: Sized,
{
// we filter this so that the agent doesn't think it should subsidize
// remotes it is unaware of
let subsidized_remotes = settings
.agent
.subsidized_remotes
.iter()
.filter(|r| settings.base.replicas.contains_key(*r))
.cloned()
.collect();
Ok(Self::new(
settings.agent.interval,
settings.as_ref().try_into_core(AGENT_NAME).await?,
settings.agent.allowed,
settings.agent.denied,
settings.agent.subsidized_remotes,
subsidized_remotes,
settings.agent.s3,
))
}
Expand Down Expand Up @@ -404,20 +413,18 @@ impl NomadAgent for Processor {
let mut tasks = vec![home_sync_task, prover_sync_task, home_fail_watch_task];

if !self.subsidized_remotes.is_empty() {
let specified_remotes: HashSet<String> =
self.replicas().keys().map(String::to_owned).collect();

// Get intersection of specified remotes (replicas in settings)
// and subsidized remotes
let specified_subsidized: Vec<&str> = self
.subsidized_remotes
.intersection(&specified_remotes)
.collect::<Vec<_>>()
.iter()
.map(|x| x.as_str())
.filter(|r| self.replicas().contains_key(*r))
.map(AsRef::as_ref)
.collect();

tasks.push(self.run_many(&specified_subsidized));
if !specified_subsidized.is_empty() {
tasks.push(self.run_many(&specified_subsidized));
}
}

// if we have a bucket, add a task to push to it
Expand Down
2 changes: 2 additions & 0 deletions nomad-base/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

### Unreleased

- bug: add checks for empty replica name arrays in `NomadAgent::run_many` and
`NomadAgent::run_all`
- add `previously_attempted` to the DB schema
- remove `enabled` flag from agents project-wide
- adds a changelog
11 changes: 11 additions & 0 deletions nomad-base/src/agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,12 @@ pub trait NomadAgent: Send + Sync + Sized + std::fmt::Debug + AsRef<AgentCore> {
#[allow(clippy::unit_arg)]
fn run_many(&self, replicas: &[&str]) -> Instrumented<JoinHandle<Result<()>>> {
let span = info_span!("run_many");

// easy check that the slice is non-empty
replicas
.first()
.expect("Attempted to run without any replicas");

let handles: Vec<_> = replicas
.iter()
.map(|replica| self.run_report_error(replica.to_string()))
Expand Down Expand Up @@ -196,6 +202,11 @@ pub trait NomadAgent: Send + Sync + Sized + std::fmt::Debug + AsRef<AgentCore> {
// this is the unused must use
let names: Vec<&str> = self.replicas().keys().map(|k| k.as_str()).collect();

// quick check that at least 1 replica is configured
names
.first()
.expect("Attempted to run without any replicas");

let run_task = self.run_many(&names);
let mut tasks = vec![run_task];

Expand Down

0 comments on commit 7e1dc8b

Please sign in to comment.