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

Resolve the role query and the number of docs lazily #48036

Merged
merged 4 commits into from
Oct 25, 2019

Conversation

jimczi
Copy link
Contributor

@jimczi jimczi commented Oct 15, 2019

This commit ensures that the creation of a DocumentSubsetReader does not
eagerly resolve the role query and the number of docs that match.
We want to delay this expensive operation in order to ensure that we really
need this information when we build it. For this reason the role query and the
number of docs are now resolved on demand. This commit also depends on
https://issues.apache.org/jira/browse/LUCENE-9003 that will also compute the global
number of docs lazily.

This commit ensures that the creation of a DocumentSubsetReader does not
eagerly resolve the role query and the number of docs that match.
We want to delay this expensive operation in order to ensure that we really
need this information when we build it. For this reason the role query and the
number of docs are now resolved on demand. This commit also depends on
https://issues.apache.org/jira/browse/LUCENE-9003 that will also compute the global
number of docs lazily.
@jimczi jimczi added >enhancement :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC v8.0.0 v7.5.0 labels Oct 15, 2019
@jimczi jimczi requested a review from jpountz October 15, 2019 07:52
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (:Security/Authorization)

private final int numDocs;
private final DocumentSubsetBitsetCache bitsetCache;
private final Query roleQuery;
private BitSet roleQueryBits;
Copy link
Contributor

Choose a reason for hiding this comment

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

does this one need to be volatile too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought it wouldn't be needed since we access the variable after the synchronized block ?

Copy link
Contributor

Choose a reason for hiding this comment

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

What about access from getLiveDocs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We discussed offline and I added a comment in the code to explain why this is not needed (happens-before):
9e98a69

@jimczi jimczi merged commit 99990cd into elastic:master Oct 25, 2019
@jimczi jimczi deleted the lazy_role_query branch October 25, 2019 15:00
@jimczi jimczi added v7.6.0 and removed v7.5.0 labels Oct 25, 2019
jimczi added a commit that referenced this pull request Oct 25, 2019
This commit ensures that the creation of a DocumentSubsetReader does not
eagerly resolve the role query and the number of docs that match.
We want to delay this expensive operation in order to ensure that we really
need this information when we build it. For this reason the role query and the
number of docs are now resolved on demand. This commit also depends on
https://issues.apache.org/jira/browse/LUCENE-9003 that will also compute the global
number of docs lazily.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants