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

Rewrite and guard against non-covering queries #3822

Closed
adamcfraser opened this Issue Nov 8, 2018 · 3 comments

Comments

Projects
None yet
3 participants
@adamcfraser
Copy link
Contributor

adamcfraser commented Nov 8, 2018

The channels and all_docs queries are currently non-covering. For channels this is due to an error in the query statement. all_docs is known to be non-covering, but should be reviewed to identify whether a covering version is possible, due to https://issues.couchbase.com/browse/MB-30467.

Along with fixing the query, we should put in place unit tests that retrieve the EXPLAIN for the query and validate that it's covering (to guard against future regressions).

@Binary-Ape

This comment has been minimized.

Copy link

Binary-Ape commented Nov 15, 2018

Hey Adam,

I noticed that the queries for the Access calls is also not covering and this can cause quite the slowdown. We can make a slight change to the index and not have to modify the query itself and achieve coverage. I have attached the query plans for the non-covering and the covering index.

Here is the current query:

SELECT meta(`My_Bucket`).xattrs._sync.access.`role:my_role`
FROM `My_Bucket`
WHERE any op in object_pairs(meta(`My_Bucket`).xattrs._sync.access) 
satisfies op.name = 'role:my_role' end;

Then we have our current index definition:

CREATE INDEX sg_access_x1
ON My_Bucket(all (array (op.name) for op in object_pairs(meta().xattrs._sync.access) end)) 
WITH {"defer_build":true, "retain_deleted_xattr":true, "num_replica":1}

And the modified one which is covering:

CREATE INDEX sg_access_x1_COVERING 
ON My_Bucket(all (array (op.name) for op in object_pairs(meta().xattrs._sync.access) end), meta().xattrs._sync.access)
WITH {"retain_deleted_xattr":true}

The only change necessary was to actually index the access field itself.
Access Call Plan - Non-Covering.txt
Access Call Plan - Covering.txt

Let me know your thoughts!

Thanks,
Lee Yarbrough

@adamcfraser

This comment has been minimized.

Copy link
Contributor

adamcfraser commented Nov 15, 2018

@Binary-Ape The main concern here was the growth in the size of the access/roleAccess index when the full access element is included in the index - particularly when documents are making multiple access grants, this can result in a significant growth to the index size, as the full access element is indexed multiple times per document.

However, I'd be interested in details on the slowdown you've been seeing associated with the non-covering access query. Since the access query only needs to be executed for a given user after a new grant, prioritizing reduced index size over access query speed was judged to be the preferred approach for most use cases. If that's not the case in your deployment, I'd like to hear the details.

@adamcfraser

This comment has been minimized.

Copy link
Contributor

adamcfraser commented Nov 15, 2018

@Binary-Ape One additional note - even for the non-covered query, our internal performance testing was still showing the access/roleAccess query performance to be between 2-3x faster than using views, and with more than 10x higher throughput. If you're seeing something different, any information you can share on your usage would be good to hear.

bbrks added a commit that referenced this issue Dec 3, 2018

Feature/backport #3822 to 2.1.2 (#3843)
* Fix for covering channels query (#3828)

* Fix for covering channels query

Adds EXPLAIN support so that unit test can validate queries are covering.

* Remove unused explain (moved down to bucket)

* Test modifications for 2.1 compatibility

Removes dependency on testify.

@adamcfraser adamcfraser referenced this issue Dec 11, 2018

Merged

DOC-4543 #34

@djpongh djpongh modified the milestones: Iridium, 2.1.2 Dec 20, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment