Adding Caching Adapter, allows caching of _Role, _User and _SCHMEA (fixes #168)

[blacha]

No description provided.

[ghost]

@blacha updated the pull request.

[facebook-github-bot]

@blacha updated the pull request.

[jcnm]

Anyway to override this value with an ENV variable ou CLI option or Config value ?

[blacha]

You can create a InMemoryCacheAdapter with any TTL you want when you initialize the server.

new ParseServer({
    cacheAdapter: new InMemoryCacheAdapter({ttl: 24 * 60 * 60 * 1000}) 
})

or you can completely disable it by having a cacheAdapter that just Promise.resolve(null) for gets/sets/del/clear

The TTL is only for Sessions, Roles and SCHEMA queries

Also any time a user modifies a Role or a Session the cache for those get destroyed.

[flovilmart]

I'd still provide an option/env in the CLI.

[codecov-io]

Current coverage is 92.40%

Merging #1664 into master will increase coverage by +0.14%

1. 3 files in src were modified. more
   • Misses +1
   • Hits -1
2. 2 files (not in diff) in src/Routers were modified. more
   • Misses +11
   • Hits -17
3. 2 files (not in diff) in ...apters/Storage/Mongo were modified. more
   • Misses -16
   • Hits -18
4. 1 files (not in diff) in src were modified. more
   • Misses -3
   • Hits -6

@@             master      #1664   diff @@
==========================================
  Files            87         91     +4   
  Lines          6201       6240    +39   
  Methods        1070       1083    +13   
  Messages          0          0          
  Branches       1289       1269    -20   
==========================================
+ Hits           5721       5766    +45   
+ Misses          480        474     -6   
  Partials          0          0          

Powered by Codecov. Last updated by 19e7407...e2928a1

[Salakar]

@blacha not trying to be pedantic 🙈 but any reason you're calling the set cache method put?

Would it make more sense to call it set, to match Redis, Memcache, object getter/setter etc method names?

[gfosco]

@flovilmart or @nlutsenko any thoughts here?

[blacha]

@Salakar I based the implementation off library that I've used in the past and that used put I think your right set would be a better name as Redis/Memcache is what I want this cache adapter to be backed by eventually.

Ill update this pull request when I get home from work.

[ghost]

@blacha updated the pull request.

[drew-gross]

A couple comments:

1. This cache adapter seems to have a contract that the promise returned by get must never reject. Are you sure thats what we want? I could see arguments on both sides.

2. I'd rather have no cache by default. I don't want people to get surprised by cache-related inconsistencies when they start running multiple parse servers.

3. Needs a rebase.

[blacha]

@drew-gross

1. I think rejecting should be left for serious errors in the adapter (Failed to connect to Redis/memcache/, Out of memory etc..), we could in theory capture the rejections in the controller then Log the error and resolve a null (not cached).
2. No cache is also possible, The only reason I had it caching by default was that it is replacing something that was already caching (Sessions I think), so if we disable it here it will be a performance regression until people update their configs. Another option would be a lower TTL? 5 - 30 seconds?
   • The cache is also "cleared" if anyone updates the _Role collection. so unless someone is manually changing the database they shouldn't notice any stale cached _Roles.
3. Will do, after we decide on default to: No Cache or Lower TTL.

[drew-gross]

1. Sure, the question is whether we want to fail the whole request when only the cache is down. Some applications will be able to handle the load with higher response times when the cache is down, some will fall over. I think failing the whole request is reasonable, but we should do something to at least help the user figure out whats wrong. Maybe later we can have the server let you choose whether to fail the whole request or load from the backing store.
2. Fair enough. I think 5s is a reasonable default TTL. Long enough that things will usually remain in the cache for multiple requests that are triggered by a single user action, short enough that you can usually save, refresh once or twice, hit a different server, and see the new data.
3. 👍

[flovilmart]

Should we handle value == null as a delete?

[ghost]

@blacha updated the pull request.

[ghost]

@blacha updated the pull request.

[gnz00]

Here is another implementation for reference: https://github.com/maysale01/parse-server/blob/new-memory-cache/src/classes/MemoryCache.js