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

Live loading a data export: Cannot mutate graphql reserved predicate dgraph.graphql.schema #5235

Closed
danielmai opened this issue Apr 17, 2020 · 9 comments
Assignees
Labels
kind/bug Something is broken. status/accepted We accept to investigate/work on it.

Comments

@danielmai
Copy link
Contributor

What version of Dgraph are you using?

master (5f74130)

Have you tried reproducing the issue with the latest release?

This doesn't happen in v20.03.0.

What is the hardware spec (RAM, OS)?

64 GB Ubuntu Linux, Docker

Steps to reproduce the issue (command/config used to run Dgraph).

  1. Run Dgraph
  2. Run a data export.
  3. Run live loader: dgraph live -f g01.rdf.gz -s g01.schema.gz. -a localhost:9080 -z localhost:5080

Expected behavior and actual result.

Live loader never finishes and continues outputting this in the log.

[10:48:17-0700] Elapsed: 19m25s Txns: 13073 N-Quads: 13073000 N-Quads/s [last 5s]: 19400 Aborts: 0
Error while mutating: Cannot mutate graphql reserved predicate dgraph.graphql.schema s.Code Unknown
...
Error while mutating: Cannot mutate graphql reserved predicate dgraph.graphql.schema s.Code Unknown
[12:27:07-0700] Elapsed: 01h58m15s Txns: 19387 N-Quads: 19386576 N-Quads/s [last 5s]:     0 Aborts: 606

The export file has the graphql predicates in the export.

$ zcat ./datav20.03.0/alpha1/export/dgraph.r7.u0417.1935/g01.rdf.gz
<0x1> <dgraph.type> "dgraph.graphql"^^<xs:string> .
<0x1> <dgraph.graphql.xid> "dgraph.graphql.schema"^^<xs:string> .
<0x1> <dgraph.graphql.schema> ""^^<xs:string> .
@danielmai danielmai added the kind/bug Something is broken. label Apr 17, 2020
@ashish-goswami
Copy link
Contributor

ashish-goswami commented Apr 23, 2020

Able to reproduce this. In this(8a650d7) change, we started inserting some graphql node.

mutations := []*dgoapi.Mutation{
		{
			SetJson: []byte(fmt.Sprintf(`
			{
				"uid": "_:%s",
				"dgraph.type": ["%s"],
				"%s": "%s",
				"%s": ""
			}`, varName, gqlType, gqlSchemaXidKey, gqlSchemaXidVal, gqlSchemaPred)),
			Cond: fmt.Sprintf(`@if(eq(len(%s),0))`, varName),
		},
	}

This node is also exported when export is triggered. Exported data for this node is as follow:

<0x1> <dgraph.type> "dgraph.graphql"^^<xs:string> .
<0x1> <dgraph.graphql.xid> "dgraph.graphql.schema"^^<xs:string> .
<0x1> <dgraph.graphql.schema> ""^^<xs:string> .

Since this node has only reserved predicates, hence live loader panics show above error while performing mutation for these records.

@ashish-goswami ashish-goswami added the status/accepted We accept to investigate/work on it. label Apr 23, 2020
@MichelDiz
Copy link
Contributor

Also, we should have a way of letting everyone know that internal predicates should be hidden from the Schema panel. As well as this fact that one should not export system predicates. A note somewhere visible maybe.

@MichelDiz
Copy link
Contributor

Note: Just an update, I have some users complaining about this.

Error while proposing initial schema: Schema does not contain a matching predicate for field dgraph.graphql.schema in type dgraph.graphql

@MichaelJCompton
Copy link
Contributor

Maybe we shouldn't export that predicate in an export and live loader should have an option to pass a GraphQL schema.

Also looks like something that should be caught by an automated test, so we should improve there too.

@abhimanyusinghgaur
Copy link
Contributor

abhimanyusinghgaur commented May 19, 2020

There is also an upcoming PR regarding internal types/predicates, where any type/predicate that start with dgraph. will be considered internal, instead of the current way of having only some specific reserved predicates. This PR is supposed to go as a breaking change in 20.07 release.
#5185
With this change, users of dgraph won't be allowed to create any types/predicates starting with dgraph.

@MichaelJCompton
Copy link
Contributor

I'm thinking firstly confirm if it's an issues on 20.03.1 once a GraphQL schema is added. I expect the easiest way to do that is to add an export then live load test to both master and 20.03, if there isn't enough already. Then either...

(option 1) more changes / better for users - also updates live loader

  • reserved predicates don't get exported
  • live loader should take a GraphQL schema option if your export was a of a GraphQL Dgraph
  • then a GraphQL user can just export data and live load data+GraphQL schema

(option 2) quicker to do / less nice for users - no changes to live loader

  • reserved predicates don't get exported
  • a user with GraphQL would export data + Dgraph schema + GraphQL schema, then live load data + Dgraph schema, then readd GraphQL schema

@martinmr
Copy link
Contributor

I think the following would unblock us the fastest:

  • Still export the reserved predicates (the dgraph.type predicate at least will always need to be exported).
  • The live loader ignores the graphql schema during the mutations.
  • At the end of the load. It updates the graphql schema via the proper endpoint.

We could have a flag whose default value is true because I think by default the live loader assumes that we want to import all the data. If that's not desired, users could flip the value of the flag to skip sending the graphql schema.

@MichaelJCompton thoughts?

@martinmr
Copy link
Contributor

martinmr commented May 20, 2020

Nevermind. There's also the xid predicate that I haven't considered. Maybe we could do the following:

  1. Export exports all the graphql information to a separate file (kind of like how the schema is currently exported). The graphql triplets wouldn't show up in the data. However, I don't know if this can be done.
  2. The live loader can be passed this file via a flag to restore the graphql state via the appropriate endpoint. The live loader is doing the same for the normal schema.

@martinmr
Copy link
Contributor

martinmr commented Jun 3, 2020

This is fixed in the last releases. There are still changes to the live loader that need to go but they are not blocking the releases. I've opened a separate ticket to track those changes.

@martinmr martinmr closed this as completed Jun 3, 2020
@martinmr martinmr self-assigned this Jun 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something is broken. status/accepted We accept to investigate/work on it.
Development

No branches or pull requests

6 participants