Skip to content

Very poor performance when using RedisCacheAdapter #5401

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

Closed
saulogt opened this issue Feb 28, 2019 · 16 comments
Closed

Very poor performance when using RedisCacheAdapter #5401

saulogt opened this issue Feb 28, 2019 · 16 comments

Comments

@saulogt
Copy link
Contributor

saulogt commented Feb 28, 2019

Issue Description

I'm running my Parse Server app on 4 Heroku dynos for some time now. It's a 10k requests per minute app.
I Recently noticed some issues where data were not found just after saving to parse. I immediately thought it would be related to caching, then I tested RedisCacheAdapter and the problem was solved. The documentation is not clear about the need to use a distributed cache. Should I use rediscache even on a (multi-cpu) clustered app?

After some time my app started to slow down to the point of getting timeouts. Adding more dynos didn't help

is-parse-server metrics heroku 2019-02-28 14-52-56

Heroku Redis (premium 0) metrics:
redis-opaque-85942 heroku data 2019-02-28 14-56-51

I'm under the impression that I reached the scale limit of parse server.

Steps to reproduce

Add a RedisCacheAdapter to a heavy multi-node app

Expected Results

Run smoothly

Actual Outcome

Timeouts

Environment Setup

  • Server

    • parse-server version (Be specific! Don't say 'latest'.) : 2.8.4
    • Operating System: N/A
    • Hardware: Heroku Standard-2X (4 dynos)
    • Localhost or remote server? (AWS, Heroku, Azure, Digital Ocean, etc): Heroku
  • Database

    • MongoDB version: 3.2.20
    • Storage engine: WiredTiger
    • Hardware: mLab dedicated
    • Localhost or remote server? (AWS, mLab, ObjectRocket, Digital Ocean, etc): mlab

Logs/Trace

@georgesjamous
Copy link
Contributor

georgesjamous commented Mar 1, 2019

I believe parse uses the cache only for roles and sessions. No actual query results are saved or fetched from cache.
If your Redis server can handle the load, you can run as many nodes as you can. I don't think its a parse problem.

For this:

I Recently noticed some issues where data were not found just after saving to parse.

Are you perhaps not using the default MongoDB readPreference?
Because if you are running a multi-node database, reading from secondaries could lead to stale data.

@saulogt
Copy link
Contributor Author

saulogt commented Mar 1, 2019

The data problem is indeed related to user, session and role. And after setting RedisCacheAdapter the data inconsistency is solved. The issue seems to be ONLY related to performace when distributed cache is on.

Apparently the problem is because I rely on fields from request.user in the beforeSave hook

Parse.Cloud.beforeSave('TableName', function(request, response) {
  const account = request.user && request.user.get('account'); // <<< the account object is not present in the cached user
  if (account && !request.object.has('account')) {
    request.object.set('account', account);
  }
  
}

@georgesjamous
Copy link
Contributor

@ saulogt Oh ok, I understand now. Can you clarify what you mean by distributed cache ? It it a redis thing? or you mean without a Redis adapter?

@georgesjamous
Copy link
Contributor

georgesjamous commented Mar 4, 2019

And one more thing, when using the default in-memory-cache and running multiple nodes.
Each server will have its own cache, thus, load balanced requests between different instances will result in many cache-hit fails and will cause a database request almost on every request.
That's why your performance issue was solved when you introduced a single cache, the Redis adapter.

Now regarding your code snippet.
This was added a while back, I don't recall if it was there originally.

const userJSON = await cacheController.user.get(sessionToken);

Anyhow, it seems like when the user is being updated, the cache is not being cleared, I don't know if I am mistaken, but in practice, it shouldn't be cleared. Data inconsistency should be tolerated in a cache. Otherwise, it would be a replicated database.
I suggest, for the time being, if you rely on up-to-date data, fetch the user on each request. Performance is the price of consistency.

@saulogt
Copy link
Contributor Author

saulogt commented Mar 4, 2019

Hey @georgesjamous . Thanks for your response. By "distributed cache", I actually meant "distributedly available and consistent cache with ReidsCache" :)

And using RedisCacheAdapter actually caused the performance issue, but solved the data consistency problem.

@saulogt
Copy link
Contributor Author

saulogt commented Mar 4, 2019

There is another case where I found a problem.
See the sequence of operations uning two node with in-memory cache:

  1. User is created (dyno1)
  2. Role (role_xyz) is created (dyno 2)
  3. User is added to role_xyz (dyno2)
  4. Object is created with read access to the role_xyz only (dyno2)
  5. Object is retrieved (dyno1) <<<<<< Object not found because dyno 1 does not know that role_xyz has the user

This case can also be solved with redis cache, despite the perf issue

@georgesjamous
Copy link
Contributor

There is another case where I found a problem.
See the sequence of operations uning two node with in-memory cache:

  1. User is created (dyno1)
  2. Role (role_xyz) is created (dyno 2)
  3. User is added to role_xyz (dyno2)
  4. Object is created with read access to the role_xyz only (dyno2)
  5. Object is retrieved (dyno1) <<<<<< Object not found because dyno 1 does not know that role_xyz has the user

This case can also be solved with redis cache, despite the perf issue

Yes exactly, in-memory cache is out of the question for a cluster environment

@georgesjamous
Copy link
Contributor

georgesjamous commented Mar 4, 2019

ok, Just thinking out loud here... If you are sure that the "RedisCacheAdapter" is the problem and not something else...

I was curious about how using the "RedisCacheAdapter" could cause performance issues if the Redis cluster and network can handle the load. I looked through the code of the adapter and found that all requests to Redis are being queued up and executed one by one. I don't know exactly what is the purpose of this, maybe to keep cache as consistent as possible. (chain operations) But if there is a capping problem, it must be in the adapter itself.

this.p = this.p.then(() => {

I suggest you try something out.
Use a custom Redis adapter, but remove the this.p.then().. allowing all request to go through as they come, but use a custom rate limiter, limiting a total of x consecutive requests to the cache.

Be aware, this may cause some inconsistent data since a Del request invoked before a Set but evaluated after, results in no data cached. But this could be solved by creating a Promise chain per cache key. That way all request for a "User:123456" would be chained up, this uses a greater amount of memory though. Worth a try.

There is something like client.command_queue_length documented here clientcommand_queue_length but I have never used it, or you can create a custom counter.

@saulogt
Copy link
Contributor Author

saulogt commented Mar 4, 2019

Interesting idea.
Maybe the memory usage problem can be minimized by combining this per-key-chain with a lru cache

https://github.com/saulogt/custom-redis-cache-adapter/blob/master/CustomRedisCacheAdapter.ts

@georgesjamous
Copy link
Contributor

georgesjamous commented Mar 5, 2019

@saulogt actually there is a problem in your code, the promise is executed as soon as it is initialized, no chaining is done on a key basis.
new Promise<any>(resolve => { executes as soon as it is created
https://github.com/saulogt/custom-redis-cache-adapter/blob/6a592c7a678fcd598187247bbedee1e493c17942/CustomRedisCacheAdapter.ts#L42

Check this sample I created just to give you an idea...I haven't tested it
https://gist.github.com/georgesjamous/560802de39cfd0c46a35f2af5cf0aa4a

@saulogt
Copy link
Contributor Author

saulogt commented Mar 5, 2019

@georgesjamous , you are totally right.
Thank you for your review

@mohammed-alsiddeeq
Copy link

mohammed-alsiddeeq commented Mar 7, 2019

Your problem solved @saulogt ?

@saulogt
Copy link
Contributor Author

saulogt commented Mar 13, 2019

Hey @georgesjamous !
Thank you for your help. I'm using this custom adapter https://github.com/saulogt/custom-redis-cache-adapter

I hope to see a solution like this integrated to the this repo

The timeouts still happen sometimes, but they don't crash the server. I didn't have time to debug them yet.

@georgesjamous
Copy link
Contributor

georgesjamous commented Mar 14, 2019

@saulogt I'm glad that this worked for you.
So this solution maintains the data consistency by chaining the promises together on a key-basis and improves the performance by not having unrelated keys waiting for each other.

I will try to open-up a pull request modifying the default Redis Cache Adapter to reflect our modifications.

The timeouts need to be debugged in another ticket since we are not sure yet if its an adapter problem

@TomWFox
Copy link
Contributor

TomWFox commented Apr 3, 2019

Can we close this now?

@dplewis
Copy link
Member

dplewis commented Apr 3, 2019

Closing via #5420

@dplewis dplewis closed this as completed Apr 3, 2019
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

No branches or pull requests

5 participants