From b3ec0148d18526e31c1152ef12becdd5efbb89ef Mon Sep 17 00:00:00 2001 From: Martin Martinez Rivera Date: Tue, 25 Aug 2020 14:40:46 -0700 Subject: [PATCH 1/2] Fix(Dgraph): Fix how visited nodes are detected in recurse queries. 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 --- query/query3_test.go | 7 +++---- query/recurse.go | 8 ++++---- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/query/query3_test.go b/query/query3_test.go index 9da965a4a6d..c7db81edc93 100644 --- a/query/query3_test.go +++ b/query/query3_test.go @@ -79,7 +79,6 @@ func TestRecurseNestedError2(t *testing.T) { } func TestRecurseQuery(t *testing.T) { - query := ` { me(func: uid(0x01)) @recurse { @@ -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) { @@ -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) } @@ -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) { diff --git a/query/recurse.go b/query/recurse.go index 40a4475c117..31cb087d298 100644 --- a/query/recurse.go +++ b/query/recurse.go @@ -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 @@ -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 } From 6750e9981c21cb8a0f6a213e887bdd0516575756 Mon Sep 17 00:00:00 2001 From: Martin Martinez Rivera Date: Tue, 25 Aug 2020 15:02:59 -0700 Subject: [PATCH 2/2] Fix deepsource warning. --- query/recurse.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/query/recurse.go b/query/recurse.go index 31cb087d298..abaefbeab63 100644 --- a/query/recurse.go +++ b/query/recurse.go @@ -118,7 +118,7 @@ func (start *SubGraph) expandRecurse(ctx context.Context, maxDepth uint64) error sg.updateUidMatrix() } - for mIdx, _ := range sg.SrcUIDs.Uids { + for mIdx := range sg.SrcUIDs.Uids { if allowLoop { for _, ul := range sg.uidMatrix { numEdges += uint64(len(ul.Uids))