From 81caafbe84240bfb730f5001b33add97ebcbe9dc Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 7 Sep 2023 18:55:25 +0200 Subject: [PATCH] Fix performances of profile directory (#26842) --- app/models/account.rb | 2 +- ..._stats_on_last_status_at_and_account_id.rb | 9 +++++++++ db/schema.rb | 3 ++- .../api/v1/directories_controller_spec.rb | 19 ++++++++++++++----- 4 files changed, 26 insertions(+), 7 deletions(-) create mode 100644 db/migrate/20230907150100_add_index_account_stats_on_last_status_at_and_account_id.rb diff --git a/app/models/account.rb b/app/models/account.rb index 40b9bba0c..679093a59 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -125,7 +125,7 @@ class Account < ApplicationRecord scope :matches_domain, ->(value) { where(arel_table[:domain].matches("%#{value}%")) } scope :without_unapproved, -> { left_outer_joins(:user).merge(User.approved.confirmed).or(remote) } scope :searchable, -> { without_unapproved.without_suspended.where(moved_to_account_id: nil) } - scope :discoverable, -> { searchable.without_silenced.where(discoverable: true).left_outer_joins(:account_stat) } + scope :discoverable, -> { searchable.without_silenced.where(discoverable: true).joins(:account_stat) } scope :followable_by, ->(account) { joins(arel_table.join(Follow.arel_table, Arel::Nodes::OuterJoin).on(arel_table[:id].eq(Follow.arel_table[:target_account_id]).and(Follow.arel_table[:account_id].eq(account.id))).join_sources).where(Follow.arel_table[:id].eq(nil)).joins(arel_table.join(FollowRequest.arel_table, Arel::Nodes::OuterJoin).on(arel_table[:id].eq(FollowRequest.arel_table[:target_account_id]).and(FollowRequest.arel_table[:account_id].eq(account.id))).join_sources).where(FollowRequest.arel_table[:id].eq(nil)) } scope :by_recent_status, -> { order(Arel.sql('account_stats.last_status_at DESC NULLS LAST')) } scope :by_recent_sign_in, -> { order(Arel.sql('users.current_sign_in_at DESC NULLS LAST')) } diff --git a/db/migrate/20230907150100_add_index_account_stats_on_last_status_at_and_account_id.rb b/db/migrate/20230907150100_add_index_account_stats_on_last_status_at_and_account_id.rb new file mode 100644 index 000000000..17ac65547 --- /dev/null +++ b/db/migrate/20230907150100_add_index_account_stats_on_last_status_at_and_account_id.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddIndexAccountStatsOnLastStatusAtAndAccountId < ActiveRecord::Migration[7.0] + disable_ddl_transaction! + + def change + add_index :account_stats, [:last_status_at, :account_id], order: { last_status_at: 'DESC NULLS LAST' }, algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index 21eff86bf..37020c2d7 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_09_04_134623) do +ActiveRecord::Schema[7.0].define(version: 2023_09_07_150100) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -99,6 +99,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_09_04_134623) do t.datetime "updated_at", precision: nil, null: false t.datetime "last_status_at", precision: nil t.index ["account_id"], name: "index_account_stats_on_account_id", unique: true + t.index ["last_status_at", "account_id"], name: "index_account_stats_on_last_status_at_and_account_id", order: { last_status_at: "DESC NULLS LAST" } end create_table "account_statuses_cleanup_policies", force: :cascade do |t| diff --git a/spec/controllers/api/v1/directories_controller_spec.rb b/spec/controllers/api/v1/directories_controller_spec.rb index dddbf40e4..308a8874c 100644 --- a/spec/controllers/api/v1/directories_controller_spec.rb +++ b/spec/controllers/api/v1/directories_controller_spec.rb @@ -15,12 +15,13 @@ describe Api::V1::DirectoriesController do describe 'GET #show' do context 'with no params' do before do - _local_unconfirmed_account = Fabricate( + local_unconfirmed_account = Fabricate( :account, domain: nil, user: Fabricate(:user, confirmed_at: nil, approved: true), username: 'local_unconfirmed' ) + local_unconfirmed_account.create_account_stat! local_unapproved_account = Fabricate( :account, @@ -28,15 +29,17 @@ describe Api::V1::DirectoriesController do user: Fabricate(:user, confirmed_at: 10.days.ago), username: 'local_unapproved' ) + local_unapproved_account.create_account_stat! local_unapproved_account.user.update(approved: false) - _local_undiscoverable_account = Fabricate( + local_undiscoverable_account = Fabricate( :account, domain: nil, user: Fabricate(:user, confirmed_at: 10.days.ago, approved: true), discoverable: false, username: 'local_undiscoverable' ) + local_undiscoverable_account.create_account_stat! excluded_from_timeline_account = Fabricate( :account, @@ -44,14 +47,16 @@ describe Api::V1::DirectoriesController do discoverable: true, username: 'remote_excluded_from_timeline' ) + excluded_from_timeline_account.create_account_stat! Fabricate(:block, account: user.account, target_account: excluded_from_timeline_account) - _domain_blocked_account = Fabricate( + domain_blocked_account = Fabricate( :account, domain: 'test.example', discoverable: true, username: 'remote_domain_blocked' ) + domain_blocked_account.create_account_stat! Fabricate(:account_domain_block, account: user.account, domain: 'test.example') end @@ -63,6 +68,7 @@ describe Api::V1::DirectoriesController do discoverable: true, username: 'local_discoverable' ) + local_discoverable_account.create_account_stat! eligible_remote_account = Fabricate( :account, @@ -70,6 +76,7 @@ describe Api::V1::DirectoriesController do discoverable: true, username: 'eligible_remote' ) + eligible_remote_account.create_account_stat! get :show @@ -84,6 +91,8 @@ describe Api::V1::DirectoriesController do user = Fabricate(:user, confirmed_at: 10.days.ago, approved: true) local_account = Fabricate(:account, domain: nil, user: user) remote_account = Fabricate(:account, domain: 'host.example') + local_account.create_account_stat! + remote_account.create_account_stat! get :show, params: { local: '1' } @@ -110,9 +119,9 @@ describe Api::V1::DirectoriesController do context 'when ordered by new' do it 'returns accounts in order of creation' do - account_old = Fabricate(:account) + account_old = Fabricate(:account_stat).account travel_to 10.seconds.from_now - account_new = Fabricate(:account) + account_new = Fabricate(:account_stat).account get :show, params: { order: 'new' }