Skip to content

Commit

Permalink
Eagerly load statuses with the main query in Api::V1::FavouritesContr…
Browse files Browse the repository at this point in the history
…oller (mastodon#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.
  • Loading branch information
akihikodaki authored Aug 28, 2020
1 parent a10f53a commit 552e886
Showing 1 changed file with 2 additions and 5 deletions.
7 changes: 2 additions & 5 deletions app/controllers/api/v1/favourites_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,11 @@ def load_statuses
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)
)
Expand Down

0 comments on commit 552e886

Please sign in to comment.