-
Notifications
You must be signed in to change notification settings - Fork 14
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
Added an "aggressive paging" mode to PagedNoteDataSource #1206
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all makes sense and the code looks good. However, I'm seeing a lot of these console messages when I go to the profile and scroll to the bottom:
Notice from nostr.fmt.wiz.biz: [NOTICE, ERROR: too many concurrent REQs]
Notice from offchain.pub: [NOTICE, ERROR: too many concurrent REQs]
Notice from soloco.nl: [NOTICE, ERROR: too many concurrent REQs]
Notice from e.nos.lol: [NOTICE, ERROR: too many concurrent REQs]
Notice from nos.lol: [NOTICE, ERROR: too many concurrent REQs]
To be fair, I'm seeing some from main
, too, but not nearly as many as I see in this branch. And this branch actually loads the old posts, whereas main
does not. So it works, but maybe we can back off some of the requests a bit, or something?
@joshuatbrown great catch! I did not see those but they were really bad. I found that I was calling the wrong |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking for a couple more changes. This is much better -- no more issues with too many concurrent REQs.
CHANGELOG.md
Outdated
@@ -8,11 +8,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
|
|||
## [Unreleased] | |||
|
|||
<<<<<<< fix-profile-not-loading-notes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops!
@@ -72,7 +72,7 @@ class PagedRelaySubscription { | |||
} | |||
|
|||
newUntilDates[subscription.relayAddress] = newDate | |||
await subscriptionManager.decrementSubscriptionCount(for: subscriptionID) | |||
await relayService.decrementSubscriptionCount(for: subscriptionID) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect!
Issues covered
#1192
Description
The problem here was that in paginated lists we could get a full page of notes from relays, none of which would actually get displayed in the UI. For example a user might post 20 replies in a row without posting a root note. Because
PagedNoteDataSource
was usingcellForRowAt()
to tell the pager to load more data ifcellForRowAt()
stopped getting called then we stopped loading more data.My fix was to detect when we are getting to the end of the list and go into "aggressive paging" mode where we start calling
pager.loadMore()
on a timer. Callingpager.loadMore()
constantly isn't actually as bad as it sounds. Unlike RESTful paging where you request page 0, then 1, then 2, in Nostr we use a sliding time window. We are asking relays "give me the 10 most recent notes published before X date". AndX date
is the oldest event returned so far, and it's unique to every relay we have a connection with. So callingloadMore()
just slides the time window further back and doesn't queue up more than a page's worth of events to download at any given time.How to test
This is tricky because it depends on live data that could change. i suppose I could've crafted some but I actually just used Onigirl (
npub18jvyjwpmm65g8v9azmlvu8knd5m7xlxau08y8vt75n53jtkpz2ys6mqqu3
) from our Discover tab and as of right now it looks like it's still reproducible on their profile.npub18jvyjwpmm65g8v9azmlvu8knd5m7xlxau08y8vt75n53jtkpz2ys6mqqu3