Commit graph

9 commits

Author SHA1 Message Date
Daniel Uber
b2c7f7a5a3
Use Sidekiq::Job module alias for Sidekiq::Worker (#17526) 2022-05-02 11:12:58 -05:00
Daniel Uber
cc7c342bb5
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
2022-03-09 08:51:00 -06:00
Jamie Gaskins
9423060299
Fan out Feeds::ImportArticlesWorker (#16818)
* Fan out Feeds::ImportArticlesWorker

Doing all that work within a single Sidekiq job has begun taking over an
hour on DEV. Regardless of the reasons we did it that way originally, we
should be able to handle this concurrently. If we cannot, we need to
investigate why and handle it properly rather than consigning it to
sequential work.

* Fix specs

The specs make assumptions about how the code under test is implemented.
This commit does not change that, as much as I would like to. Instead,
it just aligns the assumptions with the new implementation.

The previous tests didn't actually represent reality though, since we
can't get a Time or ActiveSupport::TimeWithZone instance inside of the
`perform` method while running it through Sidekiq. Instead, the specs
seem to be relying on the fact that the time instance gets serialized to
ISO-8601/RFC3339 format and that that format is understood by Postgres.
Otherwise, I'm not sure how it would work in production as written.

The new specs reflect reality more closely. The `earlier_than` value
will be converted into an ISO8601/RFC3339 string when passed through
Sidekiq.

* Add parens to perform_bulk call

Turns out, we actually do this pretty consistently. I could've sworn I
saw a bunch of these calls without parens. ¯\_(ツ)_/¯

* Improve variable naming

This is not a list of ids, it's a list of lists of arguments for Sidekiq
jobs, the inner of which contains an id, but that's not the only thing
it contains.
2022-03-08 15:39:16 -05:00
Jeremy Friesen
4e76771867
Favoring User scope as parameter for feed fetching (#16763)
This change makes it easier to resolve forem/forem#16487.

What do I mean by that?  To address forem/forem#16487, I need to only
select user's who are authorized to create articles.  In
forem/forem#16732 I added a method that will allow chaining of a User
scope.  So with this current commit and forem/forem#16732, I'm
triangulating on an approach that will make that change easier.

I did not want to conflate those two, as mixing this PR's change and
what is necessary for the closing PR would create a more complicated
review.  Not unduly complicated, but one that will require more tests
and a change in logic.  And for someone reviewing the diff, those
concerns could easily be lost.

Yes, I have changed the method signature, but that method signature is
limited to one location:

```shell
❯ rg "Feeds::Import.call"

app/workers/feeds/import_articles_worker.rb
21:      ::Feeds::Import.call(users_scope: users_scope, earlier_than: earlier_than)
```

So I look to the method signature a bit as an internal API, hence the change.

Related to forem/forem#16487
2022-03-02 16:45:06 -05:00
Molly Struve
e0512d8689
Enforce Uniqueness for Feed Import and Push Notification Cleanup Workers (#14196) 2021-07-13 14:37:13 -04:00
Michael Kohl
8f42828028
Change signature of Feeds::ImportArticlesWorker (#13971) 2021-06-17 14:28:23 -04:00
rhymes
e7f9735354
Remove RssReader (#12169)
* Add script to remove feature flag

* Remove :feeds_import feature flag and RssReader and related classes

* Change Feeds::ImportArticlesWorker signature to support Sidekiq Cron serialization

* Replace RssReader::Assembler with Feeds::AssembleArticleMarkdown

* Removing Assembler
2021-01-08 11:45:15 -05:00
rhymes
4aaeb3e955
Feeds::Import: only import feeds that haven't been recently updated (#12126)
* Add earlier_than to Feeds::Import

* Ignore feeds that have been imported in the last 4 hours

* Make earlier_than mandatory in Feeds::ImportArticlesWorker
2021-01-06 16:58:42 +01:00
rhymes
1741e52e83
Add missing features to new feed importer (#11501)
* Add explanatory comment to how RssReaderFetchUserWorker works

* Add Feeds::ImportWorker worker

* Enable Feeds::Import for single user feed fetching behind a feature flag

* Add Feeds::ValidateUrl service

* Add feature flag to user's validate feed URL feature

* Remove todo notices

* Add feature flag to fetch_all_rss task

* Add feature flag to RssReaderWorker

* Bring back short-circuit for Articles::RssReaderWorker
2020-11-24 18:11:13 +01:00