Skip to content

Revert using objectId as tie breaker in infinite scrolling #1799

Closed

Description

@mtrezza

New Issue Checklist

Issue Description

Since #1706, creating an index to optimize dashboard queries requires compound indices including objectId. For example, when browsing a class sorted by createdAt a compound index createdAt, objectId is required.

if (field !== 'objectId') {
if (sortDir === '-') {
query.addDescending('objectId');
} else {
query.addAscending('objectId');
}
}

This approach requires special indices just for dashboard browsing. If an app is using a complex or resource intensive index, dashboard requires the same index plus the additional objectId in the index compound. I don't think this is a feasible solution, given the high cost of indices on a DB. Maybe we need to look for an alternative solution.

Additionally, #1706 broke existing "query optimization" and should have been treated as a breaking change. The reason this was released as non-breaking was that we didn't have the policy that a changelog entry has to be written as part of the PR back then. It was not noticeable without reading the PR discussion in detail that it was a breaking change. This should not happen anymore as we are now requiring a changelog entry with the PR.

Steps to reproduce

  1. Create new call
  2. Browse class, sorted by createdAt
  3. Query createdAt, objectId sent to server

Actual Outcome

Dashboard currently paginates by issuing an entirely new query which requires to construct a complicated pagination mechanism (#1706), which creates the indexing issue described here.

Expected Outcome

In MongoDB at least, the proper and resource efficient way to paginate is to use the query cursor, see getMore. That may require some changes on the server side, for example sending the cursor ID to dashboard as query results meta data. And not sure whether Postgres supports cursor based pagination.

In any case, I think we should revert #1706 ASAP and release as a fix, or at least the part that adds the object ID to the query, because that can render the dashboard unusable or cause stability issues for some deployments.

@sadakchap @davimacedo @Klaitos What do you think?

Environment

n/a

Dashboard

  • Parse Dashboard version: 2.2.0, master

Logs

n/a

Activity

  1. parse-github-assistant commented on Sep 14, 2021

    @parse-github-assistant

    Thanks for opening this issue!

    • 🚀 You can help us to fix this issue faster by opening a pull request with a failing test. See our Contribution Guide for how to make a pull request, or read our New Contributor's Guide if this is your first time contributing.
  2. changed the title [-]Default indices missing for sort by createdAt[/-] [+]docs: add indexing considerations to optimize dashboard queries[/+] on Sep 14, 2021
  3. changed the title [-]docs: add indexing considerations to optimize dashboard queries[/-] [+]Add indexing considerations to optimize dashboard queries[/+] on Sep 14, 2021
  4. changed the title [-]Add indexing considerations to optimize dashboard queries[/-] [+]Revert using objectId as tie breaker in infinite scrolling[/+] on Sep 14, 2021
  5. added
    type:bugImpaired feature or lacking behavior that is likely assumed
    and removed on Sep 14, 2021
  6. sadakchap commented on Sep 14, 2021

    @sadakchap
    Member

    @mtrezza, yeah.. we should revert #1706 . Could you please do that? And then we can tackle #1551 later separately (probably using getMore).

  7. Klaitos commented on Sep 14, 2021

    @Klaitos
    Contributor

    I'm glad you changed your mind about it 🙂.

  8. mtrezza commented on Sep 14, 2021

    @mtrezza
    MemberAuthor

    It's unfortunate that we need to revert it, because that opens the original scrolling issue again, and @sadakchap put some good work into fixing this. However, I think the implications we discovered now do not justify the type of fix.

    @Klaitos Would you want to give it a try to open a PR to revert this?

  9. Klaitos commented on Sep 15, 2021

    @Klaitos
    Contributor

    Done here #1800

  10. mtrezza commented on Sep 16, 2021

    @mtrezza
    MemberAuthor

    Closing via #1800

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    type:bugImpaired feature or lacking behavior that is likely assumed

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions