Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize follower_accounts and following_accounts #2820

Merged
merged 1 commit into from May 6, 2017

Conversation

Projects
None yet
3 participants
@alpaca-tc
Copy link
Contributor

alpaca-tc commented May 5, 2017

Example for /users/:acocunt_name/followers of the account with 20000 followers.
This slow queries are frequent in pawoo.net and mstdn.jp

It's normal that higher offsets with INNER JOIN slow the query down.
This optimization is finding records without INNER JOIN.

before after
2017-05-06 2 14 32 2017-05-06 2 15 23
@Gargron

This comment has been minimized.

Copy link
Member

Gargron commented May 5, 2017

Shouldn't the API benefit from this as well?

@alpaca-tc

This comment has been minimized.

Copy link
Contributor Author

alpaca-tc commented May 5, 2017

I just checked the api. It seems that /api/v1/accounts/:id/followers and /api/v1/accounts/:id/following don't use INNER JOIN 😄
Or, did you say about other api?

@mjankowski

This comment has been minimized.

Copy link
Collaborator

mjankowski commented May 5, 2017

can you EXPLAIN the "before" and "after" queries ... it'd be interesting to see how much of the slowness is from which parts

@@ -4,6 +4,6 @@ class FollowerAccountsController < ApplicationController
include AccountControllerConcern

def index
@accounts = @account.followers.page(params[:page]).per(FOLLOW_PER_PAGE)
@follows = Follow.where(target_account: @account).order(id: :desc).page(params[:page]).per(FOLLOW_PER_PAGE).preload(:account)

This comment has been minimized.

Copy link
@mjankowski

mjankowski May 5, 2017

Collaborator

What about moving the query Follow.where(...) into an ordered_follows method or something, and then just have @follows = ordered_follows.page(...) in the action.

@@ -6,4 +6,4 @@

= render 'accounts/header', account: @account

= render 'accounts/follow_grid', accounts: @accounts
= render 'accounts/follow_grid', follows: @follows, accounts: @follows.map(&:account)

This comment has been minimized.

Copy link
@mjankowski

mjankowski May 5, 2017

Collaborator

I think instead of passing accounts into the partial, we can change the partial to check for follows.empty? and then have it run the map on follows before it passes along again to grid_card?

@Gargron

Gargron approved these changes May 6, 2017

@Gargron

This comment has been minimized.

Copy link
Member

Gargron commented May 6, 2017

@alpaca-tc You are right, the API controller is already optimized (I just forgot)

@Gargron Gargron merged commit ddc34fe into tootsuite:master May 6, 2017

2 checks passed

codeclimate no new or fixed issues
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@alpaca-tc alpaca-tc deleted the alpaca-tc:optimize_accounts_page branch May 6, 2017

@alpaca-tc

This comment has been minimized.

Copy link
Contributor Author

alpaca-tc commented May 6, 2017

@mjankowski

can you EXPLAIN the "before" and "after" queries ... it'd be interesting to see how much of the slowness is from which parts

Okay 😄

before

EXPLAIN ANALYZE SELECT  "accounts".* FROM "accounts" INNER JOIN "follows" ON "accounts"."id" = "follows"."account_id" WHERE "follows"."target_account_id" = XXXXX ORDER BY follows.id desc LIMIT 12 OFFSET 5988

Limit  (cost=89433.25..89433.28 rows=12 width=899) (actual time=176.333..176.343 rows=12 loops=1)
  ->  Sort  (cost=89418.28..89448.10 rows=11927 width=899) (actual time=171.567..175.390 rows=6000 loops=1)
        Sort Key: follows.id DESC
        Sort Method: external merge  Disk: 9808kB
        ->  Nested Loop  (cost=341.29..83878.71 rows=11927 width=899) (actual time=3.068..130.017 rows=10821 loops=1)
              ->  Bitmap Heap Scan on follows  (cost=340.86..29327.56 rows=11927 width=8) (actual time=3.045..36.295 rows=10821 loops=1)
                    Recheck Cond: (target_account_id = XXXXX)
                    Heap Blocks: exact=7758
                    ->  Bitmap Index Scan on index_follows_on_target_account_id  (cost=0.00..337.88 rows=11927 width=0) (actual time=1.873..1.873 rows=10835 loops=1)
                          Index Cond: (target_account_id = XXXXX)
              ->  Index Scan using accounts_pkey on accounts  (cost=0.42..4.56 rows=1 width=895) (actual time=0.006..0.007 rows=1 loops=10821)
                    Index Cond: (id = follows.account_id)
Planning time: 1.088 ms
Execution time: 178.454 ms

after

EXPLAIN ANALYZE SELECT  "follows".* FROM "follows" WHERE "follows"."target_account_id" = XXXXX ORDER BY "follows"."id" DESC LIMIT 12 OFFSET 5988

Limit  (cost=30150.10..30150.13 rows=12 width=28) (actual time=20.347..20.353 rows=12 loops=1)
  ->  Sort  (cost=30135.13..30164.95 rows=11927 width=28) (actual time=18.214..19.371 rows=6000 loops=1)
        Sort Key: id DESC
        Sort Method: quicksort  Memory: 1230kB
        ->  Bitmap Heap Scan on follows  (cost=340.86..29327.56 rows=11927 width=28) (actual time=2.725..13.812 rows=10821 loops=1)
              Recheck Cond: (target_account_id = XXXXX)
              Heap Blocks: exact=7758
              ->  Bitmap Index Scan on index_follows_on_target_account_id  (cost=0.00..337.88 rows=11927 width=0) (actual time=1.473..1.473 rows=10835 loops=1)
                    Index Cond: (target_account_id = XXXXX)
Planning time: 0.064 ms
Execution time: 20.484 ms

By the way, this PR has already merged, should I refactor? 🤔

@mjankowski

This comment has been minimized.

Copy link
Collaborator

mjankowski commented May 6, 2017

If you want to refactor in a followup PR go ahead, but not really needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.