这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@arvi3411301
Copy link
Member

Description

Optimize migrate API which was slowing migrate requests from console

Affected components

  • CLI

Related Issues

Solution and Design

Steps to test and verify

Limitations, known bugs & workarounds

@arvi3411301 arvi3411301 added k/enhancement New feature or improve an existing feature c/cli Related to CLI labels Sep 17, 2019
@netlify
Copy link

netlify bot commented Sep 17, 2019

Deploy preview for hasura-docs ready!

Built with commit 43bfff7

https://deploy-preview-2895--hasura-docs.netlify.com

@hasura-bot
Copy link
Contributor

Review app for commit ee9f514 deployed to Heroku: https://hge-ci-pull-2895.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2895-ee9f5147

@hasura-bot
Copy link
Contributor

Review app for commit 4a75e6c deployed to Heroku: https://hge-ci-pull-2895.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2895-4a75e6ca

@hasura-bot
Copy link
Contributor

Review app for commit 43bfff7 deployed to Heroku: https://hge-ci-pull-2895.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2895-43bfff79

Copy link
Member

@shahidhk shahidhk left a comment

Choose a reason for hiding this comment

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

LGTM

@shahidhk shahidhk changed the title optimize migrate api calls which was slowing migrate requests from console optimise migrate api for console Sep 18, 2019
@shahidhk shahidhk changed the title optimise migrate api for console cli: optimise migrate api for console Sep 18, 2019
@shahidhk shahidhk changed the title cli: optimise migrate api for console optimise migrate api for console on cli Sep 18, 2019
@shahidhk shahidhk merged commit 5f3294f into hasura:master Sep 18, 2019
@hasura-bot
Copy link
Contributor

Review app https://hge-ci-pull-2895.herokuapp.com is deleted

@shahidhk shahidhk deleted the imporve-cli-migrate-api-performance branch September 18, 2019 05:36
polRk pushed a commit to polRk/graphql-engine that referenced this pull request Feb 12, 2020
hasura-bot pushed a commit that referenced this pull request Dec 1, 2021
GraphQL types can refer to each other in a circular way. The PDV framework used to use values of type `Unique` to recognize two fragments of GraphQL schema as being the same instance. Internally, this is based on `Data.Unique` from the `base` package, which simply increases a counter on every creation of a `Unique` object.

**NB**: The `Unique` values are _not_ used for knot tying the schema combinators themselves (i.e. `Parser`s). The knot tying for `Parser`s is purely based on keys provided to `memoizeOn`. The `Unique` values are _only_ used to recognize two pieces of GraphQL _schema_ as being identical. Originally, the idea was that this would help us with a perfectly correct identification of GraphQL types. But this fully correct equality checking of GraphQL types was never implemented, and does not seem to be necessary to prevent bugs.

Specifically, these `Unique` values are stored as part of `data Definition a`, which specifies a part of our internal abstract syntax tree for the GraphQL types that we expose. The `Unique` values get initialized by the `SchemaT` effect.

In #2894 and #2895, we are experimenting with how (parts of) the GraphQL types can be hidden behind certain permission predicates. This would allow a single GraphQL schema in memory to serve all roles, implementing #2711. The permission predicates get evaluated at query parsing time when we know what role is doing a certain request, thus outputting the correct GraphQL types for that role.

If the approach of #2895 is followed, then the `Definition` objects, and thus the `Unique` values, would be hidden behind the permission predicates. Since the permission predicates are evaluated only after the schema is already supposed to be built, this means that the permission predicates would prevent us from initializing the `Unique` values, rendering them useless.

The simplest remedy to this is to remove our usage of `Unique` altogether from the GraphQL schema and schema combinators. It doesn't serve a functional purpose, doesn't prevent bugs, and requires extra bookkeeping.

PR-URL: hasura/graphql-engine-mono#2980
GitOrigin-RevId: 50d3f9e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c/cli Related to CLI k/enhancement New feature or improve an existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants