-
Notifications
You must be signed in to change notification settings - Fork 210
add "priority" to the build queue, and decouple builds from reading new crates #344
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
QuietMisdreavus
merged 7 commits into
rust-lang:master
from
QuietMisdreavus:build-queue-priority
May 4, 2019
+223
−91
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
11b587c
run "load new crates" and "build crates from queue" in different threads
QuietMisdreavus 19e31e3
add priority column to build queue
QuietMisdreavus 5360c51
add `cratesfyi queue add` command
QuietMisdreavus fd0b92f
rebuild existing crates if they appear on the queue
QuietMisdreavus b99a0ac
remove the old build_packages_queue
QuietMisdreavus d0c4083
cycle caches at 10 builds, not 11
QuietMisdreavus 94cf10f
don't ping hubs if we just refreshed our caches
QuietMisdreavus File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
|
||
use std::{env, thread}; | ||
use std::panic::{catch_unwind, AssertUnwindSafe}; | ||
use std::process::exit; | ||
use std::fs::File; | ||
use std::io::Write; | ||
|
@@ -44,80 +45,159 @@ pub fn start_daemon() { | |
exit(0); | ||
} | ||
|
||
|
||
// check new crates every minute | ||
thread::spawn(move || { | ||
// space this out to prevent it from clashing against the queue-builder thread on launch | ||
thread::sleep(Duration::from_secs(30)); | ||
loop { | ||
let opts = opts(); | ||
let mut doc_builder = DocBuilder::new(opts); | ||
|
||
debug!("Checking new crates"); | ||
match doc_builder.get_new_crates() { | ||
Ok(n) => debug!("{} crates added to queue", n), | ||
Err(e) => error!("Failed to get new crates: {}", e), | ||
} | ||
|
||
thread::sleep(Duration::from_secs(60)); | ||
} | ||
}); | ||
|
||
// build new crates every minute | ||
thread::spawn(move || { | ||
let opts = opts(); | ||
let mut doc_builder = DocBuilder::new(opts); | ||
|
||
/// Represents the current state of the builder thread. | ||
enum BuilderState { | ||
/// The builder thread has just started, and hasn't built any crates yet. | ||
Fresh, | ||
/// The builder has just seen an empty build queue. | ||
EmptyQueue, | ||
/// The builder has just seen the lock file. | ||
Locked, | ||
/// The builder has just finished building a crate. The enclosed count is the number of | ||
/// crates built since the caches have been refreshed. | ||
QueueInProgress(usize), | ||
} | ||
|
||
let mut opts = opts(); | ||
opts.skip_if_exists = true; | ||
let mut status = BuilderState::Fresh; | ||
|
||
loop { | ||
if !status.is_in_progress() { | ||
thread::sleep(Duration::from_secs(60)); | ||
} | ||
|
||
// check lock file | ||
if opts.prefix.join("cratesfyi.lock").exists() { | ||
if doc_builder.is_locked() { | ||
warn!("Lock file exits, skipping building new crates"); | ||
status = BuilderState::Locked; | ||
continue; | ||
} | ||
|
||
let mut doc_builder = DocBuilder::new(opts); | ||
if status.count() >= 10 { | ||
// periodically, we need to flush our caches and ping the hubs | ||
debug!("10 builds in a row; flushing caches"); | ||
status = BuilderState::QueueInProgress(0); | ||
|
||
debug!("Checking new crates"); | ||
let queue_count = match doc_builder.get_new_crates() { | ||
Ok(size) => size, | ||
Err(e) => { | ||
error!("Failed to get new crates: {}", e); | ||
continue; | ||
match pubsubhubbub::ping_hubs() { | ||
Err(e) => error!("Failed to ping hub: {}", e), | ||
Ok(n) => debug!("Succesfully pinged {} hubs", n) | ||
} | ||
}; | ||
|
||
// Only build crates if there is any | ||
if queue_count == 0 { | ||
debug!("Queue is empty, going back to sleep"); | ||
continue; | ||
} | ||
if let Err(e) = doc_builder.load_cache() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This load/save dance is here in case the on-disk cache has been updated since the builder thread has last saved it. Since it's represented in-memory as a |
||
error!("Failed to load cache: {}", e); | ||
} | ||
|
||
info!("Building {} crates from queue", queue_count); | ||
if let Err(e) = doc_builder.save_cache() { | ||
error!("Failed to save cache: {}", e); | ||
} | ||
|
||
// update index | ||
if let Err(e) = update_sources() { | ||
error!("Failed to update sources: {}", e); | ||
continue; | ||
if let Err(e) = update_sources() { | ||
error!("Failed to update sources: {}", e); | ||
continue; | ||
} | ||
} | ||
|
||
if let Err(e) = doc_builder.load_cache() { | ||
error!("Failed to load cache: {}", e); | ||
continue; | ||
// Only build crates if there are any to build | ||
debug!("Checking build queue"); | ||
match doc_builder.get_queue_count() { | ||
Err(e) => { | ||
error!("Failed to read the number of crates in the queue: {}", e); | ||
continue; | ||
} | ||
Ok(0) => { | ||
if status.count() > 0 { | ||
// ping the hubs before continuing | ||
match pubsubhubbub::ping_hubs() { | ||
Err(e) => error!("Failed to ping hub: {}", e), | ||
Ok(n) => debug!("Succesfully pinged {} hubs", n) | ||
} | ||
|
||
if let Err(e) = doc_builder.save_cache() { | ||
error!("Failed to save cache: {}", e); | ||
} | ||
} | ||
debug!("Queue is empty, going back to sleep"); | ||
status = BuilderState::EmptyQueue; | ||
continue; | ||
} | ||
Ok(queue_count) => { | ||
info!("Starting build with {} crates in queue (currently on a {} crate streak)", | ||
queue_count, status.count()); | ||
} | ||
} | ||
|
||
// if we're starting a new batch, reload our caches and sources | ||
if !status.is_in_progress() { | ||
if let Err(e) = doc_builder.load_cache() { | ||
error!("Failed to load cache: {}", e); | ||
continue; | ||
} | ||
|
||
if let Err(e) = update_sources() { | ||
error!("Failed to update sources: {}", e); | ||
continue; | ||
} | ||
} | ||
|
||
// Run build_packages_queue in it's own thread to catch panics | ||
// Run build_packages_queue under `catch_unwind` to catch panics | ||
// This only panicked twice in the last 6 months but its just a better | ||
// idea to do this. | ||
let res = thread::spawn(move || { | ||
match doc_builder.build_packages_queue() { | ||
Err(e) => error!("Failed build new crates: {}", e), | ||
Ok(n) => { | ||
if n > 0 { | ||
match pubsubhubbub::ping_hubs() { | ||
Err(e) => error!("Failed to ping hub: {}", e), | ||
Ok(n) => debug!("Succesfully pinged {} hubs", n) | ||
} | ||
} | ||
} | ||
} | ||
|
||
if let Err(e) = doc_builder.save_cache() { | ||
error!("Failed to save cache: {}", e); | ||
let res = catch_unwind(AssertUnwindSafe(|| { | ||
match doc_builder.build_next_queue_package() { | ||
Err(e) => error!("Failed to build crate from queue: {}", e), | ||
Ok(crate_built) => if crate_built { | ||
status.increment(); | ||
} | ||
|
||
debug!("Finished building new crates, going back to sleep"); | ||
}) | ||
.join(); | ||
} | ||
})); | ||
|
||
if let Err(e) = res { | ||
error!("GRAVE ERROR Building new crates panicked: {:?}", e); | ||
} | ||
} | ||
|
||
impl BuilderState { | ||
fn count(&self) -> usize { | ||
match *self { | ||
BuilderState::QueueInProgress(n) => n, | ||
_ => 0, | ||
} | ||
} | ||
|
||
fn is_in_progress(&self) -> bool { | ||
match *self { | ||
BuilderState::QueueInProgress(_) => true, | ||
_ => false, | ||
} | ||
} | ||
|
||
fn increment(&mut self) { | ||
*self = BuilderState::QueueInProgress(self.count() + 1); | ||
} | ||
} | ||
}); | ||
|
||
|
||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
//! Utilities for interacting with the build queue | ||
use postgres::Connection; | ||
use error::Result; | ||
|
||
pub fn add_crate_to_queue(conn: &Connection, name: &str, version: &str, priority: i32) -> Result<()> { | ||
try!(conn.execute("INSERT INTO queue (name, version, priority) VALUES ($1, $2, $3)", | ||
&[&name, &version, &priority])); | ||
Ok(()) | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When i initially wrote this enum, i had thought that the logic would be more complicated than it wound up being. This can probably be reduced to an
Option<usize>
, but i left this state enum in to make the usage code easier to read.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update: I tried to reduce this to a
usize
and then to anOption<usize>
, and proceeded to introduce bugs into the implementation. I think i'll keep this state enum as-is, since it provides a more readable implementation, IMO. (It'll take up the same space on the stack as anOption<usize>
regardless.)