From 75f1b01c6638a1219c38ee5f27c47718b36bf6a9 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Mon, 19 Aug 2019 12:32:07 -0600 Subject: [PATCH] Change the default sorting on search to something less expensive This changes the behavior of `/api/v1/crates` if no sort option is provided. This does not change the behavior of the website when accessed through a browser, as the Ember frontend always specifies a sort field. The query run by search when sorting by recent downloads currently accounts for nearly 40% of our total DB load. It takes ~60ms on average, which means it's extremely high load. Our logs for traffic to this endpoint with an explicit `sort=recent-downloads` don't match up with this. We can fix this by making the query faster, but this has drawbacks that were determined not to be worth it (see #1755). So if we can't decrease execution time, the only other option is to decrease execution count. A significant number of crawlers hit us without specifying any sorting, which should mean they don't care about ordering. I'm sure there's someone out there who is relying on this behavior, but most crawlers I've seen don't specify ordering, and none of them care about the order they get the results in since they're crawling the whole registry anyway. What's important is that this lowers the execution time from ~60ms to ~12ms. And sorting on recent_downloads will grow linearly with the table size, while this can be done on an index alone (which should be log(log(n)) IIRC). >90% of the query is spent on getting the count, which will be going away soon as we change pagination strategies, bringing us to sub-millisecond execution. I do feel strongly that we either need to make this change, or accept the drawbacks in #1755, as this will soon become a scaling issue for us. --- src/controllers/krate/search.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/controllers/krate/search.rs b/src/controllers/krate/search.rs index 552cb7d6ef..d2c100e451 100644 --- a/src/controllers/krate/search.rs +++ b/src/controllers/krate/search.rs @@ -37,10 +37,7 @@ pub fn search(req: &mut dyn Request) -> CargoResult { let conn = req.db_conn()?; let params = req.query(); - let sort = params - .get("sort") - .map(|s| &**s) - .unwrap_or("recent-downloads"); + let sort = params.get("sort").map(|s| &**s); let include_yanked = params .get("include_yanked") .map(|s| s == "yes") @@ -156,11 +153,11 @@ pub fn search(req: &mut dyn Request) -> CargoResult { )); } - if sort == "downloads" { + if sort == Some("downloads") { query = query.then_order_by(crates::downloads.desc()) - } else if sort == "recent-downloads" { + } else if sort == Some("recent-downloads") { query = query.then_order_by(recent_crate_downloads::downloads.desc().nulls_last()) - } else if sort == "recent-updates" { + } else if sort == Some("recent-updates") { query = query.order(crates::updated_at.desc()); } else { query = query.then_order_by(crates::name.asc())