Skip to content

Conversation

christophehenry
Copy link
Contributor

@christophehenry christophehenry commented May 7, 2025

I remember encountering this issue when working on FreshRSS Android: the n query param for reading-list endpoint limits the number of item that can be retreived and it defaults to a stupidely low value (20) when unset. This will prevent fetching items from feed that heven't published for some time. This is a problem for initial fetching since these feed will always appear empty. There's currently no way to force fetch those older items. Setting the n query parameter to a high value during initial fetch mitigates this issue. It doesn't resolve though, since there's still a chance that a server has more than 2 147 483 647 (which is the value of Int.MAX_VALUE on Android) items. I think there's some paging mecanism on this endpoint but I don't remember where it's documented.

Note: IMHO MAX_ITEMS and MAX_STARRED_ITEMS should be replaced by Int.MAX_VALUE everywhere even when not doing initial fetch because the problem also risks to reproduce in the case where Readrops has performed an initial sync but hasen't synced in a sufficiently long time that the number of new items to process is higher than MAX_ITEMS or MAX_STARRED_ITEMS. And this case, howerver unlikely, will be very tricky to debug. The ot query parameter is already there to limit the number of results. I don"t think it is a good idea to limit it again with n. At least not until TimelineTab has a bottom pull-to-refresh feature that force fetching older items.

This should solve #282

@christophehenry christophehenry force-pushed the fix-initial-sync-problem branch from e33e24b to b308502 Compare May 7, 2025 10:42
Copy link

codecov bot commented May 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 17.73%. Comparing base (44d00e3) to head (b308502).

Additional details and impacted files
@@            Coverage Diff             @@
##             develop     #306   +/-   ##
==========================================
  Coverage      17.73%   17.73%           
  Complexity       460      460           
==========================================
  Files            190      190           
  Lines          10327    10327           
  Branches        1611     1611           
==========================================
+ Hits            1831     1832    +1     
+ Misses          8398     8397    -1     
  Partials          98       98           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Shinokuni
Copy link
Member

Using Int.MAX_VALUE seems to me a bad idea if only because it could create a ton of PHP OOM for medium-end instances and below. I already had reports that 5000 items was too much for certain PHP configurations, so I can't imagine what it would be with Int.MAX_VALUE.

Pagination should be the thing to, but design wise, I am against fetching the entire database. We could argue on the number of items to fetch during initial synchronization but I don't think that Readrops should be a complete mirror of the web version.

On the other hand, using pull to refresh to fetch older items seems a good way to dodge this limitation.

Regarding #282, there is a thing that FreshRSS does a bit under the hood, it uses the insertion date as the default way of sorting, which isn't a bad idea, but when you ask to the API the most recent items, it's not clear what kind of sort is really applied.

@christophehenry
Copy link
Contributor Author

christophehenry commented Jun 2, 2025

Using Int.MAX_VALUE seems to me a bad idea if only because it could create a ton of PHP OOM for medium-end instances and below.

Fair enough. I confess I left this problem for later optimisation while working on FreshRSS-Android and, admittedly, never got back to it…

Ok, as a compromise, how about a button buried in the settings? At least while the pull-to-fetch feature is developed? I often find myself searching for old articles to repost them and it's really inconvenient to having to go back to the web UI for that.

I don't think that Readrops should be a complete mirror of the web version.

Well if we want full-text search at some point, this is necessary. Unless FreshRSS has an endpoint for performing research I don't know about?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants