Skip to content

Commit

Permalink
Add a timeout to cache lookups
Browse files Browse the repository at this point in the history
This should help ensure that we don't wait *too* long for the cache to respond
(for example on an excessively slow network) and time out the server
unnecessarily.
  • Loading branch information
alexcrichton committed Mar 13, 2017
1 parent ca2f8fc commit baf9610
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 12 deletions.
61 changes: 50 additions & 11 deletions src/compiler/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ use std::time::{
Instant,
};
use tempdir::TempDir;
use tokio_core::reactor::{Handle, Timeout};

use errors::*;

Expand Down Expand Up @@ -174,6 +175,8 @@ pub enum MissType {
Normal,
/// Cache lookup was overridden, recompilation was forced.
ForcedRecache,
/// Cache took too long to respond.
TimedOut,
}

/// Information about a successful cache write.
Expand Down Expand Up @@ -298,7 +301,8 @@ impl Compiler {
parsed_args: &ParsedArguments,
cwd: &str,
cache_control: CacheControl,
pool: &CpuPool)
pool: &CpuPool,
handle: &Handle)
-> SFuture<(CompileResult, process::Output)>
where T: CommandCreatorSync
{
Expand All @@ -319,6 +323,7 @@ impl Compiler {
let storage = storage.clone();
let pool = pool.clone();
let creator = creator.clone();
let handle = handle.clone();

Box::new(result.and_then(move |preprocessor_result| -> SFuture<_> {
// If the preprocessor failed, just return that result.
Expand Down Expand Up @@ -355,6 +360,20 @@ impl Compiler {
storage.get(&key)
};

// Wait at most a minute for the cache to respond before we forge
// ahead ourselves with a compilation.
let timeout = Duration::new(60, 0);
let timeout = Timeout::new(timeout, &handle).into_future().flatten();

let cache_status = cache_status.map(Some);
let timeout = timeout.map(|_| None).chain_err(|| "timeout error");
let cache_status = cache_status.select(timeout).then(|r| {
match r {
Ok((a, _other)) => Ok(a),
Err((e, _other)) => Err(e),
}
});

Box::new(cache_status.and_then(move |result| {
let duration = start.elapsed();
let pwd = Path::new(&cwd);
Expand All @@ -363,7 +382,7 @@ impl Compiler {
.collect::<HashMap<_, _>>();

let miss_type = match result {
Cache::Hit(mut entry) => {
Some(Cache::Hit(mut entry)) => {
debug!("[{}]: Cache hit!", parsed_args.output_file());
let mut stdout = io::Cursor::new(vec!());
let mut stderr = io::Cursor::new(vec!());
Expand All @@ -386,14 +405,18 @@ impl Compiler {
(result, output)
})) as SFuture<_>
}
Cache::Miss => {
Some(Cache::Miss) => {
debug!("[{}]: Cache miss!", parsed_args.output_file());
MissType::Normal
}
Cache::Recache => {
Some(Cache::Recache) => {
debug!("[{}]: Cache recache!", parsed_args.output_file());
MissType::ForcedRecache
}
None => {
debug!("[{}]: Cache timed out!", parsed_args.output_file());
MissType::TimedOut
}
};
me.compile(&creator,
preprocessor_result,
Expand Down Expand Up @@ -662,6 +685,7 @@ mod test {
use std::time::Duration;
use std::usize;
use test::utils::*;
use tokio_core::reactor::Core;

#[test]
fn test_detect_compiler_kind_gcc() {
Expand Down Expand Up @@ -742,6 +766,8 @@ mod test {
let creator = new_creator();
let f = TestFixture::new();
let pool = CpuPool::new(1);
let core = Core::new().unwrap();
let handle = core.handle();
let storage = DiskCache::new(&f.tempdir.path().join("cache"),
usize::MAX,
&pool);
Expand Down Expand Up @@ -778,7 +804,8 @@ mod test {
&parsed_args,
cwd,
CacheControl::Default,
&pool).wait().unwrap();
&pool,
&handle).wait().unwrap();
// Ensure that the object file was created.
assert_eq!(true, fs::metadata(&obj).and_then(|m| Ok(m.len() > 0)).unwrap());
match cached {
Expand All @@ -802,7 +829,8 @@ mod test {
&parsed_args,
cwd,
CacheControl::Default,
&pool).wait().unwrap();
&pool,
&handle).wait().unwrap();
// Ensure that the object file was created.
assert_eq!(true, fs::metadata(&obj).and_then(|m| Ok(m.len() > 0)).unwrap());
assert_eq!(CompileResult::CacheHit(Duration::new(0, 0)), cached);
Expand All @@ -818,6 +846,8 @@ mod test {
let creator = new_creator();
let f = TestFixture::new();
let pool = CpuPool::new(1);
let core = Core::new().unwrap();
let handle = core.handle();
let storage = DiskCache::new(&f.tempdir.path().join("cache"),
usize::MAX,
&pool);
Expand Down Expand Up @@ -854,7 +884,8 @@ mod test {
&parsed_args,
cwd,
CacheControl::Default,
&pool).wait().unwrap();
&pool,
&handle).wait().unwrap();
// Ensure that the object file was created.
assert_eq!(true, fs::metadata(&obj).and_then(|m| Ok(m.len() > 0)).unwrap());
match cached {
Expand All @@ -879,7 +910,8 @@ mod test {
&parsed_args,
cwd,
CacheControl::Default,
&pool).wait().unwrap();
&pool,
&handle).wait().unwrap();
// Ensure that the object file was created.
assert_eq!(true, fs::metadata(&obj).and_then(|m| Ok(m.len() > 0)).unwrap());
assert_eq!(CompileResult::CacheHit(Duration::new(0, 0)), cached);
Expand All @@ -895,6 +927,8 @@ mod test {
let creator = new_creator();
let f = TestFixture::new();
let pool = CpuPool::new(1);
let core = Core::new().unwrap();
let handle = core.handle();
let storage = DiskCache::new(&f.tempdir.path().join("cache"),
usize::MAX,
&pool);
Expand Down Expand Up @@ -935,7 +969,8 @@ mod test {
&parsed_args,
cwd,
CacheControl::Default,
&pool).wait().unwrap();
&pool,
&handle).wait().unwrap();
// Ensure that the object file was created.
assert_eq!(true, fs::metadata(&obj).and_then(|m| Ok(m.len() > 0)).unwrap());
match cached {
Expand All @@ -956,7 +991,8 @@ mod test {
&parsed_args,
cwd,
CacheControl::ForceRecache,
&pool).wait().unwrap();
&pool,
&handle).wait().unwrap();
// Ensure that the object file was created.
assert_eq!(true, fs::metadata(&obj).and_then(|m| Ok(m.len() > 0)).unwrap());
match cached {
Expand All @@ -978,6 +1014,8 @@ mod test {
let creator = new_creator();
let f = TestFixture::new();
let pool = CpuPool::new(1);
let core = Core::new().unwrap();
let handle = core.handle();
let storage = DiskCache::new(&f.tempdir.path().join("cache"),
usize::MAX,
&pool);
Expand All @@ -1002,7 +1040,8 @@ mod test {
&parsed_args,
cwd,
CacheControl::Default,
&pool).wait().unwrap();
&pool,
&handle).wait().unwrap();
assert_eq!(cached, CompileResult::Error);
assert_eq!(exit_status(1), res.status);
// Shouldn't get anything on stdout, since that would just be preprocessor spew!
Expand Down
6 changes: 5 additions & 1 deletion src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,8 @@ impl<C> SccacheService<C>
&parsed_arguments,
&cwd,
cache_control,
&self.pool);
&self.pool,
&self.handle);
let me = self.clone();
let task = result.then(move |result| {
let mut res = ServerResponse::new();
Expand All @@ -575,6 +576,9 @@ impl<C> SccacheService<C>
stats.cache_misses += 1;
stats.forced_recaches += 1;
}
MissType::TimedOut => {
stats.cache_misses += 1;
}
}
stats.cache_read_miss_duration += duration;
cache_write = Some(future);
Expand Down

0 comments on commit baf9610

Please sign in to comment.