Revert support from loading Home timeline from database

Unfortunately, the database query could turn out very inefficient and I did not
manage to find a way to improve that. Furthermore, there were still behavior
inconsistencies between fetching the timeline from Redis and fetching it from
Postgres.
This commit is contained in:
Claire 2022-04-19 14:54:43 +02:00
parent ec4a8d81b1
commit 5c808ee0de
2 changed files with 13 additions and 111 deletions

View file

@ -9,52 +9,4 @@ class HomeFeed < Feed
def regenerating? def regenerating?
redis.exists?("account:#{@account.id}:regeneration") redis.exists?("account:#{@account.id}:regeneration")
end end
def get(limit, max_id = nil, since_id = nil, min_id = nil)
limit = limit.to_i
max_id = max_id.to_i if max_id.present?
since_id = since_id.to_i if since_id.present?
min_id = min_id.to_i if min_id.present?
if min_id.present?
redis_min_id = fetch_min_redis_id
return from_redis(limit, max_id, since_id, min_id) if redis_min_id && min_id >= redis_min_id
statuses = from_database(limit, redis_min_id, since_id, min_id)
return statuses if statuses.size >= limit
remaining_limit = limit - statuses.size
min_id = statuses.first.id unless statuses.empty?
from_redis(remaining_limit, max_id, since_id, min_id) + statuses
else
statuses = from_redis(limit, max_id, since_id, min_id)
return statuses if statuses.size >= limit
if since_id.present?
redis_min_id = fetch_min_redis_id
return statuses if redis_min_id.present? && since_id >= redis_min_id
end
remaining_limit = limit - statuses.size
max_id = statuses.last.id unless statuses.empty?
statuses + from_database(remaining_limit, max_id, since_id, min_id)
end
end
protected
def from_database(limit, max_id, since_id, min_id)
scope = Status.where(account: @account).or(Status.where(account: @account.following))
scope = scope.where(visibility: %i(public unlisted private))
scope
.to_a_paginated_by_id(limit, min_id: min_id, max_id: max_id, since_id: since_id)
.reject { |status| FeedManager.instance.filter?(:home, status, @account) }
.sort_by { |status| -status.id }
end
private
def fetch_min_redis_id
redis.zrangebyscore(key, '(0', '(+inf', limit: [0, 1], with_scores: true).first&.first&.to_i
end
end end

View file

@ -1,72 +1,32 @@
require 'rails_helper' require 'rails_helper'
RSpec.describe HomeFeed, type: :model do RSpec.describe HomeFeed, type: :model do
let(:account) { Fabricate(:account) } let(:account) { Fabricate(:account) }
let(:followed) { Fabricate(:account) }
let(:other) { Fabricate(:account) }
subject { described_class.new(account) } subject { described_class.new(account) }
describe '#get' do describe '#get' do
before do before do
account.follow!(followed) Fabricate(:status, account: account, id: 1)
Fabricate(:status, account: account, id: 2)
Fabricate(:status, account: account, id: 1) Fabricate(:status, account: account, id: 3)
Fabricate(:status, account: account, id: 2) Fabricate(:status, account: account, id: 10)
status = Fabricate(:status, account: followed, id: 3)
Fabricate(:mention, account: account, status: status)
Fabricate(:status, account: account, id: 10)
Fabricate(:status, account: other, id: 11)
Fabricate(:status, account: followed, id: 12, visibility: :private)
Fabricate(:status, account: followed, id: 13, visibility: :direct)
end end
context 'when feed is generated' do context 'when feed is generated' do
before do before do
FeedManager.instance.populate_home(account) Redis.current.zadd(
FeedManager.instance.key(:home, account.id),
[[4, 4], [3, 3], [2, 2], [1, 1]]
)
end end
it 'gets statuses with ids in the range from redis with database' do it 'gets statuses with ids in the range from redis' do
results = subject.get(3) results = subject.get(3)
expect(results.map(&:id)).to eq [12, 10, 3] expect(results.map(&:id)).to eq [3, 2]
expect(results.first.attributes.keys).to eq %w(id updated_at) expect(results.first.attributes.keys).to eq %w(id updated_at)
end end
it 'with since_id present' do
results = subject.get(3, nil, 3, nil)
expect(results.map(&:id)).to eq [12, 10]
end
it 'with min_id present' do
results = subject.get(3, nil, nil, 0)
expect(results.map(&:id)).to eq [3, 2, 1]
end
end
context 'when feed is only partial' do
before do
FeedManager.instance.populate_home(account)
Redis.current.zremrangebyrank(FeedManager.instance.key(:home, account.id), 0, -2)
end
it 'gets statuses with ids in the range from redis with database' do
results = subject.get(3)
expect(results.map(&:id)).to eq [12, 10, 3]
expect(results.first.attributes.keys).to eq %w(id updated_at)
end
it 'with since_id present' do
results = subject.get(3, nil, 3, nil)
expect(results.map(&:id)).to eq [12, 10]
end
it 'with min_id present' do
results = subject.get(3, nil, nil, 0)
expect(results.map(&:id)).to eq [3, 2, 1]
end
end end
context 'when feed is being generated' do context 'when feed is being generated' do
@ -74,20 +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 'returns from database' do it 'returns nothing' do
results = subject.get(3) results = subject.get(3)
expect(results.map(&:id)).to eq [12, 10, 3] expect(results.map(&:id)).to eq []
end
it 'with since_id present' do
results = subject.get(3, nil, 3, nil)
expect(results.map(&:id)).to eq [12, 10]
end
it 'with min_id present' do
results = subject.get(3, nil, nil, 0)
expect(results.map(&:id)).to eq [3, 2, 1]
end end
end end
end end