Skip to content

Commit f01fd19

Browse files
committed
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.
1 parent 111b7eb commit f01fd19

File tree

1 file changed

+4
-7
lines changed

1 file changed

+4
-7
lines changed

Diff for: src/controllers/krate/search.rs

+4-7
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,7 @@ pub fn search(req: &mut dyn Request) -> CargoResult<Response> {
3939
let conn = req.db_conn()?;
4040
let (offset, limit) = req.pagination(10, 100)?;
4141
let params = req.query();
42-
let sort = params
43-
.get("sort")
44-
.map(|s| &**s)
45-
.unwrap_or("recent-downloads");
42+
let sort = params.get("sort").map(|s| &**s);
4643
let mut has_filter = false;
4744
let include_yanked = params
4845
.get("include_yanked")
@@ -167,11 +164,11 @@ pub fn search(req: &mut dyn Request) -> CargoResult<Response> {
167164
));
168165
}
169166

170-
if sort == "downloads" {
167+
if sort == Some("downloads") {
171168
query = query.then_order_by(crates::downloads.desc())
172-
} else if sort == "recent-downloads" {
169+
} else if sort == Some("recent-downloads") {
173170
query = query.then_order_by(recent_crate_downloads::downloads.desc().nulls_last())
174-
} else if sort == "recent-updates" {
171+
} else if sort == Some("recent-updates") {
175172
query = query.order(crates::updated_at.desc());
176173
} else {
177174
query = query.then_order_by(crates::name.asc())

0 commit comments

Comments
 (0)