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.

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

Implement raw sources functionalities to move runtime risks to build … #1522

Closed
wants to merge 3 commits into from

Conversation

santino
Copy link
Contributor

@santino santino commented Jan 31, 2021

In order to make Mesh more reliable and Production-ready, I'm proposing the following change.

The problem

Currently, Mesh handles data sources at runtime.
When invoking getMesh() this will loop through all data sources, download their original schemas (when remote, probably very likely), translates those into GraphQL, and finally apply all transforms and stitching.
This means that after building the server bundle and deploying it to Production we risk the application not being able to start because potentially one of the data sources might experience downtime; even if a temporary glitch.
Effectively this means that all data sources become a dependency to start our servers.

This makes things troublesome because we would need to restart our server, which might not still be enough even if a just single data source is still unavailable.
Effectively, at this stage, our server is unable to serve traffic and we're blocked even though our builds are successful.
In this case, even a rollback won't solve the issue; since the previous build would also attempt to download the data sources at runtime.

The solution

I've prepared this PR to allow developers to set up data sources download at build time in just 2 simple steps.

  1. Configure a directory where to store raw data sources specifications, through meshrc.yaml
  2. Use a Mesh CLI command, at build time, to download all raw data sources specs

By doing this we can fail builds, in case of data sources downtime, and so avoid promoting the build to production which would cause potential unavailability.
Effectively this makes Mesh predictable and reliable; since we removed "third-party dependencies", we can trust that our builds are always going to work.

For the first point, I have introduced a new property to Mesh config file called rawSourcesDir which is a string that can be used as follows:

rawSourcesDir: ./rawSources
sources: [...]
transforms: [...]

As for the second point, I have introduced a prepare-raw-sources command to Mesh CLI that doesn't require any argument.
This command will loop through all data sources, download the specifications for remote source only from their provider and store one file per remote data source in the given directory.
This command doesn't need any argument, since it only requires the destination directory which is read from the Mesh config file.
A build configuration could look like follow:

{
  "name": "graphql-mesh-server",
  ...
  "main": "src/index.js",
  "scripts": {
    "prepareRawSources": "graphql-mesh prepare-raw-sources",
    "build": "npm run prepareRawSources && babel src -d dist",
    ...
  },
  ...
}

Next steps

I believe this proposal is extremely valid, but I want to open a conversation before completing this.
Right now I have implemented the new functionalities on the OpenAPI and the GraphQL handlers.
All other handlers still need to be extended with the newly introduced getRawSource() method.

Included

I have introduced new types; updated existing types and added a new page to the Documentation website.

…time

currently fully implemented on OpenAPI handler only
@changeset-bot
Copy link

changeset-bot bot commented Jan 31, 2021

⚠️ No Changeset found

Latest commit: c3dc8af

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@santino
Copy link
Contributor Author

santino commented Feb 7, 2021

Hello folks,
any chance you would be willing to come back to me on this PR?

I appreciate your time availability, hence why I have been patiently waiting for a week before chasing; but I would be grateful if you could review this and give me some feedback.

I am keen on implementing this in my project and I would like to have an expectation whether or not you're willing to bake this functionality into Mesh as per my proposal; otherwise, I will have to create some custom scripts to handle build-time download of data sources definitions directly within my project.

cc @Urigo, @ardatan; hope you don't mind me tagging you directly 🙏

@ardatan
Copy link
Owner

ardatan commented Feb 8, 2021

Sorry for late response! Wow, thank you so much for this!
I have some thoughts on this;
Actually the idea seems having an initial cache that can be created once immediately after schema generation phase. For example, we can create this initial cache by running a command like you already implemented before production then this initial cache will have all raw sources statically then everytime we run mesh with this initial cache it will use those already downloaded sources quickly instead of downloading it.
So what if we have something like this;

  • A config option that points to the path for the file that contains initial cache.
  • A command that creates this initial cache; yarn mesh generate-initial-cache which overrides TTL for everything is done during schema generation phase
  • Everytime Mesh is started, initial cache is read from the file then written to the selected cache impl(inmemory lru by default)
  • Some necessary modifications that respect initial cache more

@santino
Copy link
Contributor Author

santino commented Feb 8, 2021

Are you thinking about generatig a cache with the actual generated schema or just raw source as per my PR?

If you're thinking to cache the generated GraphQL schema; how would that work in terms of becoming executable and being hydrated with resolvers, subscribers, etc?

If you remember one of the conversations we had, I was mentioning how I was impressed with SwagQL since it does generate static JS files that can be just executed at runtime.
A similar setup would be amazing, but from what I understand with existing Mesh and GraphQL-Tools; this is probably not achievable.

Hence why, in this case, I went to create a proposal that does save raw sources specifications with an explicit process/command that Mesh can use at runtime without really altering existing executable schema capabilities of getMeshSource().

@ardatan
Copy link
Owner

ardatan commented Feb 9, 2021

We are thinking of seperating execution and schema generation logic in Mesh that will speed up the bootstrap instead of code generation like SwagQL. @dotansimha and me discussed a lot about it before.
Before all of those, you can see we put static urls and stuff like swagger files, graphql introspection files etc in cache (by respecting HTTP headers) but we can force those to be cached once and saved in a file like you implemented instead of having a different layer that collects raw sources. And this will keep the abstraction because some handlers might need different logic to collect and restore raw sources like Postgraphile handler.
Like you implemented, yarn generate-initial-cache can generate the Mesh schema like it does in other commands by forcing the stuff to be cached on schema generation phase and save it in a file with @graphql-mesh/cache-file. This file can be defined inside .meshrc.yml

initialCache: ./initial-cache.json

Then on regular bootstrap, Mesh can check that file and restores the cache onto actual cache then generates schema without downloading and recollecting raw sources in the same way you implemented.

@santino
Copy link
Contributor Author

santino commented Feb 9, 2021

We are thinking of seperating execution and schema generation logic in Mesh that will speed up the bootstrap

This sounds great; bootstrap can definitely be improved

we can force those to be cached once and saved in a file

Yes, the idea is fine; I guess if we go towards this approach I would be concerned with how the cache would be handled. How long does it live? When does it get cached?
Not sure if a command to potentially purge the cache would be a good addition to have the assurance that you're not using stale cache as opposed to a fresh one.
This might not be an issue in relation to a deployment; you deploy on a server and at the time you invoke yarn generate-initial-cache no cache will be there, so you would be building it from scratch.
Then when executing getMesh() this will access the cache saved in a static file.

In this regard, this is similar to the approach I was proposing.
Probably a bit less explicit; with my approach, the "cached" sources would actually be part of the build.
Maybe the purge cache command would be more relevant with this approach; thinking in use-cases like rolling back f.i.; where your build does not include the cached sources and you don't want to risk serving from cached data which might refer to a different version of a REST API f.i.

instead of having a different layer that collects raw sources

Yes, I agree this is the bit I wasn't 100% happy about.
However, the idea is that if you want to only generate the static cache you still need a command that does only that; and until you do separate execution and schema generation you cannot currently do that.
You wouldn't want to run a full getMesh() so that you can just save the cache; you would introduce overhead to that step as well.

some handlers might need different logic to collect and restore raw sources

Yes, I realised this when implementing rawSources on the GraphQL handler; in fact, I ended up re-using the logic built within getMeshSource up to the point where that then deals with executable.


Having said all this, how do you want to move forward?
How far are you from separating executable and schema generation logic?
I bet that's going to be a significant change.

Until that is done I don't see a good way of implementing something like the cache you are proposing; unless in order to build the static cache you want to run the full getMesh(); which would probably be less ideal in terms of overhead and performance, compared to the dedicated getRawSource().

@ardatan ardatan force-pushed the master branch 2 times, most recently from 01600cb to 5ae2997 Compare February 11, 2021 22:49
@dotansimha
Copy link
Collaborator

Hi @santino , thank you so much for you contributions.

First, we do want to have a better way to validate the eventual GraphQL schema produced by GraphQL-Mesh.

I'll break my comment/review to 2 parts here: short-term and long-term.

In the short-term, I think we need a dead-simple cache mechanism, managed by the Mesh core/runtime/cli, that allow handlers to persist data in cache and get it. That means, we can create a very simple cache object (with persistence) in the Mesh core, and share it with the handlers. This way, handlers that supports it can just use it as save their upstream schemas.
We can also introduce a flag for clearing that cache (and setting the location of it), leading to re-loading of the streams by the handlers. I think this kind of solution might be the best way to reloading the schemas every time, and also make sure that we are able to load schema that were loaded before (by setting the cache directory manually).
Do you think this is something that can solve your issues as well? If so, Do you wish to do that as part of your PR? (we can also implement it)

In the long-term, we have a bigger plan to refactor GraphQL-Mesh and break it to 2 parts: schema creation (that occurs in build time) and schema execution (runtime). This way we'll be able to maintain a better compiled sources, have validations on it and cache it.
It also related to the other 2 PRs you created (for adding bare), since my optimal solution here is to have a non-GraphQL instance of each handler, and then to build the schema based on the original upstream, and the changes provided by the developer, and produce a single schema, without any wrapping, that just knows how each field is being resolved on each handler.

@santino
Copy link
Contributor Author

santino commented Feb 18, 2021

Hey Dotan,
thanks for your comment.

The cache mechanism you're describing is not much different from what I proposed in this PR.
I believe it can be achieved, somehow simply, by taking advantage of the existing getMesh which can then be "destroyed". My point was that getMesh does a lot more than required to build some cache, but I guess it'll work for the time being and so until you move to the long-term solution.

I am happy to look into this however, I see myself fairly busy in the next couple of weeks; just to give some expectations. Feel free to jump on it sooner if you wish to handle it on your side earlier.

The long-term plan sounds good to me and I am particularly happy you're considering reducing the pattern of the "wrapping schema" approach since it does have several downsides, as I highlighted a few times.

As for my other PRs for introducing bare mode on transforms, I do hope you're willing to merge those.
The long-term solution will require months and especially one of the two PRs is really a blocker for me. And in any case, being conscious of how "wrapping" works, I wouldn't want to go to Prod with that; especially given I'm proposing what seems to be a good and easy alternative.
I won't insist on that here, feel free to comment on those PRs; I would love to have an update for them.

In the meantime, I'm going to close this PR so that CI resources are released.
Will open a new one when I have time to look into simple caching as discussed; unless you do that before me.

@santino santino closed this Feb 18, 2021
@santino santino deleted the raw-sources branch March 25, 2021 09:20
klippx pushed a commit to klippx/graphql-mesh that referenced this pull request Oct 9, 2024
* feat: non-breaking GraphQL Engine

* feat: make envelop take in engine functions

* structural typings

* remove grapqhl as peer dep from types pkg

* make core more agnostic

* chore(dependencies): updated changesets for modified dependencies

* remove traced schema

* remove traced orchestrator

* make plugin agnostic

* drop EnvelopError

* remove more graphql import

* Drop useTiming

* make core completely free of graphql-js

* add eslint rule

* eslint disallow in types too

* more agnostic packages

* remove introspection util

* chore(dependencies): updated changesets for modified dependencies

* prettier

* TEMP: make bot calm down

* update all docs

* test matrix for core

* experimenting traced schema (ardatan#1501)

* experimenting traced schema

* Fix

* remove comment

* do optional chaining since we are not strongly typing

* document

* cleanup

Co-authored-by: Arda TANRIKULU <ardatanrikulu@gmail.com>

* feat: drop node 12 (ardatan#1505)

* feat: remove `enableIf` utility (ardatan#1504)

* feat: remove enableIf utility

* make types happpy

* make types happy

* add changeset

* feat: remove async schema plugin and rename lazy loaded schema plugin (ardatan#1506)

* remove useAsyncSchema

* rename to useSchemaByContext

* add eslint rule (ardatan#1509)

* feat: `@envelop/on-resolve` plugin for hooking into schema resolvers (ardatan#1500)

* on resolve plugin

* changeset

* no more onResolverCalled

* unused import

* args is a record

* integrate and useOnResolve

* resolversHooksSymbol does not exist

* plugincontext for OnPluginInit

* on-resolve uses addPlugin

* onresolvercalled is no more

* refactor for new on-resolve

* fix open-telemetry tests

* fix newrelic

* opentelemetry graphql as peer dep

* tests

* addPlugin doesnt need to be used

* reorder

* respects onPluginInit context

* drop unused import

* fixes false positive eslint warnings

Co-authored-by: Dimitri POSTOLOV <dmytropostolov@gmail.com>

* docs: order of plugins matter (ardatan#1513)

* feat: remove handler for validation and parse errors (ardatan#1510)

* feat: remove hanlder for validation and parse errors

* tests

* make it work

* Add docs

* Fix serialization issue

* Go

* ..

* update docs

* update test

* make it work

* feat: add originalError in dev mode (ardatan#1514)

* remove Fn appendix

* name graphql error

* make ts happy

* make toJSON required

Co-authored-by: Arda TANRIKULU <ardatanrikulu@gmail.com>
Co-authored-by: Laurin Quast <laurinquast@googlemail.com>

* add changeset

* Update .changeset/nervous-seas-own.md

* Update .changeset/rude-cats-peel.md

* no-use-before-define (ardatan#1522)

* feat: trigger on context, validate and parse errors (ardatan#1511)

* feat: trigger on context, validate and parse errors

* trying

* make it work

* pass in phase

* add phase details

* feedback

* docs: migration guide (ardatan#1520)

* docs: migration guide

* remove slashes

* Update website/docs/guides/migrating-from-v2-to-v3.mdx

Co-authored-by: Denis Badurina <badurinadenis@gmail.com>

* feedback

* document removing of orchestrated tracer

* more feedback

* update docs

* update examples

* async schema example

Co-authored-by: Denis Badurina <badurinadenis@gmail.com>

* Add redirects

* remove deafult skip error from sentry plugin

* export masked error plugin

* sentry plugin default skip GraphQLError

* sentry: og error is not graphql error then send to sentry

* doc: drop introspection utils

* should fix ts issues on v15

* test: stack error we should not match the error name since it can be different across impl

* feat: use engine plugin (ardatan#1535)

* feat: use engine plugin

* remove assertion

* make useEngine plugin the only way to pass engine functions (ardatan#1536)

* make useEngine plugin the only way to pass engine fns

* update types

* update docs

* Update packages/plugins/apollo-datasources/README.md

Co-authored-by: Arda TANRIKULU <ardatanrikulu@gmail.com>

* prettier:

Co-authored-by: Arda TANRIKULU <ardatanrikulu@gmail.com>

Co-authored-by: Arda TANRIKULU <ardatanrikulu@gmail.com>

* fix

* versioned docs with v3 default

* fix build

* fix

* feat this is cool

* docs reword schema tracing

* docs add graphql error example

* docs restructuring, fix typos

* chore remove autogenerated changeset

* cleanup changeset

* docs sycn patch

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Arda TANRIKULU <ardatanrikulu@gmail.com>
Co-authored-by: Denis Badurina <denis@domonda.com>
Co-authored-by: Dimitri POSTOLOV <dmytropostolov@gmail.com>
Co-authored-by: Laurin Quast <laurinquast@googlemail.com>
Co-authored-by: Denis Badurina <badurinadenis@gmail.com>
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

Successfully merging this pull request may close these issues.

3 participants