Limit per-user feed import fanout to users with feed urls set (#16831)

* Limit feeds import fanout to users with feeds

This filtering for feed settings occurs in
Feeds::Import#filter_users_from already, so the enqueued ForUser
jobs were mostly no-op, but the queue latency spiked as hundreds of
thousands of jobs were added to default.

Prescreen users to avoid enqueuing a noop job every hour for every single
user without a feed.

* Update tests to check only jobs for users with feeds are enqueued

* spec cleanup

Only include alice when we check no job is enqueued for her.

Assert no job is enqueued for alice.

Rename bob to user when alice isnt there to contrast.

* Update spec

Use update_columns to bypass setting validation (checking feed is
valid/reachable)

Remove unneeded timecop block when passing a set time

Remove sidekiq: fake and allow Import.call to be received

* Empty commit to trigger CI rebuild
This commit is contained in:
Daniel Uber 2022-03-09 08:51:00 -06:00 committed by GitHub
parent 9be5c8c6b0
commit cc7c342bb5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 18 additions and 20 deletions

View file

@ -16,6 +16,7 @@ module Feeds
# the last time a feed was fetched at
earlier_than = nil
else
users_scope = users_scope.where(id: Users::Setting.with_feed.select(:user_id))
earlier_than ||= 4.hours.ago
end

View file

@ -2,46 +2,43 @@ require "rails_helper"
RSpec.describe Feeds::ImportArticlesWorker, type: :worker, sidekiq: :inline do
let(:worker) { subject }
let(:feed_url) { "https://medium.com/feed/@vaidehijoshi" }
include_examples "#enqueues_on_correct_queue", "medium_priority"
describe "#perform" do
it "calls the Feeds::Import defaulting to 4 hours ago" do
it "processes jobs for users with feeds" do
allow(Feeds::Import).to receive(:call)
alice = create(:user)
bob = create(:user)
bob.setting.update_columns(feed_url: feed_url)
# bob has a feed, and alice doesn't, so we only enqueued for bob
Timecop.freeze(Time.current) do
worker.perform
# Each user is run separately. Note that this will not be run
# sequentially like this in production. This only works like this due
# to the `sidekiq: :inline` tag on the `RSpec.describe` block
expect(Feeds::Import)
.to have_received(:call)
.with(users_scope: User.where(id: [alice.id]), earlier_than: 4.hours.ago.iso8601)
expect(Feeds::Import)
.to have_received(:call)
.with(users_scope: User.where(id: [bob.id]), earlier_than: 4.hours.ago.iso8601)
expect(Feeds::Import)
.not_to have_received(:call)
.with(users_scope: User.where(id: [alice.id]), earlier_than: 4.hours.ago.iso8601)
end
end
it "calls the Feeds::Import with the given time" do
it "enqueues job for user with the given time" do
allow(Feeds::Import).to receive(:call)
alice = create(:user)
bob = create(:user)
user = create(:user)
user.setting.update_columns(feed_url: feed_url)
Timecop.freeze(Time.current) do
worker.perform([], 1.minute.ago)
earlier_than = 1.minute.ago
worker.perform([], earlier_than)
# Each user is run separately
expect(Feeds::Import)
.to have_received(:call)
.with(users_scope: User.where(id: [alice.id]), earlier_than: 1.minute.ago.iso8601)
expect(Feeds::Import)
.to have_received(:call)
.with(users_scope: User.where(id: [bob.id]), earlier_than: 1.minute.ago.iso8601)
end
expect(Feeds::Import).to have_received(:call).with(
users_scope: User.where(id: user.id),
earlier_than: earlier_than.iso8601,
)
end
it "calls Feeds::Import with the users from the given user ids and no time" do