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 <[email protected]> Gerrit-Reviewer: Austin Nobis <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Comment-Date: Tue, 30 Apr 2019 19:21:29 +0000 Gerrit-HasComments: Yes
