Skip to content

Commit

Permalink
Fix(Dgraph): Fix how visited nodes are detected in recurse queries. (#…
Browse files Browse the repository at this point in the history
…6272)

Currently a node is marked as visited if it's been visited before AND
has been visited from the same source UID. In dense graphs, the second
condition leads to exponential growth of the data and to other issues
such as responses that are too big to encode.  Removing this condition
fixes the issue.

Fixed tests and verified the new output makes sense.

Fixes DGRAPH-2337

(cherry picked from commit 3638c12)
  • Loading branch information
martinmr committed Aug 26, 2020
1 parent da208b6 commit 85b3842
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 8 deletions.
7 changes: 3 additions & 4 deletions query/query3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ func TestRecurseNestedError2(t *testing.T) {
}

func TestRecurseQuery(t *testing.T) {

query := `
{
me(func: uid(0x01)) @recurse {
Expand All @@ -90,7 +89,7 @@ func TestRecurseQuery(t *testing.T) {
}`
js := processQueryNoErr(t, query)
require.JSONEq(t,
`{"data": {"me":[{"name":"Michonne", "friend":[{"name":"Rick Grimes", "friend":[{"name":"Michonne"}]},{"name":"Glenn Rhee"},{"name":"Daryl Dixon"},{"name":"Andrea", "friend":[{"name":"Glenn Rhee"}]}]}]}}`, js)
`{"data": {"me":[{"name":"Michonne", "friend":[{"name":"Rick Grimes", "friend":[{"name":"Michonne"}]},{"name":"Glenn Rhee"},{"name":"Daryl Dixon"},{"name":"Andrea"}]}]}}`, js)
}

func TestRecurseExpand(t *testing.T) {
Expand Down Expand Up @@ -132,7 +131,7 @@ func TestRecurseQueryOrder(t *testing.T) {
}`
js := processQueryNoErr(t, query)
require.JSONEq(t,
`{"data": {"me":[{"dob":"1910-01-01T00:00:00Z","friend":[{"dob":"1910-01-02T00:00:00Z","friend":[{"dob":"1910-01-01T00:00:00Z","name":"Michonne"}],"name":"Rick Grimes"},{"dob":"1909-05-05T00:00:00Z","name":"Glenn Rhee"},{"dob":"1909-01-10T00:00:00Z","name":"Daryl Dixon"},{"dob":"1901-01-15T00:00:00Z","friend":[{"dob":"1909-05-05T00:00:00Z","name":"Glenn Rhee"}],"name":"Andrea"}],"name":"Michonne"}]}}`,
`{"data": {"me":[{"dob":"1910-01-01T00:00:00Z","friend":[{"dob":"1910-01-02T00:00:00Z","friend":[{"dob":"1910-01-01T00:00:00Z","name":"Michonne"}],"name":"Rick Grimes"},{"dob":"1909-05-05T00:00:00Z","name":"Glenn Rhee"},{"dob":"1909-01-10T00:00:00Z","name":"Daryl Dixon"},{"dob":"1901-01-15T00:00:00Z","name":"Andrea"}],"name":"Michonne"}]}}`,
js)
}

Expand All @@ -147,7 +146,7 @@ func TestRecurseQueryAllowLoop(t *testing.T) {
}
}`
js := processQueryNoErr(t, query)
require.JSONEq(t, `{"data":{"me":[{"friend":[{"friend":[{"dob":"1910-01-01T00:00:00Z","name":"Michonne"}],"dob":"1910-01-02T00:00:00Z","name":"Rick Grimes"},{"dob":"1909-05-05T00:00:00Z","name":"Glenn Rhee"},{"dob":"1909-01-10T00:00:00Z","name":"Daryl Dixon"},{"friend":[{"dob":"1909-05-05T00:00:00Z","name":"Glenn Rhee"}],"dob":"1901-01-15T00:00:00Z","name":"Andrea"}],"dob":"1910-01-01T00:00:00Z","name":"Michonne"}]}}`, js)
require.JSONEq(t, `{"data":{"me":[{"friend":[{"friend":[{"dob":"1910-01-01T00:00:00Z","name":"Michonne"}],"dob":"1910-01-02T00:00:00Z","name":"Rick Grimes"},{"dob":"1909-05-05T00:00:00Z","name":"Glenn Rhee"},{"dob":"1909-01-10T00:00:00Z","name":"Daryl Dixon"},{"dob":"1901-01-15T00:00:00Z","name":"Andrea"}],"dob":"1910-01-01T00:00:00Z","name":"Michonne"}]}}`, js)
}

func TestRecurseQueryAllowLoop2(t *testing.T) {
Expand Down
8 changes: 4 additions & 4 deletions query/recurse.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
)

func (start *SubGraph) expandRecurse(ctx context.Context, maxDepth uint64) error {
// Note: Key format is - "attr|fromUID|toUID"
// Note: Key format is - "attr|toUID"
reachMap := make(map[string]struct{})
allowLoop := start.Params.RecurseArgs.AllowLoop
var numEdges uint64
Expand Down Expand Up @@ -118,15 +118,15 @@ func (start *SubGraph) expandRecurse(ctx context.Context, maxDepth uint64) error
sg.updateUidMatrix()
}

for mIdx, fromUID := range sg.SrcUIDs.Uids {
for mIdx := range sg.SrcUIDs.Uids {
if allowLoop {
for _, ul := range sg.uidMatrix {
numEdges += uint64(len(ul.Uids))
}
} else {
algo.ApplyFilter(sg.uidMatrix[mIdx], func(uid uint64, i int) bool {
key := fmt.Sprintf("%s|%d|%d", sg.Attr, fromUID, uid)
_, seen := reachMap[key] // Combine fromUID here.
key := fmt.Sprintf("%s|%d", sg.Attr, uid)
_, seen := reachMap[key]
if seen {
return false
}
Expand Down

0 comments on commit 85b3842

Please sign in to comment.