Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16160 )
Change subject: Add refreshing Ranger policies ...................................................................... Patch Set 1: (7 comments) http://gerrit.cloudera.org:8080/#/c/16160/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16160/1//COMMIT_MSG@24 PS1, Line 24: This means that even if Ranger itself is down, we : can still get an OK response when calling this method and there's no way : to guarantee clearing the cache. > Can we also clear the local cached state of the Ranger client when requesti I don't think we can, this is why we initially didn't implement this (Ranger doesn't seem to support cache invalidation, only best effort refreshing of policies). http://gerrit.cloudera.org:8080/#/c/16160/1/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/ranger/authorization/RangerKuduAuthorizer.java File java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/ranger/authorization/RangerKuduAuthorizer.java: http://gerrit.cloudera.org:8080/#/c/16160/1/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/ranger/authorization/RangerKuduAuthorizer.java@128 PS1, Line 128: plugin.refreshPoliciesAndTags(); > It looks like this method catches any exceptions and only logs: It only catches InterruptedExceptions though, other exceptions are not even thrown. I think this is a design decision and if they start throwing exceptions it would be a breaking change on their side, so I'm not sure if it makes sense to open a JIRA. http://gerrit.cloudera.org:8080/#/c/16160/1/src/kudu/master/master.proto File src/kudu/master/master.proto: http://gerrit.cloudera.org:8080/#/c/16160/1/src/kudu/master/master.proto@942 PS1, Line 942: message ResetAuthzCacheRequestPB { > Similar to the tools comments, should this be Refresh? Done http://gerrit.cloudera.org:8080/#/c/16160/1/src/kudu/ranger/mini_ranger.h File src/kudu/ranger/mini_ranger.h: http://gerrit.cloudera.org:8080/#/c/16160/1/src/kudu/ranger/mini_ranger.h@191 PS1, Line 191: 200ms so that tests don't have to wait too long until freshly : // created policies can be used. > Adjust this comment. Is 30s matching the ranger default? Done http://gerrit.cloudera.org:8080/#/c/16160/1/src/kudu/ranger/ranger_client.cc File src/kudu/ranger/ranger_client.cc: http://gerrit.cloudera.org:8080/#/c/16160/1/src/kudu/ranger/ranger_client.cc@342 PS1, Line 342: "-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=5005", > What is this change for? Oops left that in by mistake. It was for attaching a debugger to the subprocess. http://gerrit.cloudera.org:8080/#/c/16160/1/src/kudu/tools/tool_action_master.cc File src/kudu/tools/tool_action_master.cc: http://gerrit.cloudera.org:8080/#/c/16160/1/src/kudu/tools/tool_action_master.cc@319 PS1, Line 319: Reset > Is "refresh" a more accurate description? If I understand correctly this wi Done http://gerrit.cloudera.org:8080/#/c/16160/1/src/kudu/tools/tool_action_master.cc@329 PS1, Line 329: a Kudu Master > The actions operate on all the kudu masters right? Done -- To view, visit http://gerrit.cloudera.org:8080/16160 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie90b1c23b92060e081ab514fbae417b4f31c2b66 Gerrit-Change-Number: 16160 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Bukor <abu...@apache.org> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Attila Bukor <abu...@apache.org> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Greg Solovyev <gsolov...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 09 Jul 2020 15:22:48 +0000 Gerrit-HasComments: Yes
