-
Notifications
You must be signed in to change notification settings - Fork 2.8k
multiplexed subscriptions #1934
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
Conversation
Deploy preview for hasura-docs ready! Built with commit 8d9c88f |
Review app for commit 7f676fc deployed to Heroku: https://hge-ci-pull-1934.herokuapp.com |
Review app https://hge-ci-pull-1934.herokuapp.com is deleted |
Review app for commit 300ff79 deployed to Heroku: https://hge-ci-pull-1934.herokuapp.com |
Review app for commit febe8b1 deployed to Heroku: https://hge-ci-pull-1934.herokuapp.com |
Review app for commit 2a85c02 deployed to Heroku: https://hge-ci-pull-1934.herokuapp.com |
Review app for commit e7bac10 deployed to Heroku: https://hge-ci-pull-1934.herokuapp.com |
Review app for commit 11b3a6d deployed to Heroku: https://hge-ci-pull-1934.herokuapp.com |
Review app for commit 0ecfd6c deployed to Heroku: https://hge-ci-pull-1934.herokuapp.com |
Review app for commit f3dd18d deployed to Heroku: https://hge-ci-pull-1934.herokuapp.com |
Review app for commit 3818b1f deployed to Heroku: https://hge-ci-pull-1934.herokuapp.com |
Review app for commit 117d178 deployed to Heroku: https://hge-ci-pull-1934.herokuapp.com |
Review app for commit e937caf deployed to Heroku: https://hge-ci-pull-1934.herokuapp.com |
This will help when we add caching of frequently used ASTs
Previously the value was always "current_setting('hasura.user')"
Review app for commit 6ba520f deployed to Heroku: https://hge-ci-pull-1934.herokuapp.com |
Review app https://hge-ci-pull-1934.herokuapp.com is deleted |
Review app for commit 8d9c88f deployed to Heroku: https://hge-ci-pull-1934.herokuapp.com |
* add types to represent unparsed http gql requests This will help when we add caching of frequently used ASTs * query plan caching * move livequery to execute * add multiplexed module * session variable can be customised depending on the context Previously the value was always "current_setting('hasura.user')" * get rid of typemap requirement in reusable plan * subscriptions are multiplexed when possible * use lazytx for introspection to avoid acquiring a pg connection * refactor to make execute a completely decoupled module * don't issue a transaction for a query * don't use current setting for explained sql * move postgres related types to a different module * validate variableValues on postgres before multiplexing subs * don't user current_setting for queries over ws * plan_cache is only visible when developer flag is enabled * introduce 'batch size' when multiplexing subscriptions * bump stackage to 13.16 * fix schema_stitching test case error code * store hashes instead of actual responses for subscriptions * internal api to dump subscriptions state * remove PlanCache from SchemaCacheRef * allow live query options to be configured on server startup * capture metrics for multiplexed subscriptions * more metrics captured for multiplexed subs * switch to tvar based hashmap for faster snapshotting * livequery modules do not expose internal details * fix typo in live query env vars * switch to hasura's pg-client-hs
Query plan caching was introduced by - I believe - #1934 in order to reduce the query response latency. During the development of PDV in #4111, it was found out that the new architecture (for which query plan caching wasn't implemented) performed comparably to the pre-PDV architecture with caching. Hence, it was decided to leave query plan caching until some day in the future when it was deemed necessary. Well, we're in the future now, and there still isn't a convincing argument for query plan caching. So the time has come to remove some references to query plan caching from the codebase. For the most part, any code being removed would probably not be very well suited to the post-PDV architecture of query execution, so arguably not much is lost. Apart from simplifying the code, this PR will contribute towards making the GraphQL schema generation more modular, testable, and easier to profile. I'd like to eventually work towards a situation in which it's easy to generate a GraphQL schema parser *in isolation*, without being connected to a database, and then parse a GraphQL query *in isolation*, without even listening any HTTP port. It is important that both of these operations can be examined in detail, and in isolation, since they are two major performance bottlenecks, as well as phases where many important upcoming features hook into. Implementation The following have been removed: - The entirety of `server/src-lib/Hasura/GraphQL/Execute/Plan.hs` - The core phases of query parsing and execution no longer have any references to query plan caching. Note that this is not to be confused with query *response* caching, which is not affected by this PR. This includes removal of the types: - - `Opaque`, which is replaced by a tuple. Note that the old implementation was broken and did not adequately hide the constructors. - - `QueryReusability` (and the `markNotReusable` method). Notably, the implementation of the `ParseT` monad now consists of two, rather than three, monad transformers. - Cache-related tests (in `server/src-test/Hasura/CacheBoundedSpec.hs`) have been removed . - References to query plan caching in the documentation. - The `planCacheOptions` in the `TenantConfig` type class was removed. However, during parsing, unrecognized fields in the YAML config get ignored, so this does not cause a breaking change. (Confirmed manually, as well as in consultation with @sordina.) - The metrics no longer send cache hit/miss messages. There are a few places in which one can still find references to query plan caching: - We still accept the `--query-plan-cache-size` command-line option for backwards compatibility. The `HASURA_QUERY_PLAN_CACHE_SIZE` environment variable is not read. hasura/graphql-engine-mono#1815 GitOrigin-RevId: 17d92b2
`SchemaCacheVer` has been a write-only value for a while. It was introduced by #1934. PR-URL: hasura/graphql-engine-mono#9786 GitOrigin-RevId: aa9d5600d2be347ea0538903b7f41f7ed745fafc
TODO