-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Improve single schema cache #7214
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7214 +/- ##
==========================================
- Coverage 93.97% 93.90% -0.08%
==========================================
Files 179 179
Lines 13138 13152 +14
==========================================
+ Hits 12347 12350 +3
- Misses 791 802 +11
Continue to review full report at Codecov.
|
Nice!! |
@vitaly-t If you have a second can you look this over? I'm basically using Notify / Listen to get notified of changes to the Any feedback would be greatly appreciated. |
@davimacedo @SebC99 @mtrezza At this point the tests are passing (excluding flaky ones). I'm going to run a few more tests. |
0f7b4c7
to
ebf67d3
Compare
Quick update, looks like the PushController.spec and Parse.Push.spec doesn’t like this PR. Everything passes locally. I’ll try to rewrite the tests or increase the timeout. Edit: If you want to reproduce this locally setTimeout to |
The tests are passing finally, I thought I would never see it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great PR, can't wait to try this out! Just MongoDB needs an upgrade it seems to the CI check to pass.
I'm going to run this in production and report back |
Oh, but did you upgrade MongoDB? The CI check didn't pass. |
Congratulations @dplewis !!!! |
* Initial Commit * fix flaky test * temporary set ci timeout * turn off ci check * fix postgres tests * fix tests * node flaky test * remove improvements * Update SchemaPerformance.spec.js * fix tests * revert ci * Create Singleton Object * properly clear cache testing * Cleanup * remove fit * try PushController.spec * try push test rewrite * try push enqueue time * Increase test timeout * remove pg server creation test * xit push tests * more xit * remove skipped tests * Fix conflicts * reduce ci timeout * fix push tests * Revert "fix push tests" This reverts commit 05aba62. * improve initialization * fix flaky tests * xit flaky test * Update CHANGELOG.md * enable debug logs * Update LogsRouter.spec.js * create initial indexes in series * lint * horizontal scaling documentation * Update Changelog * change horizontalScaling db option * Add enableSchemaHooks option * move enableSchemaHooks to databaseOptions
I think this should be looked at closely before making a release with this PR merged, because we want to make sure there isn’t a bug in the new schema mechanism. In retrospect I think a PR of this magnitude should have been phased in instead of being a sudden breaking change, to collect feedback over some time. That speaks for the deprecation policy that we’ll apply from |
@dplewis Do you have any feedback to share yet? |
🎉 This change has been released in version 5.0.0-beta.1 |
🎉 This change has been released in version 5.0.0 |
New Pull Request Checklist
Issue Description
The Server has been known to show increasing memory usage over time.
The server keeps a copy of the schema per request. In order to make a copy it must query the database
_SCHEMA
class. To prevent an invalid schema it may query the database multiple times.Closes: #6405, #6193, #7036, #6543
Approach
Create a global schema cache instead of a cache per request. This global schema cache will be initialized the the server starts and will be updated internally. If you are running multiple instances of the server and they connect to a single database the global schema will also update by a database hook that listens to the
_SCHEMA
class.Removes
enableSchemaCache
andschemaCacheTTL
optionsAdds
horizontalScaling
option to use db hooks.This PR is a continuation of #6743 Shoutout to @SebC99 for starting the Proof of Concept. 🎉
TODOs before merging