Skip to content

Commit d8a7e76

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 f73b7a7 commit d8a7e76

File tree

1 file changed

+4
-5
lines changed

1 file changed

+4
-5
lines changed

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

+4-5
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,7 @@ pub fn search(req: &mut dyn Request) -> CargoResult<Response> {
4141
let params = req.query();
4242
let sort = params
4343
.get("sort")
44-
.map(|s| &**s)
45-
.unwrap_or("recent-downloads");
44+
.map(|s| &**s);
4645
let mut has_filter = false;
4746
let include_yanked = params
4847
.get("include_yanked")
@@ -167,11 +166,11 @@ pub fn search(req: &mut dyn Request) -> CargoResult<Response> {
167166
));
168167
}
169168

170-
if sort == "downloads" {
169+
if sort == Some("downloads") {
171170
query = query.then_order_by(crates::downloads.desc())
172-
} else if sort == "recent-downloads" {
171+
} else if sort == Some("recent-downloads") {
173172
query = query.then_order_by(recent_crate_downloads::downloads.desc().nulls_last())
174-
} else if sort == "recent-updates" {
173+
} else if sort == Some("recent-updates") {
175174
query = query.order(crates::updated_at.desc());
176175
} else {
177176
query = query.then_order_by(crates::name.asc())

0 commit comments

Comments
 (0)