Skip to content

Commit 6b9dbb5

Browse files
committed
Move update-downloads to a background job
This replaces the `update-downloads` binary with a background job, and a binary that is used to queue up a given job, which we will run from Heroku scheduler. This accomplishes 2 things: - It makes it easier to write tasks that need to run periodically (e.g. cleaning up stale rate limit buckets), since we don't need to create a new standalone binary. - `update_downloads` and any future recurring tasks will automatically get monitoring if they fail, since we are already monitoring for background jobs not being successfully run. Right now the intent is to have `enqueue-job update_downloads` get run periodically by Heroku scheudler (and a similar scheduled task for any future tasks that are added). Once swirl gains the ability to schedule jobs to be run at arbitrary points in the future, we could instead have these jobs re-queue themselves once they complete, and have the cron task just look to see if any job is queued for each given type, queuing it if not. That would have a bit less boilerplate, but a lot more complexity. Fixes #1797.
1 parent 25b52da commit 6b9dbb5

File tree

4 files changed

+36
-19
lines changed

4 files changed

+36
-19
lines changed

src/bin/enqueue-job.rs

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
use cargo_registry::util::{CargoError, CargoResult};
2+
use cargo_registry::{db, tasks};
3+
use std::env::args;
4+
use swirl::Job;
5+
6+
fn main() -> CargoResult<()> {
7+
let conn = db::connect_now()?;
8+
9+
match &*args().nth(1).unwrap_or_default() {
10+
"update_downloads" => tasks::update_downloads()
11+
.enqueue(&conn)
12+
.map_err(|e| CargoError::from_std_error(e))?,
13+
other => panic!("Unrecognized job type `{}`", other),
14+
};
15+
16+
Ok(())
17+
}

src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ pub mod middleware;
4040
mod publish_rate_limit;
4141
pub mod render;
4242
pub mod schema;
43+
pub mod tasks;
4344
mod test_util;
4445
pub mod uploaders;
4546
pub mod util;

src/tasks.rs

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
mod update_downloads;
2+
3+
pub use update_downloads::update_downloads;

src/bin/update-downloads.rs renamed to src/tasks/update_downloads.rs

+15-19
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,21 @@
1-
#![deny(warnings, clippy::all, rust_2018_idioms)]
2-
3-
#[macro_use]
4-
extern crate diesel;
5-
6-
use cargo_registry::{
7-
db,
1+
use crate::{
2+
background_jobs::Environment,
83
models::VersionDownload,
94
schema::{crates, metadata, version_downloads, versions},
10-
util::CargoResult,
115
};
126

137
use diesel::prelude::*;
8+
use swirl::PerformError;
149

15-
fn main() -> CargoResult<()> {
16-
let conn = db::connect_now()?;
10+
#[swirl::background_job]
11+
pub fn update_downloads(env: &Environment) -> Result<(), PerformError> {
12+
let conn = env.connection()?;
1713
update(&conn)?;
1814
Ok(())
1915
}
2016

2117
fn update(conn: &PgConnection) -> QueryResult<()> {
22-
use crate::version_downloads::dsl::*;
18+
use self::version_downloads::dsl::*;
2319
use diesel::dsl::now;
2420
use diesel::select;
2521

@@ -84,7 +80,7 @@ fn collect(conn: &PgConnection, rows: &[VersionDownload]) -> QueryResult<()> {
8480
#[cfg(test)]
8581
mod test {
8682
use super::*;
87-
use cargo_registry::{
83+
use crate::{
8884
env,
8985
models::{Crate, NewCrate, NewUser, NewVersion, User, Version},
9086
};
@@ -143,7 +139,7 @@ mod test {
143139
.execute(&conn)
144140
.unwrap();
145141

146-
crate::update(&conn).unwrap();
142+
super::update(&conn).unwrap();
147143
let version_downloads = versions::table
148144
.find(version.id)
149145
.select(versions::downloads)
@@ -154,7 +150,7 @@ mod test {
154150
.select(crates::downloads)
155151
.first(&conn);
156152
assert_eq!(Ok(1), crate_downloads);
157-
crate::update(&conn).unwrap();
153+
super::update(&conn).unwrap();
158154
let version_downloads = versions::table
159155
.find(version.id)
160156
.select(versions::downloads)
@@ -179,7 +175,7 @@ mod test {
179175
))
180176
.execute(&conn)
181177
.unwrap();
182-
crate::update(&conn).unwrap();
178+
super::update(&conn).unwrap();
183179
let processed = version_downloads::table
184180
.filter(version_downloads::version_id.eq(version.id))
185181
.select(version_downloads::processed)
@@ -203,7 +199,7 @@ mod test {
203199
))
204200
.execute(&conn)
205201
.unwrap();
206-
crate::update(&conn).unwrap();
202+
super::update(&conn).unwrap();
207203
let processed = version_downloads::table
208204
.filter(version_downloads::version_id.eq(version.id))
209205
.select(version_downloads::processed)
@@ -253,7 +249,7 @@ mod test {
253249
.filter(crates::id.eq(krate.id))
254250
.first::<Crate>(&conn)
255251
.unwrap();
256-
crate::update(&conn).unwrap();
252+
super::update(&conn).unwrap();
257253
let version2 = versions::table
258254
.find(version.id)
259255
.first::<Version>(&conn)
@@ -266,7 +262,7 @@ mod test {
266262
.unwrap();
267263
assert_eq!(krate2.downloads, 2);
268264
assert_eq!(krate2.updated_at, krate_before.updated_at);
269-
crate::update(&conn).unwrap();
265+
super::update(&conn).unwrap();
270266
let version3 = versions::table
271267
.find(version.id)
272268
.first::<Version>(&conn)
@@ -301,7 +297,7 @@ mod test {
301297
.execute(&conn)
302298
.unwrap();
303299

304-
crate::update(&conn).unwrap();
300+
super::update(&conn).unwrap();
305301
let versions_changed = versions::table
306302
.select(versions::updated_at.ne(now - 2.days()))
307303
.get_result(&conn);

0 commit comments

Comments
 (0)