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

Regression with queries that use value variables causing panics #3470

Closed
erhlee-bird opened this issue May 26, 2019 · 1 comment · Fixed by #3505
Closed

Regression with queries that use value variables causing panics #3470

erhlee-bird opened this issue May 26, 2019 · 1 comment · Fixed by #3505
Labels
kind/bug Something is broken.

Comments

@erhlee-bird
Copy link

If you suspect this could be a bug, follow the template.

  • What version of Dgraph are you using?

All experiments run with Dgraph Docker images from Docker Hub

dgraph/dgraph:master
dgraph/dgraph:v1.0.15-rc9
dgraph/dgraph:v1.0.14
dgraph/dgraph:v1.0.13
dgraph/dgraph:v1.0.12
dgraph/dgraph:v1.0.11

  • Have you tried reproducing the issue with latest release?

Yes, see versions listed above.

  • What is the hardware spec (RAM, OS)?

1 Zero, 1 Alpha - 4 GB - Linux

3 Zero, 3 Alpha - Lots of memory - Linux

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

Commands to start local 1 Zero, 1 Alpha cluster.

$ docker run --rm -it --net=host dgraph/dgraph:${VERSION} dgraph zero
$ docker run --rm -it --net=host dgraph/dgraph:${VERSION} dgraph alpha --lru_mb=4096

Schema.

<number>: int @index(int) .

Query to get the highest available number value.

{
  var(func: has(number)) { 
    number as number 
  }
  var() { 
    highest as max(val(number)) 
  }

  all(func: eq(number, val(highest))) { 
    uid 
    number 
  }
}

Another query to get the highest available number value.

{
  var(func: has(number, orderdesc: number, first: 1) 
  {
    highest_number as number
  }

  all(func: eq(number, val(highest_number))) 
  { 
    uid 
  }
}
  • Expected behaviour and actual result.

The queries above, when run against a Dgraph cluster without any loaded data, should yield an empty list.

dgraph/dgraph:v1.0.11 returns an empty list appropriately

all other versions panic and the alpha crashes.

Error message yielded in client.

E           grpc._channel._Rendezvous: <_Rendezvous of RPC that terminated with:
E               status = StatusCode.UNAVAILABLE
E               details = "Socket closed"
E               debug_error_string = "{"created":"@1558883325.760513047","description":"Error received from peer ipv4:127.0.0.1:9080","file":"src/core/lib/surface/call.cc","file_line":1041,"grpc_message":"Socket closed","grpc_status":14}"
E           >

Error message from alpha logs.

panic: assignment to entry in nil map

goroutine 291 [running]:
github.com/dgraph-io/dgraph/query.(*SubGraph).fillVars(0xc00039d200, 0xc000634480, 0x1, 0x1)
        /ext-go/1/src/github.com/dgraph-io/dgraph/query/query.go:1721 +0x5a7
github.com/dgraph-io/dgraph/query.(*SubGraph).recursiveFillVars(0xc00039d200, 0xc000634480, 0xc00060b020, 0x1)
        /ext-go/1/src/github.com/dgraph-io/dgraph/query/query.go:1660 +0x39
github.com/dgraph-io/dgraph/query.(*QueryRequest).ProcessQuery(0xc0100218f8, 0x174bac0, 0xc0002b9a40, 0x1, 0x1)
        /ext-go/1/src/github.com/dgraph-io/dgraph/query/query.go:2676 +0x68a
github.com/dgraph-io/dgraph/query.(*QueryRequest).Process(0xc0100218f8, 0x174bac0, 0xc0002b9a40, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /ext-go/1/src/github.com/dgraph-io/dgraph/query/query.go:2770 +0x72
github.com/dgraph-io/dgraph/edgraph.(*Server).doQuery(0x2217178, 0x174bac0, 0xc0002bd830, 0xc00020e870, 0xc000309980, 0x0, 0x0)
        /ext-go/1/src/github.com/dgraph-io/dgraph/edgraph/server.go:652 +0x772
github.com/dgraph-io/dgraph/edgraph.(*Server).Query(0x2217178, 0x174bac0, 0xc0002b9830, 0xc00020e870, 0x2217178, 0xc0002b9320, 0x14aa080)
        /ext-go/1/src/github.com/dgraph-io/dgraph/edgraph/server.go:561 +0x98
github.com/dgraph-io/dgraph/vendor/github.com/dgraph-io/dgo/protos/api._Dgraph_Query_Handler(0x155bf60, 0x2217178, 0x174bac0, 0xc0002b9830, 0xc00020e820, 0x0, 0x0, 0x0, 0xc00023c0a0, 0x91)
        /ext-go/1/src/github.com/dgraph-io/dgraph/vendor/github.com/dgraph-io/dgo/protos/api/api.pb.go:1917 +0x23e
github.com/dgraph-io/dgraph/vendor/google.golang.org/grpc.(*Server).processUnaryRPC(0xc0005d5c80, 0x1751f40, 0xc00022a900, 0xc00021e800, 0xc00031b980, 0x2153c78, 0x0, 0x0, 0x0)
        /ext-go/1/src/github.com/dgraph-io/dgraph/vendor/google.golang.org/grpc/server.go:972 +0x4a2
github.com/dgraph-io/dgraph/vendor/google.golang.org/grpc.(*Server).handleStream(0xc0005d5c80, 0x1751f40, 0xc00022a900, 0xc00021e800, 0x0)
        /ext-go/1/src/github.com/dgraph-io/dgraph/vendor/google.golang.org/grpc/server.go:1252 +0xe02
github.com/dgraph-io/dgraph/vendor/google.golang.org/grpc.(*Server).serveStreams.func1.1(0xc00042c800, 0xc0005d5c80, 0x1751f40, 0xc00022a900, 0xc00021e800)
        /ext-go/1/src/github.com/dgraph-io/dgraph/vendor/google.golang.org/grpc/server.go:691 +0x9f
created by github.com/dgraph-io/dgraph/vendor/google.golang.org/grpc.(*Server).serveStreams.func1
        /ext-go/1/src/github.com/dgraph-io/dgraph/vendor/google.golang.org/grpc/server.go:689 +0xa1

Issues that seem relevant:

@pawanrawal
Copy link
Contributor

Seems like the issue is caused by ef24567 which tries to assign a value to the map mp[v.Name].Vals. This assignment itself is a bit weird as the map has a mapping of uid => value so assigning to 0 doesn't make much sense so I believe the default case in the switch block should be removed.

The change in ef24567 seems to have been made to counteract the behavior introduced in 18d8d25 which started throwing errors when uidToVal has length 0. The codebase prior to the change had different assumptions about the value of the map being nil vs an empty map with length 0. This has to be revisited to understand if we should change the check back to nil or make other changes to ensure no errors are thrown. I am looking into it.

Also, the correct behaviour for these queries should be an empty value.

@mangalaman93 mangalaman93 removed their assignment Jun 3, 2019
pawanrawal added a commit that referenced this issue Jun 11, 2019
This fixes #3470. Test cases have been added to verify the fix. The default case in fillVars seemed hacky because it was assigning a value corresponding to uid 0 in uidToVal, that has been removed and appropriate changes have been made at other places.
pawanrawal added a commit that referenced this issue Jun 12, 2019
This fixes #3470. Test cases have been added to verify the fix. The default case in fillVars seemed hacky because it was assigning a value corresponding to uid 0 in uidToVal, that has been removed and appropriate changes have been made at other places.
pawanrawal added a commit that referenced this issue Jun 18, 2019
This fixes #3470. Test cases have been added to verify the fix. The default case in fillVars seemed hacky because it was assigning a value corresponding to uid 0 in uidToVal, that has been removed and appropriate changes have been made at other places.
dna2github pushed a commit to dna2fork/dgraph that referenced this issue Jul 19, 2019
This fixes hypermodeinc#3470. Test cases have been added to verify the fix. The default case in fillVars seemed hacky because it was assigning a value corresponding to uid 0 in uidToVal, that has been removed and appropriate changes have been made at other places.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something is broken.
4 participants