Skip to content
This repository has been archived by the owner on Oct 11, 2022. It is now read-only.

Fix pagination issues #685

Merged
merged 1 commit into from
May 5, 2017
Merged

Fix pagination issues #685

merged 1 commit into from
May 5, 2017

Conversation

brianlovin
Copy link
Contributor

@brianlovin brianlovin commented May 4, 2017

@mxstbr okay, so I spent a while getting reaaally deep in the rethinkdb docs and found that what we're trying to do is...somewhat complex. There were no clear answers for how to easily implement a between call the options we want (author id + descending createdAt timestamp). This is buried in the docs:

Note: You cannot specify multiple orders in a compound index. See issue #2306 to track progress.

This is a problem because what we're essentially trying to do is get all posts by an author, but in descending order (indexes are created in ascending order. I still gave it a try by creating a compound index like author, [r.row('author'), r.row('createdAt')], but since you can't get the createdAt to return in descending order it didn't work properly).

So I worked around a few possible solutions trying various sorts and filters, but then realized that what we're doing is solving a problem that is only a real problem when we hit massive scale. For now, we can reasonably expect a few thousand story records and querying against those will be nothing for the server.

Given this, I took the following approach:

  1. Created an index for story author (this is optional)
    Note: I didn't create an index for story frequency but we should be consistent either way we go. If we put an index on both author and frequency it means we can run the getAll which might be slightly faster, but honestly given our size it's probably negligible against a standard filter({ key }) query.
  2. The queries return all stories for the user and frequency. Which, again, we could reasonably expect to be quite low for a while as we grow.
  3. Run those stories through your paginate function, which does all the magic of setting the pageInfo fields.
  4. Make sure we're handling the hasNextPage field properly in the initial queries, fetchMore queries, and in the story feed component itself.

@brianlovin
Copy link
Contributor Author

Going to merge to keep us all up to date, but @mxstbr would be good to see if you have any thoughts on the first bullet point in the comment above, to see if we should have two new indexes or just run a filter() on these queries :)

@brianlovin brianlovin merged commit d8f9dd6 into v2 May 5, 2017
@brianlovin brianlovin deleted the pagination branch May 5, 2017 00:41
@mxstbr
Copy link
Contributor

mxstbr commented May 5, 2017

I'm fine with just running filter, this looks good to me! Thanks for digging in 👌

@brianlovin brianlovin mentioned this pull request May 5, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants