You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Fredy Wijaya (Code Review)" <ge...@cloudera.org> on 2018/10/19 06:18:02 UTC
[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11734
Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
......................................................................
IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
Sentry stores the role names in lower case and Impala stores the role
names based on the original input role names. IMPALA-7343 introduced
a new bulk API (listAllRolesPrivileges) from Sentry that returns a map
of role name to a set of privileges. Since Impala preserves the case
sensitivity of the role names based on the original input role names,
this causes an issue when trying to retrieve a set of privileges from
a role name that is stored in Impala, especially when the role names in
Impala differ than the ones returned by listAllRolesPrivileges. This
issue will then result in privileges with mismatch role names to never
get refreshed in the Catalogd, which causes Impalad to wait indefinitely
waiting for the privileges to be updated by Catalogd. The fix is to get
a set of privileges using the role names returned by Sentry's
listAllRoles instead of using the role names stored in Impala.
Testing:
- Added a new E2E test
- Ran all E2E authorization tests
Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
---
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M tests/authorization/test_grant_revoke.py
2 files changed, 29 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/11734/4
--
To view, visit http://gerrit.cloudera.org:8080/11734
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11734 )
Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
......................................................................
Patch Set 9:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3335/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/11734
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 18:01:23 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/11734 )
Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
......................................................................
IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
Sentry stores the role names in lower case and Impala stores the role
names based on the original input role names. IMPALA-7343 introduced
a new bulk API (listAllRolesPrivileges) from Sentry that returns a map
of role name to a set of privileges. Since Impala preserves the case
sensitivity of the role names based on the original input role names,
this causes an issue when trying to retrieve a set of privileges from
a role name that is stored in Impala, especially when the role names in
Impala differ than the ones returned by listAllRolesPrivileges. This
issue will then result in privileges with mismatch role names to never
get refreshed in the Catalogd, which causes Impalad to wait indefinitely
waiting for the privileges to be updated by Catalogd. The fix is to get
a set of privileges using the role names returned by Sentry's
listAllRoles instead of using the role names stored in Impala.
Testing:
- Added a new E2E test
- Ran all E2E authorization tests
Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
---
M fe/src/main/java/org/apache/impala/catalog/Principal.java
M fe/src/main/java/org/apache/impala/catalog/Role.java
M fe/src/main/java/org/apache/impala/catalog/User.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M tests/authorization/test_grant_revoke.py
5 files changed, 71 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/11734/7
--
To view, visit http://gerrit.cloudera.org:8080/11734
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11734 )
Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
......................................................................
Patch Set 4:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/11734/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11734/4//COMMIT_MSG@10
PS4, Line 10: original input role names
I saw CatalogObjectCache is where users/roles are stored from AuthorizationPolicy. I see that the default, which is used, is to be case insensitive. So when it comes to names, they're lower-case in Impala. Issue is that it seems we sometimes use the name/key which is lowercase and sometimes the name from value (e.g., Principal) where case is preserved.
http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java
File fe/src/main/java/org/apache/impala/util/SentryProxy.java:
http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@172
PS4, Line 172: String
so this is lowercase or can it have casing that does not match the name that's stored in Role/User?
http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@185
PS4, Line 185: existingRole
... and this contains a name that is not lower-cased?
http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@186
PS4, Line 186: sentryRole.getRoleName()
afaict, this will get lowercased when performing the lookup.
http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@198
PS4, Line 198: getRoleName
... and this is lowercased?
http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:
http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py@386
PS4, Line 386: grp.getgrnam(
what's this?
http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py@388
PS4, Line 388: "invalidate metadata"
I think its good to have this test, but is there a more direct way to test this as well? For example, assign a privilege to a given role, then assign another privilege via sentry using different case. Will Impala see that the role then has both privs?
--
To view, visit http://gerrit.cloudera.org:8080/11734
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 07:06:07 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/11734 )
Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
......................................................................
Patch Set 7:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java
File fe/src/main/java/org/apache/impala/util/SentryProxy.java:
http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@172
PS4, Line 172: he key
> got it. keeping track of what's from sentry and what isn't and how that may
If for some reason, Sentry decides to make principal names to be case sensitive, I don't think we want to lower case the strings. Obviously when they do that, we also need to CatalogObjectCache to be case sensitive. Transforming the Sentry data structures into Impala data structures can be an expensive operations since we have to go through N privileges. I'll add more comments.
http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:
http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py@388
PS4, Line 388: tadata won't hang due
> sorry, meant role. this is an e2e test so if could be done here? anyways, i
Done
--
To view, visit http://gerrit.cloudera.org:8080/11734
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 17:14:22 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11734 )
Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
......................................................................
Patch Set 7:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11734/7/fe/src/main/java/org/apache/impala/util/SentryProxy.java
File fe/src/main/java/org/apache/impala/util/SentryProxy.java:
http://gerrit.cloudera.org:8080/#/c/11734/7/fe/src/main/java/org/apache/impala/util/SentryProxy.java@199
PS7, Line 199: // sentryRole.getRoleName() always returns the role name in lower case.
: // However role.getName() preserves the case sensitivity of the role name.
: // It is important to get the set of privileges from allRolesPrivileges using
: // sentryRole.getRoleName() instead of role.getName(). If Sentry changes
: // the role names to be upper case or case sensitive, we don't need to
: // change this particular code since the case for allRolesPrivileges' keys
: // will always match sentryRole.getRoleName().
terser at the call-sites: allRolesPrivileges keys and sentryRole are used here since they both come from Sentry so agree in case.
http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:
http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py@388
PS4, Line 388: tadata won't hang due
> Done
what was done?
--
To view, visit http://gerrit.cloudera.org:8080/11734
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 17:27:14 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/11734 )
Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
......................................................................
IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
Sentry stores the role names in lower case and Impala stores the role
names based on the original input role names. IMPALA-7343 introduced
a new bulk API (listAllRolesPrivileges) from Sentry that returns a map
of role name to a set of privileges. Since Impala preserves the case
sensitivity of the role names based on the original input role names,
this causes an issue when trying to retrieve a set of privileges from
a role name that is stored in Impala, especially when the role names in
Impala differ than the ones returned by listAllRolesPrivileges. This
issue will then result in privileges with mismatch role names to never
get refreshed in the Catalogd, which causes Impalad to wait indefinitely
waiting for the privileges to be updated by Catalogd. The fix is to get
a set of privileges using the role names returned by Sentry's
listAllRoles instead of using the role names stored in Impala.
Testing:
- Added a new E2E test
- Ran all E2E authorization tests
Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
---
M fe/src/main/java/org/apache/impala/catalog/Principal.java
M fe/src/main/java/org/apache/impala/catalog/Role.java
M fe/src/main/java/org/apache/impala/catalog/User.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M tests/authorization/test_grant_revoke.py
5 files changed, 61 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/11734/8
--
To view, visit http://gerrit.cloudera.org:8080/11734
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/11734 )
Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
......................................................................
Patch Set 8:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11734/7/fe/src/main/java/org/apache/impala/util/SentryProxy.java
File fe/src/main/java/org/apache/impala/util/SentryProxy.java:
http://gerrit.cloudera.org:8080/#/c/11734/7/fe/src/main/java/org/apache/impala/util/SentryProxy.java@199
PS7, Line 199: // allRolesPrivileges keys and sentryRole.getName() are used here since they both
: // come from Sentry so they agree in case.
: refreshPrivilegesInCatalog(sentryRole.getRoleName(), role, allRolesPrivileges);
: }
: return rolesToRemove;
: }
:
> terser at the call-sites: allRolesPrivileges keys and sentryRole are used h
Done. Comment updated.
http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:
http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py@388
PS4, Line 388: tadata won't hang due
> what was done?
I added a comment.
--
To view, visit http://gerrit.cloudera.org:8080/11734
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 17:45:29 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/11734 )
Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
......................................................................
Patch Set 4:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/11734/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11734/4//COMMIT_MSG@10
PS4, Line 10: original input role names
> I saw CatalogObjectCache is where users/roles are stored from Authorization
That's correct. The CatalogObjectCache stores can store they key in a case-sensitive way or case-insensitive way depending on the caseInsensitiveKeys_ flag. See: https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/catalog/CatalogObjectCache.java#L74. However, we store the TPrincipal name (https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/catalog/Principal.java#L47) in a case-preserving way. I also checked tested the code against the commit prior to the introduction of Principal and it looks like that's been the behavior for quite some time.
http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java
File fe/src/main/java/org/apache/impala/util/SentryProxy.java:
http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@172
PS4, Line 172: String
> so this is lowercase or can it have casing that does not match the name tha
This is lower case as of now. However, it's probably a good idea to not assume Sentry will always send this in a lower case.
http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@185
PS4, Line 185: existingRole
> ... and this contains a name that is not lower-cased?
Role.getName() will return the role name in a case-preserving way based on the original input role name.
http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@186
PS4, Line 186: sentryRole.getRoleName()
> afaict, this will get lowercased when performing the lookup.
That's correct if we're talking about getting the role name form the catalog.
http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@198
PS4, Line 198: getRoleName
> ... and this is lowercased?
Yes, this is lower case as of now. Since both listAllRoles and listAllRolesPriivleges come from Sentry, I would expect they will return the role names in a case-consistent manner.
http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@249
PS4, Line 249: sentryPrincipalName
This is pretty much the problem.
http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:
http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py@386
PS4, Line 386: grp.getgrnam(
> what's this?
It's to get a group name.
http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py@388
PS4, Line 388: "invalidate metadata"
> I think its good to have this test, but is there a more direct way to test
The issue is with role names having different cases and not with the privilege names. I can't think of an easy way of testing it without doing E2E test.
--
To view, visit http://gerrit.cloudera.org:8080/11734
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 15:20:23 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11734 )
Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
......................................................................
Patch Set 7:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/1107/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/11734
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 17:44:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11734 )
Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
......................................................................
IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
Sentry stores the role names in lower case and Impala stores the role
names based on the original input role names. IMPALA-7343 introduced
a new bulk API (listAllRolesPrivileges) from Sentry that returns a map
of role name to a set of privileges. Since Impala preserves the case
sensitivity of the role names based on the original input role names,
this causes an issue when trying to retrieve a set of privileges from
a role name that is stored in Impala, especially when the role names in
Impala differ than the ones returned by listAllRolesPrivileges. This
issue will then result in privileges with mismatch role names to never
get refreshed in the Catalogd, which causes Impalad to wait indefinitely
waiting for the privileges to be updated by Catalogd. The fix is to get
a set of privileges using the role names returned by Sentry's
listAllRoles instead of using the role names stored in Impala.
Testing:
- Added a new E2E test
- Ran all E2E authorization tests
Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Reviewed-on: http://gerrit.cloudera.org:8080/11734
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/catalog/Principal.java
M fe/src/main/java/org/apache/impala/catalog/Role.java
M fe/src/main/java/org/apache/impala/catalog/User.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M tests/authorization/test_grant_revoke.py
5 files changed, 61 insertions(+), 7 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/11734
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11734 )
Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
......................................................................
Patch Set 9: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/11734
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 21:48:27 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11734 )
Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1105/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/11734
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 06:54:44 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11734 )
Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
......................................................................
Patch Set 4:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java
File fe/src/main/java/org/apache/impala/util/SentryProxy.java:
http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@172
PS4, Line 172: String
> This is lower case as of now. However, it's probably a good idea to not ass
got it. keeping track of what's from sentry and what isn't and how that may or may not change over time is error prone. how about we just canonicalize up-front here to lower-case strings? perhaps a cleaner option is just have stuff that comes back from Sentry conform to data structures used in Impala. So in this case, that Map could be a CatalogObjectCache. If you canonicalize the map strings, then you'd need to lower-case in the look-up; if that cache is used, then it would just fall-out.
either way, but please add a comment in each of the places I raised a question since the assumptions are currently unclear and error-prone.
http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:
http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py@386
PS4, Line 386: grp.getgrnam(
> It's to get a group name.
oh... its from the import.
http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py@388
PS4, Line 388: "invalidate metadata"
> The issue is with role names having different cases and not with the privil
sorry, meant role. this is an e2e test so if could be done here? anyways, if its not possible or extremely difficult, pls add a comment/perhaps a todo. this seems a bit circuitous to debug a case issue.
--
To view, visit http://gerrit.cloudera.org:8080/11734
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 16:32:28 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11734 )
Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
......................................................................
Patch Set 8:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1109/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/11734
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 18:36:59 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11734 )
Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
......................................................................
Patch Set 8: Code-Review+2
thx for the fix!
--
To view, visit http://gerrit.cloudera.org:8080/11734
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 17:59:43 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11734 )
Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
......................................................................
Patch Set 7:
looks fine.. just one more clarification for the test.
--
To view, visit http://gerrit.cloudera.org:8080/11734
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 17:27:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11734 )
Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name
......................................................................
Patch Set 9: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11734
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 18:01:22 +0000
Gerrit-HasComments: No