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

Ensure that feed renegeration restores non-zero items #5409

Merged
merged 1 commit into from Oct 16, 2017

Conversation

Gargron
Copy link
Member

@Gargron Gargron commented Oct 16, 2017

Fix #5398

Ordering the home timeline query by account_id meant that the first
100 items belonged to a single account. There was also no reason to
reverse-iterate over the statuses. Assuming the user accesses the
feed halfway-through, it's better to have recent statuses already
available at the top. Therefore working from newer->older is ideal.

If the algorithm ends up filtering all items out during last-mile
filtering, repeat again a page further. The algorithm terminates
when either at least one item has been added, or if the database
query returns nothing (end of data reached)

Fix #5398

Ordering the home timeline query by account_id meant that the first
100 items belonged to a single account. There was also no reason to
reverse-iterate over the statuses. Assuming the user accesses the
feed halfway-through, it's better to have recent statuses already
available at the top. Therefore working from newer->older is ideal.

If the algorithm ends up filtering all items out during last-mile
filtering, repeat again a page further. The algorithm terminates
when either at least one item has been added, or if the database
query returns nothing (end of data reached)
@Gargron Gargron added the bug Something isn't working label Oct 16, 2017
@Gargron
Copy link
Member Author

Gargron commented Oct 16, 2017

Aside: Should I use the same strategy here?

https://github.com/tootsuite/mastodon/blob/894da3dcca781e27ce9c5130f1021526ac8a6887/app/models/feed.rb#L26-L30

To make sure that even on first load of inactive user, feed is not empty?

@aschmitz
Copy link
Contributor

I don't have any idea why this was grouped by account_id before, but this is clearly a better way of doing it. I don't have numbers for how many times you're likely to filter out all potential statuses, but making sure there's at least one seems fine.

In practice, this seems likely to be overkill, but it's in the right direction, so sure, I say go for it. 👍

In terms of Feed#from_database, I'd be inclined to think leaving that alone is fine: it only gets called when the worker is in the process of rebuilding using exactly this code, so an extra loop over it (while usually not actually used?) would only be doing extra work that the worker is already doing. On the off chance that it's empty, the worker will fill in a status eventually, and not block the web worker while it does it. (If you do want to use the same code, I'd consider just calling FeedManager.instance.populate_feed, to avoid needlessly duplicating the code, but then I don't really see the point of a separate worker.)

@Gargron Gargron merged commit 7cc7174 into master Oct 16, 2017
@Gargron Gargron deleted the fix-improve-feed-regeneration branch October 16, 2017 14:10
@akihikodaki akihikodaki removed their request for review October 16, 2017 15:46
@akihikodaki
Copy link
Contributor

Sorry for late review, but I think the reverse-iteration made difference in case of reblogs. When adding reblogs, add_to_feed checks if there is the reblogged status before. That would not work without reverse-iteration.

cobodo pushed a commit to cobodo/mastodon that referenced this pull request Oct 20, 2017
Fix mastodon#5398

Ordering the home timeline query by account_id meant that the first
100 items belonged to a single account. There was also no reason to
reverse-iterate over the statuses. Assuming the user accesses the
feed halfway-through, it's better to have recent statuses already
available at the top. Therefore working from newer->older is ideal.

If the algorithm ends up filtering all items out during last-mile
filtering, repeat again a page further. The algorithm terminates
when either at least one item has been added, or if the database
query returns nothing (end of data reached)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants