Optimize queries that need to sort on recent_crate_downloads.downloads

migrations/2019-05-29-192622_index_crate_downloads_by_downloads/down.sql

@@ -0,0 +1,8 @@
+DROP MATERIALIZED VIEW recent_crate_downloads;
+CREATE MATERIALIZED VIEW recent_crate_downloads (crate_id, downloads) AS
+  SELECT crate_id, SUM(version_downloads.downloads) FROM version_downloads
+    INNER JOIN versions
+      ON version_downloads.version_id = versions.id
+    WHERE version_downloads.date > date(CURRENT_TIMESTAMP - INTERVAL '90 days')
+    GROUP BY crate_id;
+CREATE UNIQUE INDEX recent_crate_downloads_crate_id ON recent_crate_downloads (crate_id);

migrations/2019-05-29-192622_index_crate_downloads_by_downloads/up.sql

@@ -0,0 +1,10 @@
+DROP MATERIALIZED VIEW recent_crate_downloads;
+CREATE MATERIALIZED VIEW recent_crate_downloads (crate_id, downloads) AS
+  SELECT crate_id, COALESCE(SUM(version_downloads.downloads), 0) FROM versions
+    LEFT JOIN version_downloads
+      ON version_downloads.version_id = versions.id
+    WHERE version_downloads.date > date(CURRENT_TIMESTAMP - INTERVAL '90 days')
+      OR version_downloads.date IS NULL
+    GROUP BY crate_id;
+CREATE UNIQUE INDEX recent_crate_downloads_crate_id ON recent_crate_downloads (crate_id);
+CREATE INDEX ON recent_crate_downloads (downloads);

src/controllers/krate/search.rs

@@ -51,7 +51,7 @@ pub fn search(req: &mut dyn Request) -> CargoResult<Response> {
         recent_crate_downloads::downloads.nullable(),
     );
     let mut query = crates::table
-        .left_join(recent_crate_downloads::table)
+        .inner_join(recent_crate_downloads::table)
         .select(selection)
         .into_boxed();
 
@@ -157,7 +157,7 @@ pub fn search(req: &mut dyn Request) -> CargoResult<Response> {
     if sort == "downloads" {
         query = query.then_order_by(crates::downloads.desc())
     } else if sort == "recent-downloads" {
-        query = query.then_order_by(recent_crate_downloads::downloads.desc().nulls_last())
+        query = query.then_order_by(recent_crate_downloads::downloads.desc())
     } else if sort == "recent-updates" {
         query = query.order(crates::updated_at.desc());
     } else {
@@ -177,7 +177,7 @@ pub fn search(req: &mut dyn Request) -> CargoResult<Response> {
                 // FIXME: Use `query.selection()` if that feature ends up in
                 // Diesel 2.0
                 selection,
-                coalesce(crates::table.count().single_value(), 0),
+                coalesce(recent_crate_downloads::table.count().single_value(), 0),
             ))
             .limit(limit)
             .offset(offset)

src/router.rs

@@ -13,7 +13,7 @@ pub fn build_router(app: &App) -> R404 {
     let mut api_router = RouteBuilder::new();
 
     // Route used by both `cargo search` and the frontend
-    api_router.get("/crates", C(krate::search::search));
+    api_router.get("/crates", C(needs_recent_downloads(krate::search::search)));
 
     // Routes used by `cargo`
     api_router.put("/crates/new", C(krate::publish::publish));
@@ -35,7 +35,10 @@ pub fn build_router(app: &App) -> R404 {
     api_router.get("/versions/:version_id", C(version::deprecated::show_by_id));
 
     // Routes used by the frontend
-    api_router.get("/crates/:crate_id", C(krate::metadata::show));
+    api_router.get(
+        "/crates/:crate_id",
+        C(needs_recent_downloads(krate::metadata::show)),
+    );
     api_router.get("/crates/:crate_id/:version", C(version::metadata::show));
     api_router.get(
         "/crates/:crate_id/:version/readme",
@@ -89,7 +92,10 @@ pub fn build_router(app: &App) -> R404 {
         "/me/crate_owner_invitations/:crate_id",
         C(crate_owner_invitation::handle_invite),
     );
-    api_router.get("/summary", C(krate::metadata::summary));
+    api_router.get(
+        "/summary",
+        C(needs_recent_downloads(krate::metadata::summary)),
+    );
     api_router.put("/confirm/:email_token", C(user::me::confirm_user_email));
     api_router.put(
         "/users/:user_id/resend",
@@ -125,12 +131,14 @@ pub fn build_router(app: &App) -> R404 {
     R404(router)
 }
 
-struct C(pub fn(&mut dyn Request) -> CargoResult<Response>);
+struct C<F: Fn(&mut dyn Request) -> CargoResult<Response> + Send + Sync + 'static>(F);
 
-impl Handler for C {
+impl<F> Handler for C<F>
+where
+    F: Fn(&mut dyn Request) -> CargoResult<Response> + Send + Sync + 'static,
+{
     fn call(&self, req: &mut dyn Request) -> Result<Response, Box<dyn Error + Send>> {
-        let C(f) = *self;
-        match f(req) {
+        match self.0(req) {
             Ok(resp) => Ok(resp),
             Err(e) => match e.response() {
                 Some(response) => Ok(response),
@@ -154,6 +162,26 @@ impl<H: Handler> Handler for R<H> {
     }
 }
 
+fn needs_recent_downloads(
+    f: fn(&mut dyn Request) -> CargoResult<Response>,
+) -> impl Fn(&mut dyn Request) -> CargoResult<Response> {
+    use crate::db::RequestTransaction;
+    use crate::middleware::app::RequestApp;
+    use diesel::prelude::*;
+
+    move |req| {
+        let app = req.app();
+        if app.config.env == Env::Test {
+            let conn = req.db_conn()?;
+            let _ = conn.transaction(|| {
+                no_arg_sql_function!(refresh_recent_crate_downloads, ());
+                diesel::select(refresh_recent_crate_downloads).execute(&*conn)
+            });
+        }
+        f(req)
+    }
+}
+
 // Can't derive Debug because of RouteBuilder.
 #[allow(missing_debug_implementations)]
 pub struct R404(pub RouteBuilder);

src/tests/builders.rs

@@ -230,7 +230,7 @@ impl<'a> CrateBuilder<'a> {
     }
 
     fn build(mut self, connection: &PgConnection) -> CargoResult<Crate> {
-        use diesel::{insert_into, select, update};
+        use diesel::{insert_into, update};
 
         let mut krate = self
             .krate
@@ -264,9 +264,6 @@ impl<'a> CrateBuilder<'a> {
                     version_downloads::downloads.eq(downloads),
                 ))
                 .execute(connection)?;
-
-            no_arg_sql_function!(refresh_recent_crate_downloads, ());
-            select(refresh_recent_crate_downloads).execute(connection)?;
         }
 
         if !self.keywords.is_empty() {

src/tests/krate.rs

@@ -248,7 +248,7 @@ fn index_sorting() {
         let krate2 = CrateBuilder::new("bar_sort", user.id)
             .description("foo_sort baz_sort foo_sort baz_sort const")
             .downloads(3333)
-            .recent_downloads(0)
+            .recent_downloads(1)
             .expect_build(conn);
 
         let krate3 = CrateBuilder::new("baz_sort", user.id)
@@ -331,7 +331,7 @@ fn exact_match_on_queries_with_sort() {
         let krate2 = CrateBuilder::new("bar_sort", user.id)
             .description("foo_sort baz_sort foo_sort baz_sort const")
             .downloads(3333)
-            .recent_downloads(0)
+            .recent_downloads(1)
             .expect_build(conn);
 
         let krate3 = CrateBuilder::new("baz_sort", user.id)