Skip to content

chore: Benchmark Auth queries and compare it with non Auth queries. #6029

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

Merged
merged 16 commits into from
Jul 29, 2020

Conversation

arijitAD
Copy link

@arijitAD arijitAD commented Jul 17, 2020

Fixes #GRAPHQL-536

Benchmark single level and nested Auth queries and compare its performance with non-Auth.


This change is Reviewable

Docs Preview: Dgraph Preview

@github-actions github-actions bot added the area/graphql Issues related to GraphQL support on Dgraph. label Jul 17, 2020
Copy link
Contributor

@pawanrawal pawanrawal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be good to go after addressing some comments.

Reviewable status: 0 of 4 files reviewed, 6 unresolved discussions (waiting on @arijitAD, @MichaelJCompton, and @pawanrawal)


graphql/bench/auth_test.go, line 33 at r3 (raw file):

const (
	graphqlURL = "http://100.83.146.128:8080/graphql"

Can this point to localhost instead of a static IP?


graphql/bench/auth_test.go, line 1 at r4 (raw file):

/*

This needs a small README about the kind of tests done and the results or a link to the discuss post.


graphql/bench/auth_test.go, line 463 at r4 (raw file):

}

func generateOwnerRestaurant(items int) Owners {

So it seems like we are generating items number of owners and also items number of restaurants are owned by each owner? Please add that as a comment.


graphql/bench/auth_test.go, line 507 at r4 (raw file):

func BenchmarkMutation(b *testing.B) {
	schemaFile := "schema.graphql"

Schema without auth is used here. Is this intentional?


graphql/bench/auth_test.go, line 525 at r4 (raw file):

	  		id 
	  		name
			owner {

formatting


graphql/bench/auth_test.go, line 548 at r4 (raw file):

			fmt.Printf("Avg Time: %d Time: %d \n", avgTime, reqTime)
		}
		b.StopTimer()

Add a comment about why we are stopping the timer here.

Copy link
Author

@arijitAD arijitAD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 5 files reviewed, 6 unresolved discussions (waiting on @MichaelJCompton and @pawanrawal)


graphql/bench/auth_test.go, line 33 at r3 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Can this point to localhost instead of a static IP?

Done.


graphql/bench/auth_test.go, line 1 at r4 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

This needs a small README about the kind of tests done and the results or a link to the discuss post.

Done.


graphql/bench/auth_test.go, line 463 at r4 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

So it seems like we are generating items number of owners and also items number of restaurants are owned by each owner? Please add that as a comment.

Done.


graphql/bench/auth_test.go, line 507 at r4 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Schema without auth is used here. Is this intentional?

Benchmarks are run twice. Once with auth schema and once with the non-auth schema. Updated the steps in Readme.


graphql/bench/auth_test.go, line 525 at r4 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

formatting

Formatting appears correct in Github.


graphql/bench/auth_test.go, line 548 at r4 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Add a comment about why we are stopping the timer here.

Done.

@arijitAD arijitAD merged commit 6ecf91e into master Jul 29, 2020
@arijitAD arijitAD deleted the arijitAD/auth_query_bench branch July 29, 2020 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/graphql Issues related to GraphQL support on Dgraph.
Development

Successfully merging this pull request may close these issues.

2 participants