chinwag/chinwagsocial
chinwag/chinwagsocial
2
0
Fork 0
Code Issues Pull requests Releases 5 Wiki Activity

Fix multiple boosts of a same toot erroneously appearing in TL (#14759)

Browse source 
* Check for and record reblog info atomically

Instead of using ZREVRANK to determine whether a reblog is a new reblog or not,
use ZADD's NX option to perform the check/addition option atomically.

* Replace ZREVRANK call with ZSCORE key which is more efficient

* Make tests a bit stricter

* Fix off-by-one
This commit is contained in:
ThibG 2020-09-07 18:00:15 +02:00 committed by GitHub
parent e79d719e92
commit 517af45e32
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 11 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
app/lib/feed_manager.rb
View file

@ -77,9 +77,11 @@ class FeedManager
# Get the score of the REBLOG_FALLOFF'th item in our feed, and stop # Get the score of the REBLOG_FALLOFF'th item in our feed, and stop
# tracking anything after it for deduplication purposes. # tracking anything after it for deduplication purposes.
falloff_rank = FeedManager::REBLOG_FALLOFF - 1 falloff_rank = FeedManager::REBLOG_FALLOFF
falloff_range = redis.zrevrange(timeline_key, falloff_rank, falloff_rank, with_scores: true) falloff_range = redis.zrevrange(timeline_key, falloff_rank, falloff_rank, with_scores: true)
falloff_score = falloff_range&.first&.last&.to_i || 0 falloff_score = falloff_range&.first&.last&.to_i
return if falloff_score.nil?
# Get any reblogs we might have to clean up after. # Get any reblogs we might have to clean up after.
redis.zrangebyscore(reblog_key, 0, falloff_score).each do |reblogged_id| redis.zrangebyscore(reblog_key, 0, falloff_score).each do |reblogged_id|
@ -279,14 +281,12 @@ class FeedManager
return false if !rank.nil? && rank < FeedManager::REBLOG_FALLOFF return false if !rank.nil? && rank < FeedManager::REBLOG_FALLOFF
reblog_rank = redis.zrevrank(reblog_key, status.reblog_of_id) # The ordered set at `reblog_key` holds statuses which have a reblog
# in the top `REBLOG_FALLOFF` statuses of the timeline
if reblog_rank.nil? if redis.zadd(reblog_key, status.id, status.reblog_of_id, nx: true)
# This is not something we've already seen reblogged, so we # This is not something we've already seen reblogged, so we
# can just add it to the feed (and note that we're # can just add it to the feed (and note that we're reblogging it).
# reblogging it).
redis.zadd(timeline_key, status.id, status.id) redis.zadd(timeline_key, status.id, status.id)
redis.zadd(reblog_key, status.id, status.reblog_of_id)
else else
# Another reblog of the same status was already in the # Another reblog of the same status was already in the
# REBLOG_FALLOFF most recent statuses, so we note that this # REBLOG_FALLOFF most recent statuses, so we note that this
@ -300,9 +300,7 @@ class FeedManager
# delay of the worker deliverying the original status, the late addition # delay of the worker deliverying the original status, the late addition
# by merging timelines, and other reasons. # by merging timelines, and other reasons.
# If such a reblog already exists, just do not re-insert it into the feed. # If such a reblog already exists, just do not re-insert it into the feed.
rank = redis.zrevrank(reblog_key, status.id) return false unless redis.zscore(reblog_key, status.id).nil?
return false unless rank.nil?
redis.zadd(timeline_key, status.id, status.id) redis.zadd(timeline_key, status.id, status.id)
end end

4
spec/lib/feed_manager_spec.rb
View file

@ -444,8 +444,8 @@ RSpec.describe FeedManager do
expect(Redis.current.exists?(reblog_set_key)).to be true expect(Redis.current.exists?(reblog_set_key)).to be true
expect(Redis.current.zrange(reblogs_key, 0, -1)).to eq [reblogged.id.to_s] expect(Redis.current.zrange(reblogs_key, 0, -1)).to eq [reblogged.id.to_s]
# Push everything off the end of the feed. # Push everything past the reblog falloff.
FeedManager::MAX_ITEMS.times do FeedManager::REBLOG_FALLOFF.times do
FeedManager.instance.push_to_home(receiver, Fabricate(:status)) FeedManager.instance.push_to_home(receiver, Fabricate(:status))
end end