about summary refs log tree commit diff
diff options
context:
space:
mode:
authorAkihiko Odaki <nekomanma@pixiv.co.jp>2020-08-28 16:27:33 +0900
committerGitHub <noreply@github.com>2020-08-28 09:27:33 +0200
commit552e886b648faa2a2229d86c7fd9abc8bb5ff99c (patch)
tree0df097a83922fd4f5f570e388c9e6acdc0da08f0
parenta10f53aa6928d9ece5fa09c67eafaa21a0404b8d (diff)
Eagerly load statuses with the main query in Api::V1::FavouritesController (#14673)
The old implementation had two queries:
1. The query constructed in Api::V1::FavouritesController#results
2. The query constructed in #cached_favourites, which is merged with 1.

Both of them are issued againt PostgreSQL. The combination of the two
queries caused the following problems:
- The small window between the two queries involves race conditions.
- Minor performance inefficiency.

Moreover, the construction of query 2, which involves merging with query
1 has a bug. Query 1 is finalized with paginate_by_id, but paginate_by_id
returns an array when min_id parameter is specified. The behavior prevents
from merging the query, and in the real world, ActiveRecord simply ignores
the merge (!), which results in querying the entire scan of statuses and
favourites table.

This change fixes these issues by simply letting query 1 get all the works
done.
-rw-r--r--app/controllers/api/v1/favourites_controller.rb7
1 files changed, 2 insertions, 5 deletions
diff --git a/app/controllers/api/v1/favourites_controller.rb b/app/controllers/api/v1/favourites_controller.rb
index 3e242905d..71a707d2a 100644
--- a/app/controllers/api/v1/favourites_controller.rb
+++ b/app/controllers/api/v1/favourites_controller.rb
@@ -17,14 +17,11 @@ class Api::V1::FavouritesController < Api::BaseController
   end
 
   def cached_favourites
-    cache_collection(
-      Status.reorder(nil).joins(:favourites).merge(results),
-      Status
-    )
+    cache_collection(results.map(&:status), Status)
   end
 
   def results
-    @_results ||= account_favourites.paginate_by_id(
+    @_results ||= account_favourites.eager_load(:status).paginate_by_id(
       limit_param(DEFAULT_STATUSES_LIMIT),
       params_slice(:max_id, :since_id, :min_id)
     )