Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Adds schema caching capabilities (5s by default) #2286

Merged
merged 10 commits into from
Jul 23, 2016
Merged

Adds schema caching capabilities (5s by default) #2286

merged 10 commits into from
Jul 23, 2016

Conversation

flovilmart
Copy link
Contributor

@flovilmart flovilmart commented Jul 14, 2016 •
edited
Loading

Adds caching of the schema.

  • configure with schemaCacheTTL, or PARSE_SERVER_SCHEMA_CACHE_TTL options.
  • 5s by default.
  • put 0s or false to disable

A new cache is created for each request, scoped with a prefixed key, so no information is leaked across requests.
This could be further optimized later on.

@ghost
Copy link

ghost commented Jul 15, 2016

@flovilmart updated the pull request.

@ghost
Copy link

ghost commented Jul 15, 2016

@flovilmart updated the pull request.

2 similar comments
@ghost
Copy link

ghost commented Jul 15, 2016

@flovilmart updated the pull request.

@ghost
Copy link

ghost commented Jul 15, 2016

@flovilmart updated the pull request.

steven-supersolid
steven-supersolid reviewed Jul 15, 2016
View reviewed changes
src/Controllers/SchemaCache.js
@@ -0,0 +1,44 @@
const CACHED_KEYS = "__CACHED_KEYS";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right :)

@ghost
Copy link

ghost commented Jul 15, 2016

@flovilmart updated the pull request.

1 similar comment
@ghost
Copy link

ghost commented Jul 15, 2016

@flovilmart updated the pull request.

@flovilmart flovilmart added this to the 2.2.17 milestone Jul 15, 2016
@drew-gross
Copy link
Contributor

Boolean parameters kinda suck, it's so hard to tell at the call site what they are for. Can you change these functions to use object-of-parameters style? reloadDate({ force }) etc.

@blacha can you look at how he is instantiating the cache adapter? I think it was designed be only be instantiated once.

@flovilmart
Copy link
Contributor Author

flovilmart commented Jul 19, 2016 •
edited
Loading

I'll update the style. Initially I didn't go for the CacheController because I needed / wanted a per request cache, and not a global one in order to minimize side effects and minimize the number of changes on the Controllers to take that into account.

@flovilmart flovilmart force-pushed the schema-caching branch 2 times, most recently from 315aad3 to a09da4b Compare July 19, 2016 14:01
@ghost
Copy link

ghost commented Jul 19, 2016

@flovilmart updated the pull request.

@ghost
Copy link

ghost commented Jul 19, 2016

@flovilmart updated the pull request.

1 similar comment
@facebook-github-bot
Copy link

@flovilmart updated the pull request.

@ghost
Copy link

ghost commented Jul 20, 2016

@flovilmart updated the pull request.

@codecov-io
Copy link

codecov-io commented Jul 20, 2016 •
edited
Loading

Current coverage is 91.83% (diff: 95.77%)

Merging #2286 into master will increase coverage by 0.02%

@@             master      #2286   diff @@
==========================================
  Files            94         95     +1   
  Lines         10479      10629   +150   
  Methods        1275       1295    +20   
  Messages          0          0          
  Branches       1713       1735    +22   
==========================================
+ Hits           9621       9761   +140   
- Misses          858        868    +10   
  Partials          0          0

Powered by Codecov. Last update ac1f762...b82d16e

@ghost
Copy link

ghost commented Jul 20, 2016

@flovilmart updated the pull request.

@flovilmart
Use CacheController instead of generator
4e8c355 
- Makes caching SchemaCache use a generated prefix
- Makes clearing the SchemaCache clear only the cached schema keys
- Enable cache by default (ttl 5s)
@flovilmart flovilmart changed the title Adds schema caching capabilities (off by default) Adds schema caching capabilities (5s by default) Jul 22, 2016
@ghost
Copy link

ghost commented Jul 22, 2016

@flovilmart updated the pull request.

@drew-gross drew-gross merged commit 09bd9e3 into master Jul 23, 2016
@drew-gross drew-gross deleted the schema-caching branch July 23, 2016 04:24
@benitech
Copy link

This sets cache for "_Role, _User and _SCHEMA".
How can i add caching for another class that i created? Possible to add the new class name somewhere in index.js?

rsouzas pushed a commit to back4app/parse-server that referenced this pull request Feb 22, 2017
@flovilmart
Adds schema caching capabilities (5s by default) (parse-community#2286)
708a180 
* Adds schema caching capabilities (off by default)

* Use InMemoryCacheAdapter

* Uses proper adapter to generate a cache

* Fix bugs when running disabled cache

* nits

* nits

* Use options object instead of boolean

* Imrpove concurrency of loadSchema

* Adds testing with SCHEMA_CACHE_ON

* Use CacheController instead of generator

- Makes caching SchemaCache use a generated prefix
- Makes clearing the SchemaCache clear only the cached schema keys
- Enable cache by default (ttl 5s)
rsouzas pushed a commit to back4app/parse-server that referenced this pull request Mar 15, 2017
@flovilmart
Adds schema caching capabilities (5s by default) (parse-community#2286)
59a37c9 
* Adds schema caching capabilities (off by default)

* Use InMemoryCacheAdapter

* Uses proper adapter to generate a cache

* Fix bugs when running disabled cache

* nits

* nits

* Use options object instead of boolean

* Imrpove concurrency of loadSchema

* Adds testing with SCHEMA_CACHE_ON

* Use CacheController instead of generator

- Makes caching SchemaCache use a generated prefix
- Makes clearing the SchemaCache clear only the cached schema keys
- Enable cache by default (ttl 5s)
rsouzas pushed a commit to back4app/parse-server that referenced this pull request Mar 16, 2017
@flovilmart
Adds schema caching capabilities (5s by default) (parse-community#2286)
cf046af 
* Adds schema caching capabilities (off by default)

* Use InMemoryCacheAdapter

* Uses proper adapter to generate a cache

* Fix bugs when running disabled cache

* nits

* nits

* Use options object instead of boolean

* Imrpove concurrency of loadSchema

* Adds testing with SCHEMA_CACHE_ON

* Use CacheController instead of generator

- Makes caching SchemaCache use a generated prefix
- Makes clearing the SchemaCache clear only the cached schema keys
- Enable cache by default (ttl 5s)
@montymxb montymxb mentioned this pull request Sep 10, 2017
Closed
@dplewis dplewis mentioned this pull request Nov 13, 2020
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@drew-gross drew-gross

Labels
None yet
Projects
None yet
Milestone
2.2.17
Development

Successfully merging this pull request may close these issues.
6 participants
@flovilmart @drew-gross @facebook-github-bot @codecov-io @benitech @steven-supersolid