Add support to show metrics #4033

poonai · 2019-09-20T08:37:10Z

Add metrics to show the number of nodes touched by each attribute
Add metrics to the response

Signed-off-by: பாலாஜி ஜின்னா [email protected]

This change is

Sample output

{
  "data": {
    "me": [...
    ]
  },
  "extensions": {
    "server_latency": {
      "parsing_ns": 14530,
      "processing_ns": 4937092,
      "encoding_ns": 132441,
      "assign_timestamp_ns": 675662
    },
    "txn": {
      "start_ts": 83
    },
    "metrics": {
      "num_uids": {
        "age": 54,
        "friend": 48,
        "name": 54
      }
    }
  }
}

Signed-off-by: பாலாஜி ஜின்னா <[email protected]>

pullrequest

✅ A review job has been created and sent to the PullRequest network.

Check the status or cancel PullRequest code review here.

Signed-off-by: பாலாஜி ஜின்னா <[email protected]>

mangalaman93

This is not enough, need two things -

Take an example dataset and show some results in the PR show that the results are what we are expecting
What is this metrics that you are computing in this PR?

Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @pawanrawal)

pullrequest

Comments on some of the overall logic and naming.

Reviewed with ❤️ by PullRequest

query/outputnode.go

pullrequest · 2019-09-21T02:27:17Z

query/outputnode.go

+// calculateMetrics populates the given map with the number of uids are gathered for each
+// attributes.
+func calculateMetrics(sg *SubGraph, metrics map[string]int) {
+	// we'll calcuate only destination because this are the uid gathered by this subgraph.


calcuate -> calculate

~~are~~ -> is

query/outputnode.go

pullrequest · 2019-09-21T02:27:17Z

query/outputnode.go

 		sgr.Children = append(sgr.Children, sg)
 	}
+	fmt.Println(metricsMap)


Is this for logging purposes or to console to validate?

pullrequest · 2019-09-21T02:27:17Z

query/outputnode.go

@@ -810,3 +814,29 @@ func (sg *SubGraph) preTraverse(uid uint64, dst outputNode) error {

 	return nil
 }
+
+// calculateMetrics populates the given map with the number of uids are gathered for each


calculateMetrics populates the given map with the number of uids ~~are~~ gathered for each attributes

pullrequest · 2019-09-21T02:27:17Z

query/outputnode.go

+	for _, filter := range sg.Filters {
+		calculateMetrics(filter, metrics)
+	}
+	// calculate metrics for the childres as well.


pawanrawal

Commented on the discuss post, this PR can be simplified. You just need to compute len(uidMatrix) to get an estimate of the number of disk seeks. Let me know if something isn't clear and we can discuss it.

Reviewable status: 0 of 1 files reviewed, 9 unresolved discussions (waiting on @balajijinnah and @pawanrawal)

query/outputnode.go, line 825 at r1 (raw file):

	// some results, because if any filters applied those uids are gone.
	prev := metrics[sg.Attr]
	// QUESTION: @manish @pawan: should I add destuid or length of posting list?.

If I understand correctly, the intention of this PR is to get an estimate of the number of disk seeks that we do?

query/outputnode.go, line 830 at r1 (raw file):

	// as well.
	for _, valList := range sg.valueMatrix {
		prev = prev + len(valList.GetValues())

Why are we adding this? To get a value list, we do one disk seek per uid and predicate combination which would be done at the parent SubGraph of this node.

pullrequest

Due to inactivity, PullRequest has cancelled this review job. You can reactivate the code review job from the PullRequest dashboard

Signed-off-by: balaji jinnah <[email protected]>

mangalaman93

Reviewable status: 0 of 1 files reviewed, 11 unresolved discussions (waiting on @balajijinnah)

query/outputnode.go, line 53 at r2 (raw file):

		}
		// calculate metrics for this query.
		calculateMetrics(sg, metricsMap)

Move this to Request.Process

query/outputnode.go, line 823 at r2 (raw file):

	// we'll calculate srcUid of the each attribute. because, these are number of uids
	// processed by this attribute.
	prev := metrics[sg.Attr]

What happens when Attr is uid.
Check for Internal(), that is O(1) computation for us

mangalaman93

Please address all the comments and add the version of dgo so that I can run it locally.

Reviewable status: 0 of 1 files reviewed, 11 unresolved discussions (waiting on @balajijinnah)

mangalaman93

@balajijinnah please add tests for this change too.

Reviewable status: 0 of 1 files reviewed, 11 unresolved discussions (waiting on @balajijinnah)

Signed-off-by: பாலாஜி <[email protected]>

…aji/metrics

Signed-off-by: பாலாஜி <[email protected]>

Signed-off-by: balaji jinnah <[email protected]>

poonai

Reviewable status: 0 of 8 files reviewed, 11 unresolved discussions (waiting on @balajijinnah, @mangalaman93, @manishrjain, @pawanrawal, and @pullrequest[bot])

query/outputnode.go, line 56 at r1 (raw file):

Previously, pullrequest[bot] wrote…

Is this for logging purposes or to console to validate?

Done.

query/outputnode.go, line 818 at r1 (raw file):

Previously, pullrequest[bot] wrote…

calculateMetrics populates the given map with the number of uids ~~are~~ gathered for each attributes

Done.

query/outputnode.go, line 821 at r1 (raw file):

Previously, pullrequest[bot] wrote…

calcuate -> calculate

~~are~~ -> is

Done.

query/outputnode.go, line 825 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

If I understand correctly, the intention of this PR is to get an estimate of the number of disk seeks that we do?

Done.

query/outputnode.go, line 830 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Why are we adding this? To get a value list, we do one disk seek per uid and predicate combination which would be done at the parent SubGraph of this node.

Done.

query/outputnode.go, line 832 at r1 (raw file):

Previously, pullrequest[bot] wrote…

It feels like the naming could be improved since we are writing to quite a generic field name Attr.

Done.

query/outputnode.go, line 834 at r1 (raw file):

Previously, pullrequest[bot] wrote…

It'd be interesting to have more insight into why Filters are included here.

Done.

query/outputnode.go, line 838 at r1 (raw file):

Previously, pullrequest[bot] wrote…

children

Done.

query/outputnode.go, line 53 at r2 (raw file):

Previously, mangalaman93 (Aman Mangal) wrote…

Move this to Request.Process

Done.

query/outputnode.go, line 823 at r2 (raw file):

Previously, mangalaman93 (Aman Mangal) wrote…

What happens when Attr is uid.

Check for Internal(), that is O(1) computation for us

Done.

pawanrawal

as a first pass. This would require more work for certain queries. The ones that I can think of are:

Sorting at root isn't captured.
Functions at root won't be captured because they don't have source uids.
Filters would only capture SrcUids but most of the work that they do is by fetching index tokens.

Reviewed 7 of 8 files at r3, 1 of 1 files at r4.
Reviewable status: all files reviewed, 11 unresolved discussions (waiting on @balajijinnah, @mangalaman93, @manishrjain, @pawanrawal, and @pullrequest[bot])

manishrjain

Add the caveats @pawanrawal mentioned in the PR description, so we can come back to this and improve it. Rest looks alright to me. Defer to @pawanrawal 's review.

Reviewed 7 of 8 files at r3, 1 of 1 files at r4.
Reviewable status: all files reviewed, 11 unresolved discussions (waiting on @balajijinnah, @mangalaman93, @pawanrawal, and @pullrequest[bot])

…aji/metrics

Signed-off-by: பாலாஜி <[email protected]>

…one more attribute name Signed-off-by: பாலாஜி <[email protected]>

…aji/metrics

add metrics for attributes

e25e61a

Signed-off-by: பாலாஜி ஜின்னா <[email protected]>

poonai requested a review from pawanrawal as a code owner September 20, 2019 08:37

pullrequest bot reviewed Sep 20, 2019

View reviewed changes

call recursively

382e964

Signed-off-by: பாலாஜி ஜின்னா <[email protected]>

mangalaman93 reviewed Sep 20, 2019

View reviewed changes

pullrequest bot reviewed Sep 21, 2019

View reviewed changes

pawanrawal suggested changes Sep 23, 2019

View reviewed changes

pullrequest bot reviewed Oct 5, 2019

View reviewed changes

change destuid to src uid

0a8c19d

Signed-off-by: balaji jinnah <[email protected]>

mangalaman93 requested changes Oct 11, 2019

View reviewed changes

mangalaman93 reviewed Oct 11, 2019

View reviewed changes

mangalaman93 reviewed Oct 12, 2019

View reviewed changes

பாலாஜி added 4 commits October 14, 2019 16:55

moved to execution result

fefda14

Signed-off-by: பாலாஜி <[email protected]>

Merge branch 'master' of https://github.com/dgraph-io/dgraph into bal…

c7c8608

…aji/metrics

add api

b4a5cff

Signed-off-by: பாலாஜி <[email protected]>

test added

24eea69

Signed-off-by: பாலாஜி <[email protected]>

poonai requested review from manishrjain and a team as code owners October 22, 2019 11:01

poonai added 2 commits October 22, 2019 22:08

don't return

9e43fb3

Signed-off-by: balaji jinnah <[email protected]>

typo fixed

2550922

Signed-off-by: balaji jinnah <[email protected]>

poonai commented Oct 22, 2019

View reviewed changes

pawanrawal reviewed Oct 29, 2019

View reviewed changes

manishrjain approved these changes Oct 30, 2019

View reviewed changes

poonai requested a review from mangalaman93 October 30, 2019 10:48

poonai and others added 4 commits November 1, 2019 08:38

Merge branch 'master' of https://github.com/dgraph-io/dgraph into bal…

d1a0444

…aji/metrics

fix sys test

ffa28fc

Signed-off-by: பாலாஜி <[email protected]>

increment one for the systest index test. beacuse, num_uid will give …

071e164

…one more attribute name Signed-off-by: பாலாஜி <[email protected]>

Merge branch 'master' of https://github.com/dgraph-io/dgraph into bal…

c5c10a8

…aji/metrics

poonai merged commit ab8984b into master Nov 7, 2019

joshua-goldstein deleted the balaji/metrics branch August 11, 2022 20:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support to show metrics #4033

Add support to show metrics #4033

poonai commented Sep 20, 2019 •

edited

Loading

pullrequest bot left a comment

mangalaman93 left a comment

pullrequest bot left a comment

pullrequest bot Sep 21, 2019

pullrequest bot Sep 21, 2019

pullrequest bot Sep 21, 2019

pullrequest bot Sep 21, 2019

pawanrawal left a comment

pullrequest bot left a comment

mangalaman93 left a comment

mangalaman93 left a comment

mangalaman93 left a comment

poonai left a comment

pawanrawal left a comment

manishrjain left a comment

Add support to show metrics #4033

Add support to show metrics #4033

Conversation

poonai commented Sep 20, 2019 • edited Loading

pullrequest bot left a comment

Choose a reason for hiding this comment

mangalaman93 left a comment

Choose a reason for hiding this comment

pullrequest bot left a comment

Choose a reason for hiding this comment

pullrequest bot Sep 21, 2019

Choose a reason for hiding this comment

pullrequest bot Sep 21, 2019

Choose a reason for hiding this comment

pullrequest bot Sep 21, 2019

Choose a reason for hiding this comment

pullrequest bot Sep 21, 2019

Choose a reason for hiding this comment

pawanrawal left a comment

Choose a reason for hiding this comment

pullrequest bot left a comment

Choose a reason for hiding this comment

mangalaman93 left a comment

Choose a reason for hiding this comment

mangalaman93 left a comment

Choose a reason for hiding this comment

mangalaman93 left a comment

Choose a reason for hiding this comment

poonai left a comment

Choose a reason for hiding this comment

pawanrawal left a comment

Choose a reason for hiding this comment

manishrjain left a comment

Choose a reason for hiding this comment

poonai commented Sep 20, 2019 •

edited

Loading