A quota-like mechanism

[pefimo]

• I have read the CONTRIBUTING.md and followed the provided tips

Problem description
Large NextCloud instances using News app face the issue #110, i.e. when some of the users subscribe large numbers of feeds and then don't read them, News tends to fill the database with unread items (auto-purging doesn't remove unread items). It slows down such instances to the point that admins have to manually execute SQL queries to get rid of excessive numbers of articles just to let their servers be less slow.

Solution idea
An idea described shortly in the issue #110 is about defining a simple quota mechanism, based on number of articles a user can fetch.

Since few people are interested in such functionality, my approach wouldn't be too sophisticated: I would just add configuration parameter (off/unset by default) that would set an upper limit of number of feed items a user can keep in all their subscribed feeds. After reaching that limit, new items would not be added to any of the subscribed feeds of the user in question.

Alternatives considered
I haven't found any working alternatives and nobody following the issue #110 did.

Right now a friend marks items as read with an SQL script and triggers news:updater:after-update command to get rid of articles nobody would read anyway, but that's obviously not a solution.

One approach I've been thinking about but have decided not to use is a query that just deletes anything that doesn't fit under certain volume limit. Example below uses the limit of 200k characters, which might be too little, but it's just a proof of concept.

DELETE FROM *PREFIX*news_items
WHERE id IN (
SELECT t.item_id
FROM (SELECT ni.id `ITEM_ID`, LENGTH(ni.body) `VOLUME`, SUM(LENGTH(ni.body)) OVER u `ACC`
      FROM *PREFIX*news_feeds nf
      JOIN *PREFIX*news_items ni ON nf.id = ni.feed_id
      WINDOW u AS (PARTITION BY user_id ROWS UNBOUNDED PRECEDING)
      ) AS `T`
WHERE t.acc > 200000
);

This query uses a running sum of lengths of feed items to select item ids to delete. It might be a way to trim the database after introducing quota mechanism, but avoiding fetching/storing feed items is a better idea. So I'm working on a mechanism to prevent downloading more than a reasonable number of items (with a configurable definition of "reasonable"). Of course this query can also be rewritten to focus on number of items, instead of their volume.

Solution details
I've been studying News app's source code and database structure and have got a sketch of an implementation. Now it is time to make an important design decision. I would like the maintainers to be involved, hence I start this discussion.

I see 3 options:

1. Naive: in FeedService.update($feedId, $userId, $forceUpdate), count how many feed items a user has got in all their feeds, and only fetch and store new items unless they have passed the limit.
2. Risky: in FeedService.updateAll(), replace the loop iterating over all feeds with two loops: one iterating over users, another one iterating over a particular user's feeds. This way, we would be able to count user's items once and then update the count while fetching feed items in FeedService.update($feedId, $userId, $forceUpdate).
3. Also risky: combine above adjustment to updateAll and introduce a FeedMapper.findAllWithinLimit() method that would return only feeds of users who haven't run out of storage.

Option 1 is really naive as it repeatedly performs COUNT queries, but should also be quite safe from overall software stability perspective. Options 2 and 3 are risky (because updateAll is not covered by tests), but they avoid the database cost of executing COUNT too often. There may be other approaches I didn't think of, so let's discuss the options.

[piggz]

Great that someone is looking at the issue, but im not sure about refusing to add new articles ... I would much prefer for old ones to be removed.

[muppeth]

@pefimo Great that you are poking around this issue. I think the big issue here is also articles_per_update row in news_feed table. I have a feed that each time pulls 10018 articles. So even if they get removed, with next update they will all be back there.

[SMillerDev]

From a ux perspective I don't think we could block updates. Users don't really have a way to delete items so this would cause unnecessary annoyance. A much better way would be a checkbox to allow removing unread items in cleanup and/or having a way to get cleanup to respect a time period rather than an amount.

[muppeth]

i agree. i think as admin having an option to decide how long you want to keep unread (or read for that matter) items would be a solution for me. if the articles ler update could also have a cap server wide,youcould lowerthe amount of items comming in or staying even in read state on the server. additionally, being able to setup how many feeds max a user can have also would help. i have users we few k feeds. it is impossible for a human to read through all that. some people are just hurding every rss feed they come across without thinking about how much impact it has on the server. 20 Nov 2020 09:16:59 Sean Molenaar <notifications@github.com>:

…

From a ux perspective I don't think we could block updates. Users don't really have a way to delete items so this would cause unnecessary annoyance. A much better way would be a checkbox to allow removing unread items in cleanup and/or having a way to get cleanup to respect a time period rather than an amount. — You are receiving this because you commented. Reply to this email directly, view it on GitHub[https://github.com/nextcloud/news/issues/953#issuecomment-731017185], or unsubscribe[https://github.com/notifications/unsubscribe-auth/ACAWQPSQUUDJMY7HEQH2PR3SQYQXRANCNFSM4T35AM3Q]. [data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAC4AAAAuCAYAAABXuSs3AAAAAXNSR0IArs4c6QAAAARzQklUCAgICHwIZIgAAAAeSURBVGiB7cEBAQAAAIIg/69uSEABAAAAAAAAADwbIT4AAflJgG4AAAAASUVORK5CYII=###24x24:true###][Tracking image][https://github.com/notifications/beacon/ACAWQPUQQMZRNA7CE3KCM43SQYQXRA5CNFSM4T35AM32YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOFOJG7YI.gif]

[pefimo]

According to my knowledge, `articles_per_update` is only used during auto-purge. But setting it manually wouldn't help because it's overwritten each time News fetches new articles. I'm studying the code right now to propose another solution. I'll post another comment when I get more ideas.

[pefimo]

@muppeth What did you mean by this line?

if the articles per update could also have a cap server wide

A global (server-wide) limit?

[pefimo]

Here goes my second proposal, taking into account all ideas mentioned in this issue:

1. Introduce 3 new configuration parameters:
   • aggressivePurging - that would control which auto-purging method should be used to clean up the database (default: false);
   • maxArticlesPerUpdate - with aggressivePurging enabled it would be the upper limit of items a feed can insert into database at once (default: not set);
   • maxArticleAgeInDays - how long an article should be kept in user's feed, regardless of its unread property (default: not set).
2. Introduce a new method to actually perform purging. It would be similar to what deleteReadOlderThanThreshold does but disrespecting unread property and removing anything older than maxArticleAgeInDays.
3. In autoPurgeOld, read aggressivePurging and choose which method should be called for purging of old items.
4. Adjust feed retrieval code to set articlesPerUpdate to min($itemCount, $maxArticlesPerUpdate).
5. Adjust admin UI to handle 3 new parameters.

With a separate method to perform purging and a parameter that would default to keep traditional approach, News application should be safe for everyone who doesn't care about resources, while larger instances would have tools to limit resource usage.

Another improvement I'd like to add will be discussed later and boils down to introducing a limit for the number of feeds a user can subscribe to.

What do you think @SMillerDev and @Grotax?

[SMillerDev]

I'm really opposed to limits that happen on fetching the feed. There's really no way I can think of where this doesn't flood us with questions about missing items in the import.

Beyond that I think this could be much simpler by removing support for clearing a number of items (since that is difficult to implement anyway) and just adding a checkbox purgeUnread and a maxItemAge.

[pefimo]

I'm really opposed to limits that happen on fetching the feed.

Then how would you approach the issue? Right now there's no mechanism to protect the infrastructure from resource depletion caused by users who are unaware of the inner workings of News application. So these users keep subscribing to too many feeds, leading to huge amounts of data being stored in the database (sometimes with many duplicates).

I'd like to find a solution that suits both maintainers and administrators.

this could be much simpler by removing support for clearing a number of items (since that is difficult to implement anyway)

What do you mean by that? In my opinion, everything I've described above can be implemented with relatively simple changes to existing code-base.

[pefimo]

@SMillerDev Could you take a look and reply please? I'd proceed with implementation when we reach an agreement about the solution/approach.

[pefimo]

If anybody would like to review my work in progress, please take a look at https://github.com/pefimo/news/tree/purge-unread and contact me.

[anoymouserver]

I would recommend to create a Draft PR, which would make it easier to review and comment.

[SMillerDev]

Then how would you approach the issue? Right now there's no mechanism to protect the infrastructure from resource depletion caused by users who are unaware of the inner workings of News application. So these users keep subscribing to too many feeds, leading to huge amounts of data being stored in the database (sometimes with many duplicates).

I'm fine with an error saying Denied: 10/10 feeds subscribed on adding a feed (with a configurable limit). But we can't tell the user why we deleted half of their feed so a limit on the import would be an issue (and would probably cause the user to re-add the feed a couple of times, creating the opposite situation from what you want)

[pefimo]

I've created the draft PR #970 as suggested above. Would anybody be interested in discussing the approach now?

[pefimo]

Considering our options, we've decided we aren't interested in using NC News any longer -- it consumes too much resources. (As an example: the NextCloud feed is fetched more than 100 times a day on the installation in question, instead of once, since any user should see the same content in a public RSS like that.)

Therefore, I suggest closing this issue.

[piggz]

Well, i'd personally still like to see a quota/way of indiscriminately deleting old news items.