-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
fix(DQL): optimize query for has function with offset. #7727
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change looks good. I have a couple of comments.
- Are there any tests that already cover this or else could we add some?
- Also add some benchmark numbers based on your testing in the description. Add a comment saying that this doesn't improve latency for predicates with lang tag.
Reviewed 4 of 4 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @manishrjain, @minhaj-shakeel, and @vvbalaji-dgraph)
protos/pb.proto, line 77 at r1 (raw file):
int32 first = 15; // used to limit the number of result. Typically, the count is value of first // field. Now, It's been used only for has query. int32 offset = 16;
add a comment that first and offset help fetch lesser results for the has
query when there is no filter and order.
query/query.go, line 2291 at r1 (raw file):
if len(sg.Params.Order) == 0 && len(sg.Params.FacetsOrder) == 0 { // There is no ordering. Just apply pagination and return. if !(len(sg.Filters) == 0 && sg.SrcFunc != nil && sg.SrcFunc.Name == "has") {
Add a comment here that for has
function when there is no filtering and sorting we fetch correct paginated results from disk, so no need to apply pagination again.
There was a problem hiding this 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 r2.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @manishrjain, @minhaj-shakeel, and @vvbalaji-dgraph)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @manishrjain, @pawanrawal, and @vvbalaji-dgraph)
protos/pb.proto, line 77 at r1 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
add a comment that first and offset help fetch lesser results for the
has
query when there is no filter and order.
Done.
query/query.go, line 2291 at r1 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
Add a comment here that for
has
function when there is no filtering and sorting we fetch correct paginated results from disk, so no need to apply pagination again.
Done.
Fixes DGRAPH-574. This PR reduces the latency of the `has` function slightly. For example the given query:- ``` { me(func: has(actor.film), first: 10, offset: 300000){ name@en } } ``` when run on the `21million` dataset takes around `80` milliseconds to execute as compared to earlier around `93` milliseconds. Machine Specs:- ``` Model Name: MacBook Pro Model Identifier: MacBookPro16,1 Processor Name: 6-Core Intel Core i7 Processor Speed: 2.6 GHz Number of Processors: 1 Total Number of Cores: 6 L2 Cache (per Core): 256 KB L3 Cache: 12 MB Memory: 16 GB ``` Note:- This does not optimize for the `has` function on predicates with the `@lang` tag.
Fixes DGRAPH-574. This PR reduces the latency of the `has` function slightly. For example the given query:- ``` { me(func: has(actor.film), first: 10, offset: 300000){ name@en } } ``` when run on the `21million` dataset takes around `80` milliseconds to execute as compared to earlier around `93` milliseconds. Machine Specs:- ``` Model Name: MacBook Pro Model Identifier: MacBookPro16,1 Processor Name: 6-Core Intel Core i7 Processor Speed: 2.6 GHz Number of Processors: 1 Total Number of Cores: 6 L2 Cache (per Core): 256 KB L3 Cache: 12 MB Memory: 16 GB ``` Note:- This does not optimize for the `has` function on predicates with the `@lang` tag.
Fixes DGRAPH-574. This PR reduces the latency of the `has` function slightly. For example the given query:- ``` { me(func: has(actor.film), first: 10, offset: 300000){ name@en } } ``` when run on the `21million` dataset takes around `80` milliseconds to execute as compared to earlier around `93` milliseconds. Machine Specs:- ``` Model Name: MacBook Pro Model Identifier: MacBookPro16,1 Processor Name: 6-Core Intel Core i7 Processor Speed: 2.6 GHz Number of Processors: 1 Total Number of Cores: 6 L2 Cache (per Core): 256 KB L3 Cache: 12 MB Memory: 16 GB ``` Note:- This does not optimize for the `has` function on predicates with the `@lang` tag. (cherry picked from commit 9ba15b7) Co-authored-by: minhaj-shakeel <minhaj@dgraph.io>
Fixes DGRAPH-574.
This PR reduces the latency of the
has
function slightly. For example the given query:-when run on the
21million
dataset takes around80
milliseconds to execute as compared to earlier around93
milliseconds.Machine Specs:-
Note:-
This does not optimize for
has
function on predicates with@lang
tag.This change is