Skip to content

(WIP) rustdoc redirect caching in the CDN #1262

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

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,17 @@ pub struct Config {
// `500` for a ratio of 7249 over 54k crates.
// For unit-tests the number has to be higher.
pub(crate) random_crate_search_view_size: u32,
// CDN / caching settings
pub(crate) cache_rustdoc_redirects: u32,

// Time to cache crate-level redirects in rustdoc, in seconds.
// This is for redirects where the destination
// can change after the release of a new version
// of a crate.
pub(crate) cache_rustdoc_redirects_crate: u32,

// Time to cache release-level redirects in rustdoc, in seconds.
// Here the destination can only change after
// rebuilds or yanks, so very infrequently.
pub(crate) cache_rustdoc_redirects_version: u32,

// Build params
pub(crate) build_attempts: u16,
Expand Down Expand Up @@ -96,7 +105,11 @@ impl Config {
registry_gc_interval: env("DOCSRS_REGISTRY_GC_INTERVAL", 60 * 60)?,

random_crate_search_view_size: env("DOCSRS_RANDOM_CRATE_SEARCH_VIEW_SIZE", 500)?,
cache_rustdoc_redirects: env("DOCSRS_CACHE_RUSTDOC_REDIRECTS", 30 * 60)?,
cache_rustdoc_redirects_crate: env("DOCSRS_CACHE_RUSTDOC_REDIRECTS_CRATE", 15 * 60)?, // 15 minutes
cache_rustdoc_redirects_version: env(
"DOCSRS_CACHE_RUSTDOC_REDIRECTS_RELEASE",
7 * 24 * 60 * 60, // 7 days
)?,

rustwide_workspace: env("CRATESFYI_RUSTWIDE_WORKSPACE", PathBuf::from(".workspace"))?,
inside_docker: env("DOCS_RS_DOCKER", false)?,
Expand Down
2 changes: 1 addition & 1 deletion src/web/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ fn redirect(url: Url) -> Response {
resp
}

/// creates a redirect-response which is cached on the CDN level for
/// Creates a redirect-response which is cached on the CDN level for
/// the given amount of seconds. Browser-Local caching is deactivated.
/// The used s-maxage is respected by CloudFront (which we can invalidate
/// if we need to), and perhaps by other proxies in between.
Expand Down
45 changes: 35 additions & 10 deletions src/web/rustdoc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ impl iron::Handler for RustLangRedirector {
let config = extension!(req, Config);
Ok(super::cached_redirect(
self.url.clone(),
config.cache_rustdoc_redirects,
config.cache_rustdoc_redirects_crate,
))
}
}
Expand All @@ -54,6 +54,7 @@ pub fn rustdoc_redirector_handler(req: &mut Request) -> IronResult<Response> {
vers: &str,
target: Option<&str>,
target_name: &str,
cache_seconds: u32,
) -> IronResult<Response> {
let mut url_str = if let Some(target) = target {
format!(
Expand All @@ -72,21 +73,25 @@ pub fn rustdoc_redirector_handler(req: &mut Request) -> IronResult<Response> {
url_str.push_str(query);
}
let url = ctry!(req, Url::parse(&url_str));
let config = extension!(req, Config);

Ok(super::cached_redirect(url, config.cache_rustdoc_redirects))
Ok(super::cached_redirect(url, cache_seconds))
}

fn redirect_to_crate(req: &Request, name: &str, vers: &str) -> IronResult<Response> {
fn redirect_to_crate(
req: &Request,
name: &str,
vers: &str,
cache_seconds: u32,
) -> IronResult<Response> {
let url = ctry!(
req,
Url::parse(&format!("{}/crate/{}/{}", redirect_base(req), name, vers)),
);

let config = extension!(req, Config);
Ok(super::cached_redirect(url, config.cache_rustdoc_redirects))
Ok(super::cached_redirect(url, cache_seconds))
}

let config = extension!(req, Config);
let metrics = extension!(req, Metrics).clone();
let mut rendering_time = RenderingTimesRecorder::new(&metrics.rustdoc_redirect_rendering_times);

Expand Down Expand Up @@ -145,6 +150,13 @@ pub fn rustdoc_redirector_handler(req: &mut Request) -> IronResult<Response> {
let req_version = router.find("version");
let mut target = router.find("target");

// redirects with an existing version in the URL can be cached for longer.
let cache_redirect_for = if let Some(_) = req_version {
config.cache_rustdoc_redirects_version
} else {
config.cache_rustdoc_redirects_crate
};

// it doesn't matter if the version that was given was exact or not, since we're redirecting
// anyway
rendering_time.step("match version");
Expand Down Expand Up @@ -179,10 +191,17 @@ pub fn rustdoc_redirector_handler(req: &mut Request) -> IronResult<Response> {

if has_docs {
rendering_time.step("redirect to doc");
redirect_to_doc(req, &crate_name, &version, target, &target_name)
redirect_to_doc(
req,
&crate_name,
&version,
target,
&target_name,
cache_redirect_for,
)
} else {
rendering_time.step("redirect to crate");
redirect_to_crate(req, &crate_name, &version)
redirect_to_crate(req, &crate_name, &version, cache_redirect_for)
}
}

Expand Down Expand Up @@ -282,7 +301,10 @@ pub fn rustdoc_html_server_handler(req: &mut Request) -> IronResult<Response> {
);
let url = ctry!(req, Url::parse(&redirect_path));

Ok(super::cached_redirect(url, config.cache_rustdoc_redirects))
Ok(super::cached_redirect(
url,
config.cache_rustdoc_redirects_version,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not always correct, for MatchVersion::Semver it should be cache_crate, not cache_version. Can you take this as a parameter instead?

Maybe we should consider changing the name of the parameter too 🤔 I think cache_release might be easier to think about than cache_version.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jyn514 I was already working on exactly this :) stumbled on the same thing.

Parameter I'll rename.

))
};

rendering_time.step("match version");
Expand Down Expand Up @@ -551,7 +573,10 @@ pub fn target_redirect_handler(req: &mut Request) -> IronResult<Response> {
);

let url = ctry!(req, Url::parse(&url));
Ok(super::cached_redirect(url, config.cache_rustdoc_redirects))
Ok(super::cached_redirect(
url,
config.cache_rustdoc_redirects_version,
))
}

pub fn badge_handler(req: &mut Request) -> IronResult<Response> {
Expand Down