WIP: Reduces number of calls to _SCHEMA

[flovilmart]

This is an attempt to reduce the number of schema calls we make to the database, invalidating it once per request.

[codecov-io]

Current coverage is 92.81%

Merging #1387 into master will decrease coverage by -0.02% as of 3bac3b6

@@            master   #1387   diff @@
======================================
  Files           86      86       
  Stmts         5443    5439     -4
  Branches      1005    1005       
  Methods          0       0       
======================================
- Hit           5053    5048     -5
  Partial         10      10       
- Missed         380     381     +1

Review entire Coverage Diff as of 3bac3b6

Powered by Codecov. Updated on successful CI builds.

[blacha]

I don't think you should delete this promise, as it is a outstanding request to refresh the schema, This would actually make performance worse.

this function should just be delete this._schema

[flovilmart]

We can get in a lock if we don't delete the promise and only the schema

[blacha]

You would still need to delete this.schemaPromise in the _loadSchema function similar to how it was originally

Compare these two scenarios where the server gets two requests (R1,R2) in quick succession, Say a initial page load that does couple queries

With the setup in this pull request

R1.invalidateSchema
R1._loadSchema
R1.schemaCollection // creates schemaPromise (Queries _SCHEMA)
R2.invalidateSchema // deletes schemaPromise before it has even returned
R2._loadSchema
R2.schemaCollection // creates schemaPromise (Queries _SCHEMA)
// R1.schemaCollection query resolves
// R2.schemaCollection query resolves

// This would result in two concurrent _SCHEMA queries

If you moved the delete this.scehamPromise into the _loadSchema

R1.invalidateSchema
R1._loadSchema
R1.schemaCollection // creates schemaPromise (Queries _SCHEMA)
R2.invalidateSchema // doesnt delete schemaPromise
R2._loadSchema
// R1.schemaCollection resolves
// R1 deletes schemaPromise
// R2 resolves from R1.schemaCollection resolution

// This would result in only one _SCHEMA query

[flovilmart]

very very good point!

[facebook-github-bot]

@flovilmart updated the pull request.

[blacha]

Could we lock the _SCHEMA when the server starts in production mode?

Do people need to be able add/remove fields on the fly in production mode?

If we could lock it, then It would only be one _SCHEMA call when the server starts. In dev mode the _SCHEMA could be queried before every request

[steven-supersolid]

We do currently change the schema without server restarts, e.g.1: preparing an updated schema for an upcoming deploy without affecting the live service. E.g.2: performing cleanup on the schema (removing old fields) while the service is running.

I'm happy with the idea of the original PR and wouldn't want optimisation to reduce capabilities we currently enjoy. It will be worth profiling this change.

[flovilmart]

Do people need to be able add/remove fields on the fly in production mode?

by default NODE_ENV=production is set on heroku or other servers, implementing that way relying on the production flag would render many deployments faulty in the eyes of many users.

[facebook-github-bot]

@flovilmart updated the pull request.

[drew-gross]

This is pretty old now, needs a rebase, and I think is obviated #1664. Feel free to reopen if you want.