Fix performance of home feed regeneration (#12084)

Fetching statuses from all followed accounts at once takes too long
within Postgres. Fetching them one by one and merging in Ruby
could be a lot less resource-intensive

Because the query for dynamically fetching the home timeline is so
heavy, we can no longer offer it when the home timeline is missing
This commit is contained in:
Eugen Rochko 2019-10-06 22:11:17 +02:00 committed by GitHub
parent efda126914
commit f665901e3c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 134 additions and 143 deletions

View file

@ -13,7 +13,7 @@ class Api::V1::Timelines::HomeController < Api::BaseController
render json: @statuses, render json: @statuses,
each_serializer: REST::StatusSerializer, each_serializer: REST::StatusSerializer,
relationships: StatusRelationshipsPresenter.new(@statuses, current_user&.account_id), relationships: StatusRelationshipsPresenter.new(@statuses, current_user&.account_id),
status: regeneration_in_progress? ? 206 : 200 status: account_home_feed.regenerating? ? 206 : 200
end end
private private
@ -62,8 +62,4 @@ class Api::V1::Timelines::HomeController < Api::BaseController
def pagination_since_id def pagination_since_id
@statuses.first.id @statuses.first.id
end end
def regeneration_in_progress?
Redis.current.exists("account:#{current_account.id}:regeneration")
end
end end

View file

@ -97,7 +97,7 @@ export function expandTimeline(timelineId, path, params = {}, done = noOp) {
api(getState).get(path, { params }).then(response => { api(getState).get(path, { params }).then(response => {
const next = getLinks(response).refs.find(link => link.rel === 'next'); const next = getLinks(response).refs.find(link => link.rel === 'next');
dispatch(importFetchedStatuses(response.data)); dispatch(importFetchedStatuses(response.data));
dispatch(expandTimelineSuccess(timelineId, response.data, next ? next.uri : null, response.code === 206, isLoadingRecent, isLoadingMore, isLoadingRecent && preferPendingItems)); dispatch(expandTimelineSuccess(timelineId, response.data, next ? next.uri : null, response.status === 206, isLoadingRecent, isLoadingMore, isLoadingRecent && preferPendingItems));
done(); done();
}).catch(error => { }).catch(error => {
dispatch(expandTimelineFail(timelineId, error, isLoadingMore)); dispatch(expandTimelineFail(timelineId, error, isLoadingMore));

View file

@ -1,17 +1,24 @@
import React from 'react'; import React from 'react';
import PropTypes from 'prop-types';
import { FormattedMessage } from 'react-intl'; import { FormattedMessage } from 'react-intl';
import illustration from 'mastodon/../images/elephant_ui_disappointed.svg';
import classNames from 'classnames';
const MissingIndicator = () => ( const MissingIndicator = ({ fullPage }) => (
<div className='regeneration-indicator missing-indicator'> <div className={classNames('regeneration-indicator', { 'regeneration-indicator--without-header': fullPage })}>
<div> <div className='regeneration-indicator__figure'>
<div className='regeneration-indicator__figure' /> <img src={illustration} alt='' />
</div>
<div className='regeneration-indicator__label'> <div className='regeneration-indicator__label'>
<FormattedMessage id='missing_indicator.label' tagName='strong' defaultMessage='Not found' /> <FormattedMessage id='missing_indicator.label' tagName='strong' defaultMessage='Not found' />
<FormattedMessage id='missing_indicator.sublabel' defaultMessage='This resource could not be found' /> <FormattedMessage id='missing_indicator.sublabel' defaultMessage='This resource could not be found' />
</div>
</div> </div>
</div> </div>
); );
MissingIndicator.propTypes = {
fullPage: PropTypes.bool,
};
export default MissingIndicator; export default MissingIndicator;

View file

@ -0,0 +1,18 @@
import React from 'react';
import { FormattedMessage } from 'react-intl';
import illustration from 'mastodon/../images/elephant_ui_working.svg';
const MissingIndicator = () => (
<div className='regeneration-indicator'>
<div className='regeneration-indicator__figure'>
<img src={illustration} alt='' />
</div>
<div className='regeneration-indicator__label'>
<FormattedMessage id='regeneration_indicator.label' tagName='strong' defaultMessage='Loading&hellip;' />
<FormattedMessage id='regeneration_indicator.sublabel' defaultMessage='Your home feed is being prepared!' />
</div>
</div>
);
export default MissingIndicator;

View file

@ -1,12 +1,12 @@
import { debounce } from 'lodash'; import { debounce } from 'lodash';
import React from 'react'; import React from 'react';
import { FormattedMessage } from 'react-intl';
import ImmutablePropTypes from 'react-immutable-proptypes'; import ImmutablePropTypes from 'react-immutable-proptypes';
import PropTypes from 'prop-types'; import PropTypes from 'prop-types';
import StatusContainer from '../containers/status_container'; import StatusContainer from '../containers/status_container';
import ImmutablePureComponent from 'react-immutable-pure-component'; import ImmutablePureComponent from 'react-immutable-pure-component';
import LoadGap from './load_gap'; import LoadGap from './load_gap';
import ScrollableList from './scrollable_list'; import ScrollableList from './scrollable_list';
import RegenerationIndicator from 'mastodon/components/regeneration_indicator';
export default class StatusList extends ImmutablePureComponent { export default class StatusList extends ImmutablePureComponent {
@ -81,18 +81,7 @@ export default class StatusList extends ImmutablePureComponent {
const { isLoading, isPartial } = other; const { isLoading, isPartial } = other;
if (isPartial) { if (isPartial) {
return ( return <RegenerationIndicator />;
<div className='regeneration-indicator'>
<div>
<div className='regeneration-indicator__figure' />
<div className='regeneration-indicator__label'>
<FormattedMessage id='regeneration_indicator.label' tagName='strong' defaultMessage='Loading&hellip;' />
<FormattedMessage id='regeneration_indicator.sublabel' defaultMessage='Your home feed is being prepared!' />
</div>
</div>
</div>
);
} }
let scrollableContent = (isLoading || statusIds.size > 0) ? ( let scrollableContent = (isLoading || statusIds.size > 0) ? (

View file

@ -4,7 +4,7 @@ import MissingIndicator from '../../components/missing_indicator';
const GenericNotFound = () => ( const GenericNotFound = () => (
<Column> <Column>
<MissingIndicator /> <MissingIndicator fullPage />
</Column> </Column>
); );

View file

@ -3127,37 +3127,27 @@ a.status-card.compact:hover {
cursor: default; cursor: default;
display: flex; display: flex;
flex: 1 1 auto; flex: 1 1 auto;
flex-direction: column;
align-items: center; align-items: center;
justify-content: center; justify-content: center;
padding: 20px; padding: 20px;
& > div {
width: 100%;
background: transparent;
padding-top: 0;
}
&__figure { &__figure {
background: url('../images/elephant_ui_working.svg') no-repeat center 0; &,
width: 100%; img {
height: 160px; display: block;
background-size: contain; width: auto;
position: absolute; height: 160px;
top: 50%; margin: 0;
left: 50%;
transform: translate(-50%, -50%);
}
&.missing-indicator {
padding-top: 20px + 48px;
.regeneration-indicator__figure {
background-image: url('../images/elephant_ui_disappointed.svg');
} }
} }
&--without-header {
padding-top: 20px + 48px;
}
&__label { &__label {
margin-top: 200px; margin-top: 30px;
strong { strong {
display: block; display: block;

View file

@ -3,9 +3,10 @@
flex-direction: column; flex-direction: column;
justify-content: center; justify-content: center;
align-items: center; align-items: center;
height: 100vh;
background: $ui-base-color;
@media screen and (max-width: 920px) { @media screen and (max-width: 920px) {
background: darken($ui-base-color, 8%);
display: block !important; display: block !important;
} }

View file

@ -19,7 +19,7 @@ class FeedManager
def filter?(timeline_type, status, receiver_id) def filter?(timeline_type, status, receiver_id)
if timeline_type == :home if timeline_type == :home
filter_from_home?(status, receiver_id) filter_from_home?(status, receiver_id, build_crutches(receiver_id, [status]))
elsif timeline_type == :mentions elsif timeline_type == :mentions
filter_from_mentions?(status, receiver_id) filter_from_mentions?(status, receiver_id)
else else
@ -29,6 +29,7 @@ class FeedManager
def push_to_home(account, status) def push_to_home(account, status)
return false unless add_to_feed(:home, account.id, status, account.user&.aggregates_reblogs?) return false unless add_to_feed(:home, account.id, status, account.user&.aggregates_reblogs?)
trim(:home, account.id) trim(:home, account.id)
PushUpdateWorker.perform_async(account.id, status.id, "timeline:#{account.id}") if push_update_required?("timeline:#{account.id}") PushUpdateWorker.perform_async(account.id, status.id, "timeline:#{account.id}") if push_update_required?("timeline:#{account.id}")
true true
@ -36,6 +37,7 @@ class FeedManager
def unpush_from_home(account, status) def unpush_from_home(account, status)
return false unless remove_from_feed(:home, account.id, status, account.user&.aggregates_reblogs?) return false unless remove_from_feed(:home, account.id, status, account.user&.aggregates_reblogs?)
redis.publish("timeline:#{account.id}", Oj.dump(event: :delete, payload: status.id.to_s)) redis.publish("timeline:#{account.id}", Oj.dump(event: :delete, payload: status.id.to_s))
true true
end end
@ -46,7 +48,9 @@ class FeedManager
should_filter &&= !ListAccount.where(list_id: list.id, account_id: status.in_reply_to_account_id).exists? should_filter &&= !ListAccount.where(list_id: list.id, account_id: status.in_reply_to_account_id).exists?
return false if should_filter return false if should_filter
end end
return false unless add_to_feed(:list, list.id, status, list.account.user&.aggregates_reblogs?) return false unless add_to_feed(:list, list.id, status, list.account.user&.aggregates_reblogs?)
trim(:list, list.id) trim(:list, list.id)
PushUpdateWorker.perform_async(list.account_id, status.id, "timeline:list:#{list.id}") if push_update_required?("timeline:list:#{list.id}") PushUpdateWorker.perform_async(list.account_id, status.id, "timeline:list:#{list.id}") if push_update_required?("timeline:list:#{list.id}")
true true
@ -54,6 +58,7 @@ class FeedManager
def unpush_from_list(list, status) def unpush_from_list(list, status)
return false unless remove_from_feed(:list, list.id, status, list.account.user&.aggregates_reblogs?) return false unless remove_from_feed(:list, list.id, status, list.account.user&.aggregates_reblogs?)
redis.publish("timeline:list:#{list.id}", Oj.dump(event: :delete, payload: status.id.to_s)) redis.publish("timeline:list:#{list.id}", Oj.dump(event: :delete, payload: status.id.to_s))
true true
end end
@ -85,16 +90,21 @@ class FeedManager
def merge_into_timeline(from_account, into_account) def merge_into_timeline(from_account, into_account)
timeline_key = key(:home, into_account.id) timeline_key = key(:home, into_account.id)
query = from_account.statuses.limit(FeedManager::MAX_ITEMS / 4) aggregate = into_account.user&.aggregates_reblogs?
query = from_account.statuses.where(visibility: [:public, :unlisted, :private]).includes(:preloadable_poll, reblog: :account).limit(FeedManager::MAX_ITEMS / 4)
if redis.zcard(timeline_key) >= FeedManager::MAX_ITEMS / 4 if redis.zcard(timeline_key) >= FeedManager::MAX_ITEMS / 4
oldest_home_score = redis.zrange(timeline_key, 0, 0, with_scores: true)&.first&.last&.to_i || 0 oldest_home_score = redis.zrange(timeline_key, 0, 0, with_scores: true).first.last.to_i
query = query.where('id > ?', oldest_home_score) query = query.where('id > ?', oldest_home_score)
end end
query.each do |status| statuses = query.to_a
next if status.direct_visibility? || status.limited_visibility? || filter?(:home, status, into_account) crutches = build_crutches(into_account.id, statuses)
add_to_feed(:home, into_account.id, status, into_account.user&.aggregates_reblogs?)
statuses.each do |status|
next if filter_from_home?(status, into_account, crutches)
add_to_feed(:home, into_account.id, status, aggregate)
end end
trim(:home, into_account.id) trim(:home, into_account.id)
@ -120,24 +130,35 @@ class FeedManager
end end
def populate_feed(account) def populate_feed(account)
added = 0 limit = FeedManager::MAX_ITEMS / 2
limit = FeedManager::MAX_ITEMS / 2 aggregate = account.user&.aggregates_reblogs?
max_id = nil timeline_key = key(:home, account.id)
loop do account.statuses.where.not(visibility: :direct).limit(limit).each do |status|
statuses = Status.as_home_timeline(account) add_to_feed(:home, account.id, status, aggregate)
.paginate_by_max_id(limit, max_id) end
break if statuses.empty? account.following.includes(:account_stat).find_each do |target_account|
if redis.zcard(timeline_key) >= limit
oldest_home_score = redis.zrange(timeline_key, 0, 0, with_scores: true).first.last.to_i
last_status_score = Mastodon::Snowflake.id_at(account.last_status_at)
statuses.each do |status| # If the feed is full and this account has not posted more recently
next if filter_from_home?(status, account) # than the last item on the feed, then we can skip the whole account
added += 1 if add_to_feed(:home, account.id, status, account.user&.aggregates_reblogs?) # because none of its statuses would stay on the feed anyway
next if last_status_score < oldest_home_score
end end
break unless added.zero? statuses = target_account.statuses.where(visibility: [:public, :unlisted, :private]).includes(:preloadable_poll, reblog: :account).limit(limit)
crutches = build_crutches(account.id, statuses)
max_id = statuses.last.id statuses.each do |status|
next if filter_from_home?(status, account, crutches)
add_to_feed(:home, account.id, status, aggregate)
end
trim(:home, account.id)
end end
end end
@ -152,31 +173,33 @@ class FeedManager
(context == :home ? Mute.where(account_id: receiver_id, target_account_id: account_ids).any? : Mute.where(account_id: receiver_id, target_account_id: account_ids, hide_notifications: true).any?) (context == :home ? Mute.where(account_id: receiver_id, target_account_id: account_ids).any? : Mute.where(account_id: receiver_id, target_account_id: account_ids, hide_notifications: true).any?)
end end
def filter_from_home?(status, receiver_id) def filter_from_home?(status, receiver_id, crutches)
return false if receiver_id == status.account_id return false if receiver_id == status.account_id
return true if status.reply? && (status.in_reply_to_id.nil? || status.in_reply_to_account_id.nil?) return true if status.reply? && (status.in_reply_to_id.nil? || status.in_reply_to_account_id.nil?)
return true if phrase_filtered?(status, receiver_id, :home) return true if phrase_filtered?(status, receiver_id, :home)
check_for_blocks = status.active_mentions.pluck(:account_id) check_for_blocks = crutches[:active_mentions][status.id] || []
check_for_blocks.concat([status.account_id]) check_for_blocks.concat([status.account_id])
if status.reblog? if status.reblog?
check_for_blocks.concat([status.reblog.account_id]) check_for_blocks.concat([status.reblog.account_id])
check_for_blocks.concat(status.reblog.active_mentions.pluck(:account_id)) check_for_blocks.concat(crutches[:active_mentions][status.reblog_of_id] || [])
end end
return true if blocks_or_mutes?(receiver_id, check_for_blocks, :home) return true if check_for_blocks.any? { |target_account_id| crutches[:blocking][target_account_id] || crutches[:muting][target_account_id] }
if status.reply? && !status.in_reply_to_account_id.nil? # Filter out if it's a reply if status.reply? && !status.in_reply_to_account_id.nil? # Filter out if it's a reply
should_filter = !Follow.where(account_id: receiver_id, target_account_id: status.in_reply_to_account_id).exists? # and I'm not following the person it's a reply to should_filter = !crutches[:following][status.in_reply_to_account_id] # and I'm not following the person it's a reply to
should_filter &&= receiver_id != status.in_reply_to_account_id # and it's not a reply to me should_filter &&= receiver_id != status.in_reply_to_account_id # and it's not a reply to me
should_filter &&= status.account_id != status.in_reply_to_account_id # and it's not a self-reply should_filter &&= status.account_id != status.in_reply_to_account_id # and it's not a self-reply
return should_filter
return !!should_filter
elsif status.reblog? # Filter out a reblog elsif status.reblog? # Filter out a reblog
should_filter = Follow.where(account_id: receiver_id, target_account_id: status.account_id, show_reblogs: false).exists? # if the reblogger's reblogs are suppressed should_filter = crutches[:hiding_reblogs][status.account_id] # if the reblogger's reblogs are suppressed
should_filter ||= Block.where(account_id: status.reblog.account_id, target_account_id: receiver_id).exists? # or if the author of the reblogged status is blocking me should_filter ||= crutches[:blocked_by][status.reblog.account_id] # or if the author of the reblogged status is blocking me
should_filter ||= AccountDomainBlock.where(account_id: receiver_id, domain: status.reblog.account.domain).exists? # or the author's domain is blocked should_filter ||= crutches[:domain_blocking][status.reblog.account.domain] # or the author's domain is blocked
return should_filter
return !!should_filter
end end
false false
@ -308,4 +331,31 @@ class FeedManager
redis.zrem(timeline_key, status.id) redis.zrem(timeline_key, status.id)
end end
def build_crutches(receiver_id, statuses)
crutches = {}
crutches[:active_mentions] = Mention.active.where(status_id: statuses.flat_map { |s| [s.id, s.reblog_of_id] }.compact).pluck(:status_id, :account_id).each_with_object({}) { |(id, account_id), mapping| (mapping[id] ||= []).push(account_id) }
check_for_blocks = statuses.flat_map do |s|
arr = crutches[:active_mentions][s.id] || []
arr.concat([s.account_id])
if s.reblog?
arr.concat([s.reblog.account_id])
arr.concat(crutches[:active_mentions][s.reblog_of_id] || [])
end
arr
end
crutches[:following] = Follow.where(account_id: receiver_id, target_account_id: statuses.map(&:in_reply_to_account_id).compact).pluck(:target_account_id).each_with_object({}) { |id, mapping| mapping[id] = true }
crutches[:hiding_reblogs] = Follow.where(account_id: receiver_id, target_account_id: statuses.map { |s| s.account_id if s.reblog? }.compact, show_reblogs: false).pluck(:target_account_id).each_with_object({}) { |id, mapping| mapping[id] = true }
crutches[:blocking] = Block.where(account_id: receiver_id, target_account_id: check_for_blocks).pluck(:target_account_id).each_with_object({}) { |id, mapping| mapping[id] = true }
crutches[:muting] = Mute.where(account_id: receiver_id, target_account_id: check_for_blocks).pluck(:target_account_id).each_with_object({}) { |id, mapping| mapping[id] = true }
crutches[:domain_blocking] = AccountDomainBlock.where(account_id: receiver_id, domain: statuses.map { |s| s.reblog&.account&.domain }.compact).pluck(:domain).each_with_object({}) { |domain, mapping| mapping[domain] = true }
crutches[:blocked_by] = Block.where(target_account_id: receiver_id, account_id: statuses.map { |s| s.reblog&.account_id }.compact).pluck(:account_id).each_with_object({}) { |id, mapping| mapping[id] = true }
crutches
end
end end

View file

@ -7,19 +7,7 @@ class HomeFeed < Feed
@account = account @account = account
end end
def get(limit, max_id = nil, since_id = nil, min_id = nil) def regenerating?
if redis.exists("account:#{@account.id}:regeneration") redis.exists("account:#{@id}:regeneration")
from_database(limit, max_id, since_id, min_id)
else
super
end
end
private
def from_database(limit, max_id, since_id, min_id)
Status.as_home_timeline(@account)
.paginate_by_id(limit, max_id: max_id, since_id: since_id, min_id: min_id)
.reject { |status| FeedManager.instance.filter?(:home, status, @account.id) }
end end
end end

View file

@ -282,10 +282,6 @@ class Status < ApplicationRecord
where(language: nil).or where(language: account.chosen_languages) where(language: nil).or where(language: account.chosen_languages)
end end
def as_home_timeline(account)
where(account: [account] + account.following).where(visibility: [:public, :unlisted, :private])
end
def as_public_timeline(account = nil, local_only = false) def as_public_timeline(account = nil, local_only = false)
query = timeline_scope(local_only).without_replies query = timeline_scope(local_only).without_replies

View file

@ -34,11 +34,10 @@ RSpec.describe HomeFeed, type: :model do
Redis.current.set("account:#{account.id}:regeneration", true) Redis.current.set("account:#{account.id}:regeneration", true)
end end
it 'gets statuses with ids in the range from database' do it 'returns nothing' do
results = subject.get(3) results = subject.get(3)
expect(results.map(&:id)).to eq [10, 3, 2] expect(results.map(&:id)).to eq []
expect(results.first.attributes.keys).to include('id', 'updated_at')
end end
end end
end end

View file

@ -296,49 +296,6 @@ RSpec.describe Status, type: :model do
end end
end end
describe '.as_home_timeline' do
let(:account) { Fabricate(:account) }
let(:followed) { Fabricate(:account) }
let(:not_followed) { Fabricate(:account) }
before do
Fabricate(:follow, account: account, target_account: followed)
@self_status = Fabricate(:status, account: account, visibility: :public)
@self_direct_status = Fabricate(:status, account: account, visibility: :direct)
@followed_status = Fabricate(:status, account: followed, visibility: :public)
@followed_direct_status = Fabricate(:status, account: followed, visibility: :direct)
@not_followed_status = Fabricate(:status, account: not_followed, visibility: :public)
@results = Status.as_home_timeline(account)
end
it 'includes statuses from self' do
expect(@results).to include(@self_status)
end
it 'does not include direct statuses from self' do
expect(@results).to_not include(@self_direct_status)
end
it 'includes statuses from followed' do
expect(@results).to include(@followed_status)
end
it 'does not include direct statuses mentioning recipient from followed' do
Fabricate(:mention, account: account, status: @followed_direct_status)
expect(@results).to_not include(@followed_direct_status)
end
it 'does not include direct statuses not mentioning recipient from followed' do
expect(@results).not_to include(@followed_direct_status)
end
it 'does not include statuses from non-followed' do
expect(@results).not_to include(@not_followed_status)
end
end
describe '.as_public_timeline' do describe '.as_public_timeline' do
it 'only includes statuses with public visibility' do it 'only includes statuses with public visibility' do
public_status = Fabricate(:status, visibility: :public) public_status = Fabricate(:status, visibility: :public)