Skip to content
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

Specify server timeout #2246

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
2 changes: 1 addition & 1 deletion src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -609,7 +609,7 @@ pub fn run_command(cmd: Command) -> Result<i32> {
// Config isn't required for all commands, but if it's broken then we should flag
// it early and loudly.
let config = &Config::load()?;
let startup_timeout = config.server_startup_timeout;
let startup_timeout = config.server_timing.startup_timeout;

match cmd {
Command::ShowStats(fmt, advanced) => {
Expand Down
70 changes: 66 additions & 4 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -555,9 +555,18 @@ impl Default for DistConfig {
pub struct FileConfig {
pub cache: CacheConfigs,
pub dist: DistConfig,
// pub timing: ServerTimingConfig,
pub server_startup_timeout_ms: Option<u64>,
pub server_shutdown_timeout_ms: Option<u64>,
pub port: Option<u16>,
}

// #[derive(Debug, Default, Serialize, Deserialize, Eq, PartialEq)]
// pub struct ServerTimingConfig {
// pub server_startup_timeout_ms: Option<u64>,
// pub server_shutdown_timeout_ms: Option<u64>,
// }

// If the file doesn't exist or we can't read it, log the issue and proceed. If the
// config exists but doesn't parse then something is wrong - return an error.
pub fn try_read_config_file<T: DeserializeOwned>(path: &Path) -> Result<Option<T>> {
Expand Down Expand Up @@ -945,7 +954,14 @@ pub struct Config {
pub cache: Option<CacheType>,
pub fallback_cache: DiskCacheConfig,
pub dist: DistConfig,
pub server_startup_timeout: Option<std::time::Duration>,
pub server_timing: ServerTiming,
pub port: Option<u16>,
}

#[derive(Debug, Default, PartialEq, Eq)]
pub struct ServerTiming {
pub startup_timeout: Option<std::time::Duration>,
pub shutdown_timeout: Option<std::time::Duration>,
}

impl Config {
Expand All @@ -967,11 +983,19 @@ impl Config {
cache,
dist,
server_startup_timeout_ms,
server_shutdown_timeout_ms,
port,
} = file_conf;
conf_caches.merge(cache);

let server_startup_timeout =
server_startup_timeout_ms.map(std::time::Duration::from_millis);
let server_shutdown_timeout =
server_shutdown_timeout_ms.map(std::time::Duration::from_millis);
let server_timing = ServerTiming {
startup_timeout: server_startup_timeout,
shutdown_timeout: server_shutdown_timeout,
};

let EnvConfig { cache } = env_conf;
conf_caches.merge(cache);
Expand All @@ -981,7 +1005,8 @@ impl Config {
cache: caches,
fallback_cache,
dist,
server_startup_timeout,
server_timing,
port,
}
}
}
Expand Down Expand Up @@ -1281,6 +1306,8 @@ fn config_overrides() {
},
dist: Default::default(),
server_startup_timeout_ms: None,
server_shutdown_timeout_ms: None,
port: None,
};

assert_eq!(
Expand All @@ -1302,7 +1329,8 @@ fn config_overrides() {
rw_mode: CacheModeConfig::ReadWrite,
},
dist: Default::default(),
server_startup_timeout: None,
server_timing: Default::default(),
port: None,
}
);
}
Expand Down Expand Up @@ -1577,7 +1605,41 @@ no_credentials = true
toolchain_cache_size: 5368709120,
rewrite_includes_only: false,
},
server_startup_timeout_ms: Some(10000),
server_startup_timeout_ms: Some(10_000),
server_shutdown_timeout_ms: None,
port: None,
}
)
}

#[test]
fn test_port_config() {
const CONFIG_STR: &str = "port = 8080";
let file_config: FileConfig = toml::from_str(CONFIG_STR).expect("Is valid toml.");
assert_eq!(
file_config,
FileConfig {
cache: Default::default(),
dist: Default::default(),
server_startup_timeout_ms: None,
server_shutdown_timeout_ms: None,
port: Some(8080),
}
)
}

#[test]
fn test_shutdown_config() {
const CONFIG_STR: &str = "server_shutdown_timeout_ms = 10000";
let file_config: FileConfig = toml::from_str(CONFIG_STR).expect("Is valid toml.");
assert_eq!(
file_config,
FileConfig {
cache: Default::default(),
dist: Default::default(),
server_startup_timeout_ms: None,
server_shutdown_timeout_ms: Some(10_000),
port: None,
}
)
}
10 changes: 7 additions & 3 deletions src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -658,11 +658,15 @@ impl<C: CommandCreatorSync> SccacheServer<C> {
}
})?;

const SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(10);
let config = Config::load().unwrap_or_default();
let shutdown_timeout: Duration = config
.server_timing
.shutdown_timeout
.unwrap_or(Duration::from_secs(10));
info!(
"moving into the shutdown phase now, waiting at most {} seconds \
for all client requests to complete",
SHUTDOWN_TIMEOUT.as_secs()
shutdown_timeout.as_secs()
);

// Once our server has shut down either due to inactivity or a manual
Expand All @@ -672,7 +676,7 @@ impl<C: CommandCreatorSync> SccacheServer<C> {
//
// Note that we cap the amount of time this can take, however, as we
// don't want to wait *too* long.
runtime.block_on(async { time::timeout(SHUTDOWN_TIMEOUT, wait).await })?;
runtime.block_on(async { time::timeout(shutdown_timeout, wait).await })?;

info!("ok, fully shutting down now");

Expand Down
2 changes: 2 additions & 0 deletions tests/harness/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,8 @@ pub fn sccache_client_cfg(
rewrite_includes_only: false, // TODO
},
server_startup_timeout_ms: None,
server_shutdown_timeout_ms: None,
port: None,
}
}

Expand Down
2 changes: 2 additions & 0 deletions tests/oauth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ fn config_with_dist_auth(
rewrite_includes_only: true,
},
server_startup_timeout_ms: None,
server_shutdown_timeout_ms: None,
port: None,
}
}

Expand Down