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

perf(GraphQL): Part-1: Pagination for root user query is applied at root auth query (GRAPHQL-854) #7100

Merged
merged 2 commits into from
Dec 10, 2020

Conversation

abhimanyusinghgaur
Copy link
Contributor

@abhimanyusinghgaur abhimanyusinghgaur commented Dec 10, 2020

This PR moves the pagination parameters for root user query to root auth query. This greatly optimizes the number of nodes that need to be processed by child auth queries.


This change is Reviewable

@github-actions github-actions bot added the area/graphql Issues related to GraphQL support on Dgraph. label Dec 10, 2020
@abhimanyusinghgaur abhimanyusinghgaur changed the title perf(GraphQL-auth-1): Pagination for root user query is applied at root auth query perf(GraphQL-auth-1): Pagination for root user query is applied at root auth query (GRAPHQL-854) Dec 10, 2020
@abhimanyusinghgaur abhimanyusinghgaur changed the title perf(GraphQL-auth-1): Pagination for root user query is applied at root auth query (GRAPHQL-854) perf(GraphQL): Part-1: Pagination for root user query is applied at root auth query (GRAPHQL-854) Dec 10, 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.

Reviewed 3 of 3 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @abhimanyusinghgaur and @MichaelJCompton)


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

		suffix += "." + sg.Attr
	}
	if len(sg.Params.Var) > 0 {

Add a comment about this if block.


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

		suffix += "." + sg.Params.Alias
	}
	ctx, span := otrace.StartSpan(ctx, "query.ProcessGraph."+suffix)

Add a comment about why we do this as well.

Copy link
Contributor Author

@abhimanyusinghgaur abhimanyusinghgaur 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: 2 of 3 files reviewed, 2 unresolved discussions (waiting on @MichaelJCompton and @pawanrawal)


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

Previously, pawanrawal (Pawan Rawal) wrote…

Add a comment about this if block.

Done.


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

Previously, pawanrawal (Pawan Rawal) wrote…

Add a comment about why we do this as well.

Done.

@abhimanyusinghgaur abhimanyusinghgaur merged commit 5042370 into master Dec 10, 2020
@abhimanyusinghgaur abhimanyusinghgaur deleted the abhimanyu/auth-perf-1 branch December 10, 2020 09:55
abhimanyusinghgaur added a commit that referenced this pull request Dec 11, 2020
…#7117)

This PR fixes the issue introduced by #7100. As pagination is moved to root auth query, so order also needs to be applied at root auth query, in addition to being applied at the root user query.
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