Skip to content

Commit 125aff9

Browse files
committed
Auto merge of #2267 - rust-lang:sg-no-sharing-worker-pool, r=jtgeibel
Create a new DB pool when rebuilding the worker When we fail to *start* (not run) new jobs, which could happen because all our worker threads are saturated by a job that is taking too long, we will rebuild the runner up to 5 times in an attempt to recover before killing the process. Importantly, if this occurred because of a slow job, this should give those slow jobs a chance to finish without blocking the rest of the queue. However, we were sharing a database pool even when we rebuilt the runner. This means that if the slow jobs held a database connection, our retries will just fail immedaitely, and we kill the process (along with any hopes of the slow job catching up). This creates a new connection pool whenever we restart the runner. r? @jtgeibel
2 parents 22cb857 + 4c5fa5b commit 125aff9

File tree

2 files changed

+34
-14
lines changed

2 files changed

+34
-14
lines changed

src/background_jobs.rs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,23 @@ impl Environment {
4949
connection_pool: DieselPool,
5050
uploader: Uploader,
5151
http_client: Client,
52+
) -> Self {
53+
Self::new_shared(
54+
Arc::new(Mutex::new(index)),
55+
connection_pool,
56+
uploader,
57+
http_client,
58+
)
59+
}
60+
61+
pub fn new_shared(
62+
index: Arc<Mutex<Repository>>,
63+
connection_pool: DieselPool,
64+
uploader: Uploader,
65+
http_client: Client,
5266
) -> Self {
5367
Self {
54-
index: Arc::new(Mutex::new(index)),
68+
index,
5569
connection_pool: AssertUnwindSafe(connection_pool),
5670
uploader,
5771
http_client: AssertUnwindSafe(http_client),

src/bin/background-worker.rs

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use cargo_registry::git::{Repository, RepositoryConfig};
1616
use cargo_registry::{background_jobs::*, db};
1717
use diesel::r2d2;
1818
use reqwest::blocking::Client;
19+
use std::sync::{Arc, Mutex};
1920
use std::thread::sleep;
2021
use std::time::Duration;
2122

@@ -24,15 +25,6 @@ fn main() {
2425

2526
let config = cargo_registry::Config::default();
2627

27-
// 2x the thread pool size -- not all our jobs need a DB connection,
28-
// but we want to always be able to run our jobs in parallel, rather
29-
// than adjusting based on how many concurrent jobs need a connection.
30-
// Eventually swirl will do this for us, and this will be the default
31-
// -- we should just let it do a thread pool size of CPU count, and a
32-
// a connection pool size of 2x that when that lands.
33-
let db_config = r2d2::Pool::builder().max_size(4);
34-
let db_pool = db::diesel_pool(&config.db_url, config.env, db_config);
35-
3628
let job_start_timeout = dotenv::var("BACKGROUND_JOB_TIMEOUT")
3729
.unwrap_or_else(|_| "30".into())
3830
.parse()
@@ -41,13 +33,27 @@ fn main() {
4133
println!("Cloning index");
4234

4335
let repository_config = RepositoryConfig::from_environment();
44-
let repository = Repository::open(&repository_config).expect("Failed to clone index");
36+
let repository = Arc::new(Mutex::new(
37+
Repository::open(&repository_config).expect("Failed to clone index"),
38+
));
4539
println!("Index cloned");
4640

47-
let environment = Environment::new(repository, db_pool.clone(), config.uploader, Client::new());
48-
4941
let build_runner = || {
50-
swirl::Runner::builder(db_pool.clone(), environment.clone())
42+
// 2x the thread pool size -- not all our jobs need a DB connection,
43+
// but we want to always be able to run our jobs in parallel, rather
44+
// than adjusting based on how many concurrent jobs need a connection.
45+
// Eventually swirl will do this for us, and this will be the default
46+
// -- we should just let it do a thread pool size of CPU count, and a
47+
// a connection pool size of 2x that when that lands.
48+
let db_config = r2d2::Pool::builder().max_size(4);
49+
let db_pool = db::diesel_pool(&config.db_url, config.env, db_config);
50+
let environment = Environment::new_shared(
51+
repository.clone(),
52+
db_pool.clone(),
53+
config.uploader.clone(),
54+
Client::new(),
55+
);
56+
swirl::Runner::builder(db_pool, environment)
5157
.thread_count(2)
5258
.job_start_timeout(Duration::from_secs(job_start_timeout))
5359
.build()

0 commit comments

Comments
 (0)