feat(performance): use $graphLookup for auth

[omairvaiyani]

trafficstars

At a high-level, this PR (work in progress) replaces the current recursive roles/auth query with a single aggregation pipeline to list all roles within the user's graph in one request. For applications at scale, this (at first set of benchmarks) appears to suggest a >10x reduction in request times before the cache kicks in.

After this discussion I have managed to land a big performance improvement. Each request by an authenticated user that results in a database query must be authorised, which currently means loading all the roles that the user is in, and loading the roles that yield to these, one level at a time.

Currently, for every new layer of roles added to an application, the request grows by 1. Depending on your setup, this can be quite a slow process. We've seen this take upwards of 5s on some of our heavy users with a significant amount of roles. There are certainly some application and schema design improvements that developers could make to reduce the total number or indeed levels of roles, however, we have a clear vector to optimise a very central part of this codebase.

Rough benchmarks: I ran these against my own application about 10 times for each of the following cases

Num roles: 5 Current: ~100-160ms New: ~25-30ms

Num roles: 50 Current: ~200-500ms New: ~40-60ms

Num roles: 1000 Current: ~900-4400ms New: ~65-120ms

Help needed The test suite uses mongo-runner which appears to have some shortcomings when dealing with $graphLookup, hence the anaemic test suite in this PR. Any suggestions here would be helpful. The current test-suite (Auth.spec.js) does not fully cover the role look up anyways.

Aggregation pipeline

// on _Join:users:_Role
[
    {
      $match: {
        relatedId: this.user.id,
      },
    },
    {
      $graphLookup: {
        from: '_Join:roles:_Role',
        startWith: '$owningId',
        connectFromField: 'owningId',
        connectToField: 'relatedId',
        as: 'childRolePath',
      },
    },
    {
      $facet: {
        directRoles: [
          {
            $lookup: {
              from: '_Role',
              localField: 'owningId',
              foreignField: '_id',
              as: 'Roles',
            },
          },
          {
            $unwind: {
              path: '$Roles',
            },
          },
          {
            $replaceRoot: {
              newRoot: {
                $ifNull: ['$Roles', { $literal: {} }],
              },
            },
          },
          {
            $project: {
              name: 1,
            },
          },
        ],
        childRoles: [
          {
            $lookup: {
              from: '_Role',
              localField: 'childRolePath.owningId',
              foreignField: '_id',
              as: 'Roles',
            },
          },
          {
            $unwind: {
              path: '$Roles',
            },
          },
          {
            $replaceRoot: {
              newRoot: {
                $ifNull: ['$Roles', { $literal: {} }],
              },
            },
          },
          {
            $project: {
              name: 1,
            },
          },
        ],
      },
    },
  ]

Work in progress:

• [x] Proof of concept
• [ ] Remove legacy functions
• [ ] Test-cases
• [ ] Postgres support or backwards compatibility
• [ ] Proper solution for DatabaseController...classExists problem

Credit to @noahsilas for paving the blocks for this

[omairvaiyani]

It seems like husky added a number of formatting changes on the staged files

[codecov[bot]]

Codecov Report

Merging #6556 into master will decrease coverage by 0.21%. The diff coverage is 73.56%.

@@            Coverage Diff             @@
##           master    #6556      +/-   ##
==========================================
- Coverage   94.01%   93.80%   -0.22%     
==========================================
  Files         169      169              
  Lines       11850    11876      +26     
==========================================
- Hits        11141    11140       -1     
- Misses        709      736      +27     

Impacted Files	Coverage Δ
src/Auth.js	86.85% <50.00%> (-13.15%)	:arrow_down:
src/Controllers/DatabaseController.js	95.19% <100.00%> (ø)
src/Adapters/Storage/Mongo/MongoStorageAdapter.js	92.82% <0.00%> (-0.68%)	:arrow_down:
src/RestWrite.js	93.48% <0.00%> (-0.17%)	:arrow_down:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d52d35b...d92dc8b. Read the comment docs.

[davimacedo]

@omairvaiyani nice idea. It looks very promising. Let me know if you need any help!

[omairvaiyani]

@davimacedo Thanks! We've battle-tested this on our production app ~25-40k heavy MAUs. No issues at all so far.

The two things stopping this PR from merge are:

• diff coverage (due to post-commit formatting issues) <- maybe I can re-submit a separate PR after resolving those?
• lack of testability for the $graphLookup as the mongodb runner in this repository is unable to properly handle it.

Any thoughts on getting around this?

[TylerBrock]

Hi all, any updates on this one, we'd love to upgrade from ParseServer 2.x but this is blocking us. ParseServer 2.x had a bug where it only returned the first 100 roles which is fixed in later versions but this makes ParseServer unusably slow for our product because we have some users that are in thousands of roles.

We'd like to help get this over the line, can @omairvaiyani tell me more about what leads to the lack of testability?

[omairvaiyani]

@TylerBrock I believe that the version of mongo-runner server used in the test-suite is unable to handle the $graphLookup stage in the pipeline, or at least, that was my observation while submitting this PR. Without it, testing in this manner is quite difficult. In our own private test-suites, we use Docker compose to test against a real Mongo instance. We have been using this pipeline to run our role auth for many months now without an issue.

Another issue making this PR a headache is the formatting changes that auto-ran on my commits, blurring the diff and failing the coverage checks.

[noahsilas]

@omairvaiyani I took a stab at rewriting the commits in this PR on top of master to remove all of the auto-formatting noise. Does https://github.com/Hustle/parse-server/commits/omairvaiyani/improve-auth-efficiency look like it captures your changes as expected?

[mtrezza]

⚠️ Important change for merging PRs from Parse Server 5.0 onwards!

We are planning to release the first beta version of Parse Server 5.0 in October 2021.

If a PR contains a breaking change and is not merged before the beta release of Parse Server 5.0, it cannot be merged until the end of 2022. Instead it has to follow the Deprecation Policy and phase-in breaking changes to be merged during the course of 2022.

One of the most voiced community feedbacks was the demand for predictability in breaking changes to make it easy to upgrade Parse Server. We have made a first step towards this by introducing the Deprecation Policy in February 2021 that assists to phase-in breaking changes, giving developers time to adapt. We will follow-up with the introduction of Release Automation and a branch model that will allow breaking changes only with a new major release, scheduled for the beginning of each calendar year.

We understand that some PRs are a long time in the making and we very much appreciate your contribution. We want to make it easy for PRs that contain a breaking change and were created before the introduction of the Deprecation Policy. These PRs can be merged with a breaking change without being phased-in before the beta release of Parse Server 5.0. We are making this exception because we appreciate that this is a time of transition that requires additional effort from contributors to adapt. We encourage everyone to prepare their PRs until the end of September and account for review time and possible adaptions.

If a PR contains a breaking change and should be merged before the beta release, please mention @parse-community/server-maintenance and we will coordinate with you to merge the PR.

Thanks for your contribution and support during this transition to Parse Server release automation!