feat(lambda): store lambda scripts within the dgraph, send it to JS s…

[NamanJain8]

Related to dgraph-io/dgraph-lambda#22

This PR changes the way lambda is handled by Dgraph. Earlier, the user had to start the server with a predefined lambda script. This does not scale well in a multi-tenant environment. As we will need N servers (or endpoints) to handle the load. Also, each time the script is changed, the lambda server needs to be restarted.

With this change, instead of loading the script from the predefined file, we will be sending the script in the request body to the lambda-server.

The details are mentioned in the discuss post. To summarize:

Storage

We will store the lambda script within dgraph.graphql.schema predicate. The value will be JSON marshal of

type GQL struct {
   Script:     String
   Schema: String
}

This change has been done in a backward-compatible manner.

New resolvers

This adds 2 new admin resolvers:

• UpdateLambdaScript → accepts base64 encoded lambda script
• GetLambdaScript → Returns base64 encoded script

mutation ($script: String!) {
  updateLambdaScript(input:{set:{script:$script}}) {
     lambdaScript{
      script
    }
  }
}

query {
  getLambdaScript {
    script
  }
}

Export/Import:

The GraphQL schema is exported in a separate file in the following JSON format:
[ExportedGQL] where ExportedGQL type is {Namespace: Int, Schema: String}. We will extend it to {Namespace: Int, Schema: String, Script: String} and hence the bulk loader can handle it properly.

---

This change is 

[manishrjain]

Had a rough look, looks good. Get a deeper review by @aman-bansal / @ahsanbarkati

Reviewed 12 of 12 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @NamanJain8)

---

protos/pb.proto, line 743 at r1 (raw file):

message UpdateGraphQLSchemaRequest {
  enum Op {
    SCHEMA = 0;

Do you need the Op?

[NamanJain8]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @manishrjain)

---

protos/pb.proto, line 743 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Do you need the Op?

Op is needed because update the schema to an empty string, then we won't be able to decide the type of update on the receiver.

[ahsanbarkati]

Looks good to me. Few nits.

Reviewable status: 8 of 20 files reviewed, 5 unresolved discussions (waiting on @manishrjain and @NamanJain8)

---

edgraph/server.go, line 500 at r3 (raw file):

		// just reinsert the GraphQL schema, no need to alter dgraph schema as this was drop_data
		if _, e := UpdateGQLSchema(ctx, graphQLSchema, ""); e != nil {
			err = errors.Errorf("While updating gql schema, got err: %+v.", e)

If e is not nil then why not return here itself? Why do we need to accumulate the errors?

---

worker/lambda_script.go, line 31 at r3 (raw file):

type LambdaScriptStore struct {
	mux    sync.RWMutex

Can we avoid the variable mux, it will make it look cleaner.

type LambdaScriptStore struct {
  sync.RQMutex
  ...
}

Then directly do ls.Lock() instead of ls.mux.Lock()

[NamanJain8]

Reviewable status: 7 of 20 files reviewed, 5 unresolved discussions (waiting on @ahsanbarkati and @manishrjain)

---

edgraph/server.go, line 500 at r3 (raw file):

Previously, ahsanbarkati (Ahsan Barkati) wrote…

If e is not nil then why not return here itself? Why do we need to accumulate the errors?

Done.

---

worker/lambda_script.go, line 31 at r3 (raw file):

Previously, ahsanbarkati (Ahsan Barkati) wrote…

Can we avoid the variable mux, it will make it look cleaner.

type LambdaScriptStore struct {
  sync.RQMutex
  ...
}

Then directly do ls.Lock() instead of ls.mux.Lock()

Done. Thanks.