Skip to content

Commit

Permalink
move builder temp-dir to prefix to avoid conflicts with second builder
Browse files Browse the repository at this point in the history
  • Loading branch information
syphar committed Oct 11, 2023
1 parent 0fb8970 commit a0d9991
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 67 deletions.
1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ tower-http = { version = "0.4.0", features = ["fs", "trace", "timeout", "catch-p
mime = "0.3.16"
percent-encoding = "2.2.0"

# NOTE: if you change this, also double-check that the comment in `queue_builder::remove_tempdirs` is still accurate.
tempfile = "3.1.0"
fn-error-context = "0.2.0"

Expand Down
3 changes: 2 additions & 1 deletion src/bin/cratesfyi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,9 @@ impl CommandLine {
start_background_metrics_webserver(Some(metric_server_socket_addr), &ctx)?;

let build_queue = ctx.build_queue()?;
let config = ctx.config()?;
let rustwide_builder = RustwideBuilder::init(&ctx)?;
queue_builder(rustwide_builder, build_queue)?;
queue_builder(rustwide_builder, build_queue, config)?;
}
Self::StartWebServer { socket_addr } => {
// Blocks indefinitely
Expand Down
7 changes: 6 additions & 1 deletion src/config.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::{cdn::CdnKind, storage::StorageKind};
use anyhow::{anyhow, bail, Context, Result};
use std::{env::VarError, error::Error, path::PathBuf, str::FromStr, time::Duration};
use std::{env::VarError, error::Error, fs, path::PathBuf, str::FromStr, time::Duration};
use tracing::trace;

#[derive(Debug)]
Expand Down Expand Up @@ -97,6 +97,7 @@ pub struct Config {
// Build params
pub(crate) build_attempts: u16,
pub(crate) rustwide_workspace: PathBuf,
pub(crate) temp_dir: PathBuf,
pub(crate) inside_docker: bool,
pub(crate) docker_image: Option<String>,
pub(crate) build_cpu_limit: Option<u32>,
Expand Down Expand Up @@ -127,6 +128,8 @@ impl Config {
}

let prefix: PathBuf = require_env("DOCSRS_PREFIX")?;
let temp_dir = prefix.join("tmp");
fs::create_dir_all(&temp_dir)?;

Ok(Self {
build_attempts: env("DOCSRS_BUILD_ATTEMPTS", 5)?,
Expand Down Expand Up @@ -196,6 +199,8 @@ impl Config {
prefix.join("archive_cache"),
)?,

temp_dir,

rustwide_workspace: env("DOCSRS_RUSTWIDE_WORKSPACE", PathBuf::from(".workspace"))?,
inside_docker: env("DOCSRS_DOCKER", false)?,
docker_image: maybe_env("DOCSRS_LOCAL_DOCKER_IMAGE")?
Expand Down
8 changes: 3 additions & 5 deletions src/docbuilder/rustwide_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use crate::error::Result;
use crate::repositories::RepositoryStatsUpdater;
use crate::storage::{rustdoc_archive_path, source_archive_path};
use crate::utils::{
copy_dir_all, get_config, parse_rustc_version, queue_builder, report_error, set_config,
CargoMetadata, ConfigName,
copy_dir_all, get_config, parse_rustc_version, report_error, set_config, CargoMetadata,
ConfigName,
};
use crate::RUSTDOC_STATIC_STORAGE_PREFIX;
use crate::{db::blacklist::is_blacklisted, utils::MetadataPackage};
Expand Down Expand Up @@ -407,9 +407,7 @@ impl RustwideBuilder {
};
krate.fetch(&self.workspace).map_err(FailureError::compat)?;

let local_storage = tempfile::Builder::new()
.prefix(queue_builder::TEMPDIR_PREFIX)
.tempdir()?;
let local_storage = tempfile::tempdir_in(&self.config.temp_dir)?;

let successful = build_dir
.build(&self.toolchain, &krate, self.prepare_sandbox(&limits))
Expand Down
3 changes: 2 additions & 1 deletion src/utils/daemon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,12 @@ pub fn start_daemon<C: Context + Send + Sync + 'static>(

// build new crates every minute
let build_queue = context.build_queue()?;
let config = context.config()?;
let rustwide_builder = RustwideBuilder::init(&*context)?;
thread::Builder::new()
.name("build queue reader".to_string())
.spawn(move || {
queue_builder(rustwide_builder, build_queue).unwrap();
queue_builder(rustwide_builder, build_queue, config).unwrap();
})
.unwrap();

Expand Down
69 changes: 11 additions & 58 deletions src/utils/queue_builder.rs
Original file line number Diff line number Diff line change
@@ -1,20 +1,22 @@
use crate::{docbuilder::RustwideBuilder, utils::report_error, BuildQueue};
use crate::{docbuilder::RustwideBuilder, utils::report_error, BuildQueue, Config};
use anyhow::{Context, Error};
use std::panic::{catch_unwind, AssertUnwindSafe};
use std::sync::Arc;
use std::time::Duration;
use std::{fs, io, thread};
use std::{fs, io, path::Path, thread};
use tracing::{debug, error, warn};

pub(crate) const TEMPDIR_PREFIX: &str = "docsrs-docs";

pub fn queue_builder(
mut builder: RustwideBuilder,
build_queue: Arc<BuildQueue>,
config: Arc<Config>,
) -> Result<(), Error> {
loop {
if let Err(e) = remove_tempdirs() {
report_error(&anyhow::anyhow!(e).context("failed to remove temporary directories"));
if let Err(e) = remove_tempdirs(&config.temp_dir) {
report_error(&anyhow::anyhow!(e).context(format!(
"failed to clean temporary directory {:?}",
&config.temp_dir
)));
}

// check lock file
Expand Down Expand Up @@ -58,57 +60,8 @@ pub fn queue_builder(
/// Sometimes, when the server hits a hard crash or a build thread panics,
/// rustwide_builder won't actually remove the temporary directories it creates.
/// Remove them now to avoid running out of disk space.
fn remove_tempdirs() -> Result<(), io::Error> {
// NOTE: hardcodes that `tempfile::tempdir()` uses `std::env::temp_dir`.
for entry in std::fs::read_dir(std::env::temp_dir())? {
let entry = entry?;
if !entry.metadata()?.is_dir() {
continue;
}

if let Some(dir_name) = entry.path().file_name() {
if dir_name.to_string_lossy().starts_with(TEMPDIR_PREFIX) {
fs::remove_dir_all(entry.path())?;
}
}
}

fn remove_tempdirs<P: AsRef<Path>>(path: P) -> Result<(), io::Error> {
fs::remove_dir_all(&path)?;
fs::create_dir_all(&path)?;
Ok(())
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn remove_existing_tempdirs() {
let file_with_prefix = tempfile::Builder::new()
.prefix(TEMPDIR_PREFIX)
.tempfile()
.unwrap();

let dir_with_prefix = tempfile::Builder::new()
.prefix(TEMPDIR_PREFIX)
.tempdir()
.unwrap();

let file_inside = dir_with_prefix.path().join("some_file_name");
fs::File::create(&file_inside).unwrap();

let other_file = tempfile::Builder::new().tempfile().unwrap();

let other_dir = tempfile::Builder::new().tempdir().unwrap();

assert!(dir_with_prefix.path().exists());

remove_tempdirs().unwrap();

assert!(!dir_with_prefix.path().exists());
assert!(!file_inside.exists());

// all these still exist
assert!(file_with_prefix.path().exists());
assert!(other_file.path().exists());
assert!(other_dir.path().exists());
}
}

0 comments on commit a0d9991

Please sign in to comment.