-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Enable multiQuery optimization for valueMap step #2444
Comments
Hi @li-boxuan , do you still have plan to implement this optimization? |
Hi, @ntrhieu89 My recent schedule doesn't allow me to do so. I could assist and/or review if anyone wants to give it a try! |
This issue is mentioned explicitly in the docs about batch processing. Just posting this here as a reminder for us that we don't forget to update the docs when we close this issue. |
Fixes JanusGraph#2444 Signed-off-by: Oleksandr Porunov <alexandr.porunov@gmail.com>
I think we could make
We could even execute both queries (for properties and for labels) in parallel in theory, but then we should decide if it's OK to create additional Thread in this step or should we use some managed thread pool for that. |
Is there anyone who will work ok this one? Otherwise, I could take a look. I think |
I was trying to do that today, but stuck a little bit with vertex labels multi-query retrieval due to small TinkerPop limitation (tying to fix it here: apache/tinkerpop#2022 ). I think I would better prefer help with #3244 (multi-query optimization for |
In my query testing with |
I need to check that, but last time I was checking I had a problem with has steps after an index. In my situation the prefetching didn’t kick in. Something like: in the above query if an index exists for name but not for age, then we will not use prefetch in the above case which will lead poor filtering performance because age properties will be prefetched one by one instead of using multi-query optimisation. P.S. I think it would make sense to keep discussion about multi-query for has step in the linked issue #3244 (sorry for staring one here) |
Fixes JanusGraph#2444 Signed-off-by: Oleksandr Porunov <alexandr.porunov@gmail.com>
Related PR: apache/tinkerpop#2027 |
TinkerPop 3.6.3 is released. After we update TinkerPop to 3.6.3 in JanusGraph we will be able to implement this optimization issue. |
Adds possibility to fetch properties and labels of vertices using valueMap, elementMap, propertyMap steps. Adds fetching modes to properties, values, valueMap, elementMap, propertyMap steps to be able to preFetch all properties (single slice query) or only required properties (separate slice query per each requested property). Fixes JanusGraph#2444 Signed-off-by: Oleksandr Porunov <alexandr.porunov@gmail.com>
Adds possibility to fetch properties and labels of vertices using valueMap, elementMap, propertyMap steps. Adds fetching modes to properties, values, valueMap, elementMap, propertyMap steps to be able to preFetch all properties (single slice query) or only required properties (separate slice query per each requested property). Fixes JanusGraph#2444 Signed-off-by: Oleksandr Porunov <alexandr.porunov@gmail.com>
Adds possibility to fetch properties and labels of vertices using valueMap, elementMap, propertyMap steps. Adds fetching modes to properties, values, valueMap, elementMap, propertyMap steps to be able to preFetch all properties (single slice query) or only required properties (separate slice query per each requested property). Fixes JanusGraph#2444 Signed-off-by: Oleksandr Porunov <alexandr.porunov@gmail.com>
… [cql-tests] [tp-tests] Adds possibility to fetch properties and labels of vertices using valueMap, elementMap, propertyMap steps. Adds fetching modes to properties, values, valueMap, elementMap, propertyMap steps to be able to preFetch all properties (single slice query) or only required properties (separate slice query per each requested property). Fixes JanusGraph#2444 Signed-off-by: Oleksandr Porunov <alexandr.porunov@gmail.com>
… [cql-tests] [tp-tests] Adds possibility to fetch properties and labels of vertices using valueMap, elementMap, propertyMap steps. Adds fetching modes to properties, values, valueMap, elementMap, propertyMap steps to be able to preFetch all properties (single slice query) or only required properties (separate slice query per each requested property). Fixes JanusGraph#2444 Signed-off-by: Oleksandr Porunov <alexandr.porunov@gmail.com>
… [cql-tests] [tp-tests] Adds possibility to fetch properties and labels of vertices using valueMap, elementMap, propertyMap steps. Adds fetching modes to properties, values, valueMap, elementMap, propertyMap steps to be able to preFetch all properties (single slice query) or only required properties (separate slice query per each requested property). Fixes JanusGraph#2444 Signed-off-by: Oleksandr Porunov <alexandr.porunov@gmail.com>
… [cql-tests] [tp-tests] Adds possibility to fetch properties and labels of vertices using valueMap, elementMap, propertyMap steps. Adds fetching modes to properties, values, valueMap, elementMap, propertyMap steps to be able to preFetch all properties (single slice query) or only required properties (separate slice query per each requested property). Fixes JanusGraph#2444 Signed-off-by: Oleksandr Porunov <alexandr.porunov@gmail.com>
… [cql-tests] [tp-tests] Adds possibility to fetch properties and labels of vertices using valueMap, elementMap, propertyMap steps. Adds fetching modes to properties, values, valueMap, elementMap, propertyMap steps to be able to preFetch all properties (single slice query) or only required properties (separate slice query per each requested property). Fixes JanusGraph#2444 Signed-off-by: Oleksandr Porunov <alexandr.porunov@gmail.com>
Hi @porunov, thanks for the recent commits. I will look at the changes to learn from them.
I troubleshooted it and realized that to be able to get all labels, currently multiQuery optimization is not applied and hence sequential getRange()s are used. If there are thousands of vertices found, |
Yes, this is on my radar. The dedicated issue for ‘label’ step is: #3813 It’s now fairly easy to add multi-query support for label step. I think we can simply replace TinkerPops label step with our label step with the optimisation in place. The only thing we need to ensure that the any strategy which uses TinkerPops label step is applied before the replacement strategy is applied OR we simply need to remove ‘final’ keyword from TinkerPops ‘LabelStep’. I will see if TinkerPop folks are OK with it and will fix #3813 after that. |
Thanks. I think it is worth mentioning that The query we encountered filter the vertices based on label and then get their ids like this:
Currently I don't think there is a work-around until |
|
Hi @porunov , in my test it looks like the For example, the twos below both use multiQuery:
However, with the two queries below, only the first one uses
Or maybe the second one actually uses it, but the barrier is somehow set to 4?
|
|
It is not We could not wait for 1.0.0 so we picked that version. |
We have JanusGraphPropertiesStep which extends PropertiesStep and supports multiQuery optimization, so that values() step can be optimized.
We can do something similar to PropertyMapStep so that multiQuery optimization can be applied to valueMap() step as well.
Requested by #2401
The text was updated successfully, but these errors were encountered: