server: heterogeneous execution of GraphQL queries #5869

abooij · 2020-09-30T09:26:08Z

Description

The server could already execute queries that either fetch data from the database, or, through remotes, from other GraphQL servers. This PR also enables mixing such data sources within one query. So one can have queries such as

query {
  articles {
    title
  }
  weather {
    temperature
  }
}

where the articles are fetched from the database, and the weather is fetched from a remote server.

Changelog

CHANGELOG.md is updated with user-facing content relevant to this PR. If no changelog is required, then add the no-changelog-required label.

Affected components

Server
Tests

Related Issues

This PR depends on #5865.

Solution and Design

For an incoming query we already used to generate an "execution plan" which specified how to execute that query. However, so far an execution plan has been a single execution step, where an execution step can be of database-type, of remote-type or of "raw"-type (used for introspection). This PR changes the type of execution plans to be an insertion-ordered hashmap of execution steps

type ExecutionPlan db remote raw = InsOrdHashMap Text (ExecutionStep db remote raw)

where the key corresponds to a root field name.

Steps to test and verify

A test suite is included.

Limitations, known bugs & workarounds

Although this PR allows to mix root fields originating from different data sources, it does not allow joining between them arbitrarily beyond the existing "remote joins" that load relationships from Postgres to a remote. Such "generalized joins" are part of future work.

Server checklist

Catalog upgrade

Does this PR change Hasura Catalog version?

No

Metadata

Does this PR add a new Metadata feature?

No

GraphQL

No new GraphQL schema is generated

Breaking changes

No Breaking changes

The "locality" field needs to get additional options. This commit takes an opinionated view on this which we should discuss further.

Co-authored-by: Alexis King <lexi.lambda@gmail.com>

abooij · 2020-10-06T09:23:38Z

@lexi-lambda Thank you very much for the thorough review!

lexi-lambda

Many thanks for the prompt changes; I think this PR generally looks good to me now. I think it would be nice to find some way to deduplicate all the tricky query execution logic and parameterize it over the transport mechanism, but I’m happy to have a call to discuss that, since it seems possibly subtle.

lexi-lambda · 2020-10-06T11:15:41Z

server/src-lib/Hasura/GraphQL/Transport/HTTP/Protocol.hs

-  | GQPreExecError ![J.Value]
-  | GQExecError ![J.Value]
-  deriving (Show, Eq, Functor, Foldable, Traversable)
+type GQResult a = ExceptT GQExecError Identity a


ExceptT over Identity is basically just an awkward version of Either. Is there a reason we need ExceptT here specifically?

lexi-lambda · 2020-10-06T11:20:10Z

server/src-lib/Hasura/GraphQL/Transport/HTTP.hs

-  -- spent in the PG query; for telemetry.
-runQueryDB reqId (query, queryParsed) asts _userInfo (tx, genSql) =  do
+  -> Text
+  -> (Tracing.TraceT (LazyTx QErr) EncJSON, Maybe EQ.GeneratedSqlMap)


Do we ever actually call logQueryLog with a map containing more than one entry in it? If not, I’d be in favor of just changing the signature—I don’t think avoiding the tiny bit of extra downstream effort to update to the new interface is worth the misleading API. (Of course, if we do call it with more than one value somewhere, please point me to it!)

lexi-lambda · 2020-10-06T11:35:48Z

server/src-lib/Hasura/GraphQL/Execute/Query.hs

+          prepArgs = fst <$> IntMap.elems args
+      in (, Just ps) $ case remoteJoinsM of
+           Nothing -> do
+             Tracing.trace "Postgres" . (runExtractProfile ep =<<) . liftTx $ asSingleRowJsonResp (instrument q) prepArgs


This is another instance of an operator section involving =<<, which I missed on the first pass, but I personally find confusing. There’s a lot going on all on one line here. I recognize this kind of thing is subjective, but I happen to prefer a formulation like this:

Suggested change

Tracing.trace "Postgres" . (runExtractProfile ep =<<) . liftTx $ asSingleRowJsonResp (instrument q) prepArgs

Tracing.trace "Postgres" $ runExtractProfile ep =<< liftTx do

asSingleRowJsonResp (instrument q) prepArgs

This puts all the “wrapping bits” on a separate line from the actual interesting functionality being executed. Another approach I use sometimes when the “wrapping bits” get complicated is something like this:

Suggested change

Tracing.trace "Postgres" . (runExtractProfile ep =<<) . liftTx $ asSingleRowJsonResp (instrument q) prepArgs

asSingleRowJsonResp (instrument q) prepArgs

& liftTx

>>= runExtractProfile ep

& Tracing.trace "Postgres"

Which has sort of a “pipeline” flavor to it. This works out nicely because & and >>= are both infixl 1. But in this particular case, I’m not sure that it’s actually better; most of the parts of the “pipeline” are pretty boring.

tirumaraiselvan

changelog

Auke Booij and others added 30 commits September 16, 2020 15:46

[skip ci] build up the encjson from different queries later

76d5c41

[skip ci] make an execution plan into a map (broken state)

2655535

[skip ci] make HTTP transport buildable

af934dd

[skip ci] make websocket transport buildable

e961203

[skip ci] write broken implementation of mutations over HTTP

6e22848

[skip ci] enable different remote execution steps

510a751

[skip ci] clean execution plan creation phase

2114ea9

[skip ci] rebuild websocket execution

cf9e9da

[skip ci] make execution plans ordered; fix shape of json result

7e56e75

[skip ci] execute introspection as raw execution steps instead of db

f050555

[skip ci] avoid lists in query execution plan building

8706003

[skip ci] thread field name throughout websocket execution

63ca880

[skip ci] fix results ordering

3528d24

[skip ci] allow heterogeneous mutations

1d36c1e

[skip ci] avoid explicit cast to Text in asObject

40d11a1

[skip ci] extract field data from remote JSON call

6c55718

[skip ci] WebSocket: fully collect results before sending data

b470782

[skip ci] document code that needs to be improved

484bc4f

Prepare for sending telemetry. Switched off for now.

d684f6d

The "locality" field needs to get additional options. This commit takes an opinionated view on this which we should discuss further.

fix build error

16b2f36

only execute websocket mutations until their first failure

d368928

Support sending telemetry from websocket transport

284bce3

Fix some remote-websocket tests

553ccae

Change GQResult into a type synonym for ExceptT

a7c4d74

Improve remote field extraction for websockets

cf17b5b

Simplifying some code

d280612

clean up Execute/Query.hs

f14d392

fix query code deduplication

5c457d9

fix the query that's sent to a remote

9e14819

simplify http transport code somewhat

da5f190

Auke Booij and others added 11 commits October 6, 2020 09:13

Use more parentheses more straightforwardly

f205ea6

Co-authored-by: Alexis King <lexi.lambda@gmail.com>

Improve docstring for GeneratedSqlMap

df29bc1

Co-authored-by: Alexis King <lexi.lambda@gmail.com>

Use fewer type variables in ExecutionStep

b74825f

[skip ci] Make TODO into WARNING

2193ecf

Use telemCacheHit from getResolvedExecPlan in HTTP transport

b2192c2

Use telemCacheHit from getResolvedExecPlan in WebSocket transport

cb1cc5f

Rename buildTypedOperation to buildExecStepRemote

439a943

Make runQueryDB take a single PreparedSql rather than a map

cdb90f0

Pass execution steps by G.Name to avoid G.unsafeMkName

0805b14

Rename rjCtx to remoteJoinCtx

6e4c082

Actually send telemetry

a3b884c

abooij requested a review from lexi-lambda October 6, 2020 08:17

Merge branch 'master' into heterogeneous-execution-new-MonadExecuteQuery

eeebc37

lexi-lambda suggested changes Oct 6, 2020

View reviewed changes

nicuveo mentioned this pull request Oct 6, 2020

server: redesign MonadExecuteQuery in preparation for heterogeneous execution #5865

Merged

Antoine Leblanc added 3 commits October 6, 2020 15:22

change GQResult to use Either

0f154d7

applied suggestion

aeb6251

slightly refactor MonadQueryLog to avoid creating a hash map

1e46495

lexi-lambda approved these changes Oct 6, 2020

View reviewed changes

abooij requested a review from tirumaraiselvan October 7, 2020 07:58

tirumaraiselvan approved these changes Oct 7, 2020

View reviewed changes

Auke Booij added 2 commits October 7, 2020 11:11

Merge branch 'master' into heterogeneous-execution-new-MonadExecuteQuery

c09f3a5

minor simplification

858f22c

abooij added the auto-update-auto-merge label Oct 7, 2020

Merge branch 'master' into heterogeneous-execution-new-MonadExecuteQuery

1c55e58

kodiakhq bot merged commit 84a129c into hasura:master Oct 7, 2020

tirumaraiselvan added this to the v1.4 milestone Oct 20, 2020

abooij mentioned this pull request Oct 30, 2020

support mixing top-level fields from two different schemas #1371

Closed

tirumaraiselvan mentioned this pull request Apr 24, 2023

Heterogenous execution (close #1371) #3153

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

server: heterogeneous execution of GraphQL queries #5869

server: heterogeneous execution of GraphQL queries #5869

Uh oh!

abooij commented Sep 30, 2020 •

edited

Loading

Uh oh!

abooij commented Oct 6, 2020

Uh oh!

lexi-lambda left a comment

Uh oh!

lexi-lambda Oct 6, 2020

Uh oh!

nicuveo Oct 6, 2020

Uh oh!

lexi-lambda Oct 6, 2020 •

edited

Loading

Uh oh!

lexi-lambda Oct 6, 2020

Uh oh!

nicuveo Oct 6, 2020

Uh oh!

tirumaraiselvan left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	Tracing.trace "Postgres" . (runExtractProfile ep =<<) . liftTx $ asSingleRowJsonResp (instrument q) prepArgs
	Tracing.trace "Postgres" $ runExtractProfile ep =<< liftTx do
	asSingleRowJsonResp (instrument q) prepArgs

-             Tracing.trace "Postgres" . (runExtractProfile ep =<<) . liftTx $ asSingleRowJsonResp (instrument q) prepArgs
+             asSingleRowJsonResp (instrument q) prepArgs
+               &   liftTx
+               >>= runExtractProfile ep
+               &   Tracing.trace "Postgres"

server: heterogeneous execution of GraphQL queries #5869

server: heterogeneous execution of GraphQL queries #5869

Uh oh!

Conversation

abooij commented Sep 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changelog

Affected components

Related Issues

Solution and Design

Steps to test and verify

Limitations, known bugs & workarounds

Server checklist

Catalog upgrade

Metadata

GraphQL

Breaking changes

Uh oh!

abooij commented Oct 6, 2020

Uh oh!

lexi-lambda left a comment

Choose a reason for hiding this comment

Uh oh!

lexi-lambda Oct 6, 2020

Choose a reason for hiding this comment

Uh oh!

nicuveo Oct 6, 2020

Choose a reason for hiding this comment

Uh oh!

lexi-lambda Oct 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lexi-lambda Oct 6, 2020

Choose a reason for hiding this comment

Uh oh!

nicuveo Oct 6, 2020

Choose a reason for hiding this comment

Uh oh!

tirumaraiselvan left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

abooij commented Sep 30, 2020 •

edited

Loading

lexi-lambda Oct 6, 2020 •

edited

Loading