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 2019/04/26 21:15:40 UTC
[Impala-ASF-CR] IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13134
Change subject: IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
......................................................................
IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
This patch adds support for Ranger cache invalidation via INVALIDATE
METADATA and REFRESH AUTHORIZATION. This patch introduces a new catalog
object type called AUTHZ_REFRESH to allow broadcasting messages from
Catalogd to Impalads to update their local Ranger caches. For better
user experience, every GRANT/REVOKE statement perform an authorization
refresh.
Testing:
- Replaced the sleep in test_ranger.py with INVALIDATE METADATA or
REFRESH AUTHORIZATION
- Ran all FE tests
- Ran all E2E authorization tests
Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java
A fe/src/main/java/org/apache/impala/catalog/AuthzRefresh.java
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M fe/src/main/java/org/apache/impala/service/FeCatalogManager.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java
M fe/src/test/resources/ranger-hive-security.xml
M tests/authorization/test_ranger.py
19 files changed, 389 insertions(+), 97 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/13134/5
--
To view, visit http://gerrit.cloudera.org:8080/13134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Gerrit-Change-Number: 13134
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
[Impala-ASF-CR] IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13134 )
Change subject: IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
......................................................................
Patch Set 9:
(1 comment)
Carry +1
http://gerrit.cloudera.org:8080/#/c/13134/8/fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java
File fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java:
http://gerrit.cloudera.org:8080/#/c/13134/8/fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java@204
PS8, Line 204: public void invalidateAuthorizationCache() {}
> nit: one line this method
Done
--
To view, visit http://gerrit.cloudera.org:8080/13134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Gerrit-Change-Number: 13134
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 30 Apr 2019 21:25:29 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13134 )
Change subject: IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
......................................................................
Patch Set 12:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4129/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/13134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Gerrit-Change-Number: 13134
Gerrit-PatchSet: 12
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 01 May 2019 05:56:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13134 )
Change subject: IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
......................................................................
Patch Set 11:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4128/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/13134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Gerrit-Change-Number: 13134
Gerrit-PatchSet: 11
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 01 May 2019 02:49:52 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13134 )
Change subject: IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
......................................................................
Patch Set 10:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/3022/ : 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/13134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Gerrit-Change-Number: 13134
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 01 May 2019 03:35:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13134 )
Change subject: IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
......................................................................
Patch Set 9:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/3017/ : 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/13134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Gerrit-Change-Number: 13134
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 30 Apr 2019 22:11:45 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13134 )
Change subject: IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
......................................................................
Patch Set 12: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/13134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Gerrit-Change-Number: 13134
Gerrit-PatchSet: 12
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 01 May 2019 11:12:09 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13134 )
Change subject: IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
......................................................................
Patch Set 10: Code-Review+2
Rebased to fix the merge conflict. Bumping it to +2 for +1s from Bharath and Austin.
--
To view, visit http://gerrit.cloudera.org:8080/13134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Gerrit-Change-Number: 13134
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 01 May 2019 02:49:36 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/13134 )
Change subject: IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
......................................................................
IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
This patch adds support for Ranger cache invalidation via INVALIDATE
METADATA and REFRESH AUTHORIZATION. This patch introduces a new catalog
object type called AUTHZ_CACHE_INVALIDATION to allow broadcasting
messages from Catalogd to Impalads to update their local Ranger caches.
For better user experience, every GRANT/REVOKE statement will invalidate
the Ranger authorization cache.
Testing:
- Replaced the sleep in test_ranger.py with INVALIDATE METADATA or
REFRESH AUTHORIZATION
- Ran all FE tests
- Ran all E2E authorization tests
Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
---
M be/src/catalog/catalog-util.cc
M common/thrift/CatalogObjects.thrift
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java
A fe/src/main/java/org/apache/impala/catalog/AuthzCacheInvalidation.java
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M fe/src/main/java/org/apache/impala/service/FeCatalogManager.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java
M fe/src/test/resources/ranger-hive-security.xml
M tests/authorization/test_ranger.py
20 files changed, 395 insertions(+), 97 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/13134/8
--
To view, visit http://gerrit.cloudera.org:8080/13134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Gerrit-Change-Number: 13134
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13134 )
Change subject: IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
......................................................................
Patch Set 11: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Gerrit-Change-Number: 13134
Gerrit-PatchSet: 11
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 01 May 2019 02:49:51 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13134 )
Change subject: IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
......................................................................
Patch Set 9: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/13134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Gerrit-Change-Number: 13134
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 30 Apr 2019 21:25:32 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/13134 )
Change subject: IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
......................................................................
IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
This patch adds support for Ranger cache invalidation via INVALIDATE
METADATA and REFRESH AUTHORIZATION. This patch introduces a new catalog
object type called AUTHZ_CACHE_INVALIDATION to allow broadcasting
messages from Catalogd to Impalads to update their local Ranger caches.
For better user experience, every GRANT/REVOKE statement will invalidate
the Ranger authorization cache.
Testing:
- Replaced the sleep in test_ranger.py with INVALIDATE METADATA or
REFRESH AUTHORIZATION
- Ran all FE tests
- Ran all E2E authorization tests
Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
---
M be/src/catalog/catalog-util.cc
M common/thrift/CatalogObjects.thrift
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java
A fe/src/main/java/org/apache/impala/catalog/AuthzCacheInvalidation.java
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M fe/src/main/java/org/apache/impala/service/FeCatalogManager.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java
M fe/src/test/resources/ranger-hive-security.xml
M tests/authorization/test_ranger.py
20 files changed, 348 insertions(+), 68 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/13134/10
--
To view, visit http://gerrit.cloudera.org:8080/13134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Gerrit-Change-Number: 13134
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13134 )
Change subject: IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
......................................................................
Patch Set 7:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/13134/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13134/5//COMMIT_MSG@11
PS5, Line 11: object type called AUTHZ_REFRESH to allow broadcasting messages from
: Catalogd to Impalads to update their local Ranger caches.
> Can you mention the granularity of invalidation? Is everything refreshed fo
TBH, I don't know the granularity of the invalidation since this is the implementation detail of Ranger. The good thing is if Ranger has a performance improvement in their refresh API, Impala will get that for free without knowing how it works.
http://gerrit.cloudera.org:8080/#/c/13134/5/common/thrift/CatalogObjects.thrift
File common/thrift/CatalogObjects.thrift:
http://gerrit.cloudera.org:8080/#/c/13134/5/common/thrift/CatalogObjects.thrift@42
PS5, Line 42: // A catalog
> nit: something like AUTHZ_CACHE_INVALIDATION? Also, given this is a 'specia
Done
http://gerrit.cloudera.org:8080/#/c/13134/5/common/thrift/CatalogObjects.thrift@591
PS5, Line 591: ft representa
> nit: same comment on naming. My point is that "refresh" is already confusin
Done
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java:
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@179
PS5, Line 179: plugin_.refreshPoliciesAndTags();
> thread-safe? What happens with authz requests in flight?
Yes, this is thread-safe now since this is the proper API from Ranger. It puts the requests into LinkedBlockingQueue
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java:
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@225
PS5, Line 225:
> Isn't this a no-op?
Yeah, I have removed it. Done.
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@689
PS5, Line 689: zCacheInvalidation:
> should we assert this of size 1?
Initially this will be 0, only an explicit grant or refresh authz/invalidate metadata will make this to have a size 1.
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2357
PS5, Line 2357:
> do we ever need to remove this?
Yeah this is not needed. Removed. Done.
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2362
PS5, Line 2362:
> Is this needed?
Removed this method. Done.
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
File fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java:
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java@371
PS5, Line 371: case AUTHZ_CACHE_INVALIDATION:
: removeAuthzCacheInvalidation(catalogObject.getAuthz_cache_invalidation(),
> like I commented elsewhere, do we ever need to drop this?
Probably not, but this is part removeCatalogObjects. So it's probably a good idea to just implement it instead of throwing an exception. Like we could have a new Impalad catalog implementation that clears everything and this may require every catalog object type to deal with the removal.
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/main/java/org/apache/impala/service/Frontend.java@266
PS5, Line 266: catalogManager_.setAuthzChecker(authzChecker_);
: authzManager_ = authzFactory.newAuthorizationManager(catalogManager_,
: authzChecker_::get);
> Curious if AuthzChecker can be a part of AuthzManager?
We don't use AuthorizationChecker in catalogd. So, it's probably good to split the two.
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/test/resources/ranger-hive-security.xml
File fe/src/test/resources/ranger-hive-security.xml:
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/test/resources/ranger-hive-security.xml@47
PS5, Line 47: 30000
> seems high, any particular reason to override?
This is actually the default polling time in Ranger. We changed it to 5 seconds to reduce the sleep time. Since we no longer need to sleep with this CR, I'm changing it back to 30 seconds. It's also good to test the code to make sure an explicit refresh works.
--
To view, visit http://gerrit.cloudera.org:8080/13134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Gerrit-Change-Number: 13134
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 30 Apr 2019 00:02:49 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13134 )
Change subject: IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
......................................................................
Patch Set 8:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/3012/ : 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/13134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Gerrit-Change-Number: 13134
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 30 Apr 2019 20:08:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
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/13134 )
Change subject: IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
......................................................................
IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
This patch adds support for Ranger cache invalidation via INVALIDATE
METADATA and REFRESH AUTHORIZATION. This patch introduces a new catalog
object type called AUTHZ_CACHE_INVALIDATION to allow broadcasting
messages from Catalogd to Impalads to update their local Ranger caches.
For better user experience, every GRANT/REVOKE statement will invalidate
the Ranger authorization cache.
Testing:
- Replaced the sleep in test_ranger.py with INVALIDATE METADATA or
REFRESH AUTHORIZATION
- Ran all FE tests
- Ran all E2E authorization tests
Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Reviewed-on: http://gerrit.cloudera.org:8080/13134
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/catalog/catalog-util.cc
M common/thrift/CatalogObjects.thrift
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java
A fe/src/main/java/org/apache/impala/catalog/AuthzCacheInvalidation.java
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M fe/src/main/java/org/apache/impala/service/FeCatalogManager.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java
M fe/src/test/resources/ranger-hive-security.xml
M tests/authorization/test_ranger.py
20 files changed, 348 insertions(+), 68 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/13134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Gerrit-Change-Number: 13134
Gerrit-PatchSet: 13
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/13134 )
Change subject: IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
......................................................................
IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
This patch adds support for Ranger cache invalidation via INVALIDATE
METADATA and REFRESH AUTHORIZATION. This patch introduces a new catalog
object type called AUTHZ_CACHE_INVALIDATION to allow broadcasting
messages from Catalogd to Impalads to update their local Ranger caches.
For better user experience, every GRANT/REVOKE statement will invalidate
the Ranger authorization cache.
Testing:
- Replaced the sleep in test_ranger.py with INVALIDATE METADATA or
REFRESH AUTHORIZATION
- Ran all FE tests
- Ran all E2E authorization tests
Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
---
M be/src/catalog/catalog-util.cc
M common/thrift/CatalogObjects.thrift
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java
A fe/src/main/java/org/apache/impala/catalog/AuthzCacheInvalidation.java
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M fe/src/main/java/org/apache/impala/service/FeCatalogManager.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java
M fe/src/test/resources/ranger-hive-security.xml
M tests/authorization/test_ranger.py
20 files changed, 393 insertions(+), 97 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/13134/9
--
To view, visit http://gerrit.cloudera.org:8080/13134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Gerrit-Change-Number: 13134
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13134 )
Change subject: IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
......................................................................
Patch Set 12: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Gerrit-Change-Number: 13134
Gerrit-PatchSet: 12
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 01 May 2019 05:56:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13134 )
Change subject: IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2984/ : 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/13134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Gerrit-Change-Number: 13134
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 30 Apr 2019 00:25:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
Posted by "Austin Nobis (Code Review)" <ge...@cloudera.org>.
Austin Nobis has posted comments on this change. ( http://gerrit.cloudera.org:8080/13134 )
Change subject: IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
......................................................................
Patch Set 8: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/13134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Gerrit-Change-Number: 13134
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 30 Apr 2019 21:52:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
Posted by "Austin Nobis (Code Review)" <ge...@cloudera.org>.
Austin Nobis has posted comments on this change. ( http://gerrit.cloudera.org:8080/13134 )
Change subject: IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
......................................................................
Patch Set 8:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13134/8/fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java
File fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java:
http://gerrit.cloudera.org:8080/#/c/13134/8/fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java@204
PS8, Line 204: public void invalidateAuthorizationCache() {
nit: one line this method
--
To view, visit http://gerrit.cloudera.org:8080/13134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Gerrit-Change-Number: 13134
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 30 Apr 2019 20:23:18 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13134 )
Change subject: IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
......................................................................
Patch Set 8: Code-Review+1
(7 comments)
Carry Bharath's +1.
http://gerrit.cloudera.org:8080/#/c/13134/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13134/7//COMMIT_MSG@11
PS7, Line 11: AUTHZ_CACHE_I
> update?
Done
http://gerrit.cloudera.org:8080/#/c/13134/7/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
File fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java:
http://gerrit.cloudera.org:8080/#/c/13134/7/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java@132
PS7, Line 132: an authorization cache
> not clear what this means.
Done
http://gerrit.cloudera.org:8080/#/c/13134/7/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java@134
PS7, Line 134: invalid
> nit: does it make sense to rename this too? (inline with invalidateAuthzCac
Done
http://gerrit.cloudera.org:8080/#/c/13134/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java:
http://gerrit.cloudera.org:8080/#/c/13134/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@179
PS7, Line 179: }
> Add some supportability logging, like SentryProxy?
Good idea. Done.
http://gerrit.cloudera.org:8080/#/c/13134/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java:
http://gerrit.cloudera.org:8080/#/c/13134/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@212
PS7, Line 212: // Add a single AUTHZ_CACHE_INVALIDATION catalog object called "ranger" and increment
> I think this could use a comment.
Done
http://gerrit.cloudera.org:8080/#/c/13134/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@222
PS7, Line 222: // Update the authorization cache invalidation marker so that the Impalads can
: // invalidate their Ranger caches. This is needed for usability reason to make s
> Mention that this is done for usability reasons and read-your-own-writes se
Done
http://gerrit.cloudera.org:8080/#/c/13134/7/fe/src/main/java/org/apache/impala/catalog/Catalog.java
File fe/src/main/java/org/apache/impala/catalog/Catalog.java:
http://gerrit.cloudera.org:8080/#/c/13134/7/fe/src/main/java/org/apache/impala/catalog/Catalog.java@551
PS7, Line 551: // Authorization cache invalidation requires a single catalog object and it
> Add a comment why this should always exist?
Done
--
To view, visit http://gerrit.cloudera.org:8080/13134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Gerrit-Change-Number: 13134
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 30 Apr 2019 19:21:29 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13134 )
Change subject: IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
......................................................................
Patch Set 5:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/13134/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13134/5//COMMIT_MSG@11
PS5, Line 11: object type called AUTHZ_REFRESH to allow broadcasting messages from
: Catalogd to Impalads to update their local Ranger caches.
Can you mention the granularity of invalidation? Is everything refreshed for every grant/revoke? If so, it is a performance problem?
http://gerrit.cloudera.org:8080/#/c/13134/5/common/thrift/CatalogObjects.thrift
File common/thrift/CatalogObjects.thrift:
http://gerrit.cloudera.org:8080/#/c/13134/5/common/thrift/CatalogObjects.thrift@42
PS5, Line 42: AUTHZ_REFRESH
nit: something like AUTHZ_CACHE_INVALIDATION? Also, given this is a 'special' kind of Catalog object, document what it does?
http://gerrit.cloudera.org:8080/#/c/13134/5/common/thrift/CatalogObjects.thrift@591
PS5, Line 591: TAuthzRefresh
nit: same comment on naming. My point is that "refresh" is already confusing enough in Impala's context.
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java:
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@179
PS5, Line 179: plugin_.refreshPoliciesAndTags();
thread-safe? What happens with authz requests in flight?
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java:
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@225
PS5, Line 225: response.result.setRemoved_catalog_objects(authzDelta.getCatalogObjectsRemoved());
Isn't this a no-op?
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@689
PS5, Line 689: getAllAuthzRefreshes
should we assert this of size 1?
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2357
PS5, Line 2357: removeAuthzRefresh
do we ever need to remove this?
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2362
PS5, Line 2362: authzRefresh.setCatalogVersion(incrementAndGetCatalogVersion());
Is this needed?
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
File fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java:
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java@371
PS5, Line 371: case AUTHZ_REFRESH:
: removeAuthzRefresh(catalogObject.getAuthz_refresh(), dropCatalogVersion);
like I commented elsewhere, do we ever need to drop this?
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/main/java/org/apache/impala/service/Frontend.java@266
PS5, Line 266: catalogManager_.setAuthzChecker(authzChecker_);
: authzManager_ = authzFactory.newAuthorizationManager(catalogManager_,
: authzChecker_::get);
Curious if AuthzChecker can be a part of AuthzManager?
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/test/resources/ranger-hive-security.xml
File fe/src/test/resources/ranger-hive-security.xml:
http://gerrit.cloudera.org:8080/#/c/13134/5/fe/src/test/resources/ranger-hive-security.xml@47
PS5, Line 47: 30000
seems high, any particular reason to override?
--
To view, visit http://gerrit.cloudera.org:8080/13134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Gerrit-Change-Number: 13134
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 29 Apr 2019 21:20:24 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13134 )
Change subject: IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
......................................................................
Patch Set 11:
> Patch Set 11: Verified-1
>
> Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4128/
04:27:41 ] Starting Impala cluster (logging to /home/ubuntu/Impala/logs/data_loading/start-impala-cluster.log)...
04:27:41 ] FAILED (Took: 0 min 11 sec)
04:27:41 ] 'start-impala' failed. Tail of log:
04:27:41 ] Log for command 'start-impala'
04:27:41 ] 03:17:39 MainThread: Starting State Store logging to /home/ubuntu/Impala/logs/data_loading/statestored.INFO
04:27:41 ] 03:17:49 MainThread: Error starting cluster
04:27:41 ] Traceback (most recent call last):
04:27:41 ] File "/home/ubuntu/Impala/bin/start-impala-cluster.py", line 665, in <module>
04:27:41 ] cluster_ops.start_statestore()
04:27:41 ] File "/home/ubuntu/Impala/bin/start-impala-cluster.py", line 402, in start_statestore
04:27:41 ] raise RuntimeError("Unable to start statestored. Check log or file permissions"
Not quite sure why. Will retry.
--
To view, visit http://gerrit.cloudera.org:8080/13134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Gerrit-Change-Number: 13134
Gerrit-PatchSet: 11
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 01 May 2019 05:56:32 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/13134 )
Change subject: IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
......................................................................
IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
This patch adds support for Ranger cache invalidation via INVALIDATE
METADATA and REFRESH AUTHORIZATION. This patch introduces a new catalog
object type called AUTHZ_REFRESH to allow broadcasting messages from
Catalogd to Impalads to update their local Ranger caches. For better
user experience, every GRANT/REVOKE statement perform an authorization
refresh.
Testing:
- Replaced the sleep in test_ranger.py with INVALIDATE METADATA or
REFRESH AUTHORIZATION
- Ran all FE tests
- Ran all E2E authorization tests
Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
---
M be/src/catalog/catalog-util.cc
M common/thrift/CatalogObjects.thrift
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java
A fe/src/main/java/org/apache/impala/catalog/AuthzCacheInvalidation.java
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M fe/src/main/java/org/apache/impala/service/FeCatalogManager.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java
M fe/src/test/resources/ranger-hive-security.xml
M tests/authorization/test_ranger.py
20 files changed, 378 insertions(+), 97 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/13134/7
--
To view, visit http://gerrit.cloudera.org:8080/13134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Gerrit-Change-Number: 13134
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13134 )
Change subject: IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2941/ : 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/13134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Gerrit-Change-Number: 13134
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 26 Apr 2019 22:05:14 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13134 )
Change subject: IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
......................................................................
Patch Set 11: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4128/
--
To view, visit http://gerrit.cloudera.org:8080/13134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Gerrit-Change-Number: 13134
Gerrit-PatchSet: 11
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 01 May 2019 04:27:42 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13134 )
Change subject: IMPALA-8293 (Part 2): Add support for Ranger cache invalidation
......................................................................
Patch Set 7: Code-Review+1
(7 comments)
Looks good to me, pending some suggestions. I'm +1'ing it to give other reviewers a chance, feel free to ugprade it to a +2 if no one else has any other comments.
http://gerrit.cloudera.org:8080/#/c/13134/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13134/7//COMMIT_MSG@11
PS7, Line 11: AUTHZ_REFRESH
update?
http://gerrit.cloudera.org:8080/#/c/13134/7/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
File fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java:
http://gerrit.cloudera.org:8080/#/c/13134/7/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java@132
PS7, Line 132: authorization refresh.
not clear what this means.
http://gerrit.cloudera.org:8080/#/c/13134/7/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java@134
PS7, Line 134: refresh
nit: does it make sense to rename this too? (inline with invalidateAuthzCache() or something)
http://gerrit.cloudera.org:8080/#/c/13134/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java:
http://gerrit.cloudera.org:8080/#/c/13134/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@179
PS7, Line 179: plugin_.refreshPoliciesAndTags();
Add some supportability logging, like SentryProxy?
Took..xxx ms.
http://gerrit.cloudera.org:8080/#/c/13134/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java:
http://gerrit.cloudera.org:8080/#/c/13134/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@212
PS7, Line 212: AuthorizationDelta authzDelta = new AuthorizationDelta();
I think this could use a comment.
http://gerrit.cloudera.org:8080/#/c/13134/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@222
PS7, Line 222: AuthorizationDelta authzDelta = refreshAuthorization(false);
: response.result.setUpdated_catalog_objects(authzDelta.getCatalogObjectsAdded());
Mention that this is done for usability reasons and read-your-own-writes semantics (consistent with Sentry)?
http://gerrit.cloudera.org:8080/#/c/13134/7/fe/src/main/java/org/apache/impala/catalog/Catalog.java
File fe/src/main/java/org/apache/impala/catalog/Catalog.java:
http://gerrit.cloudera.org:8080/#/c/13134/7/fe/src/main/java/org/apache/impala/catalog/Catalog.java@551
PS7, Line 551: throw new CatalogException("Authz cache invalidation not found: " +
Add a comment why this should always exist?
--
To view, visit http://gerrit.cloudera.org:8080/13134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7160c082298e0b8cc2742dd3facbd4978581288
Gerrit-Change-Number: 13134
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 30 Apr 2019 18:39:01 +0000
Gerrit-HasComments: Yes