Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15206 )

Change subject: KUDU-2972 Add Ranger client
......................................................................


Patch Set 26: Code-Review+2

(1 comment)

LGTM, but will leave open in case Adar or Hao have further feedback.

http://gerrit.cloudera.org:8080/#/c/15206/23/src/kudu/ranger/ranger_client.cc
File src/kudu/ranger/ranger_client.cc:

http://gerrit.cloudera.org:8080/#/c/15206/23/src/kudu/ranger/ranger_client.cc@118
PS23, Line 118:  TODO(abukor): refactor to avoid code duplication
              : Status RangerClient::AuthorizeAction(const string& user_name,
              :                                      const ActionPB& action,
              :                                      const string& table_name) {
              :   string db;
              :   Slice tbl;
              :
              :   auto s = ParseRangerTableIdentifier(table_name, &db, &tbl);
              :   if (PREDICT_FALSE(!s.ok())) {
              :     LOG(WARNING) << Substitute(kDenyNonRangerTableTemplate, 
table_name);
              :     return Status::NotAuthorized(kUnauthorizedAction);
              :   }
              :
              :   RangerRequestListPB req_list;
              :   RangerResponseListPB resp_list;
              :   req_list.set_user(user_name);
              :
              :   RangerRequestPB* req = req_list.add_requests();
              :
              :   req->set_action(action);
              :   req->set_database(db);
              :   req->set_table(tbl.ToString());
              :
              :   RETURN_NOT_OK(subprocess_.Execute(req_list, &resp_list));
              :
              :   CHECK_EQ(1, resp_list.responses_size());
              :   if (resp_list.responses().begin()->allowed()) {
              :     return Status::OK();
              :   }
> as discussed offline it seems there's a significant (~20%) perf impact betw
Curious how we'd fare if we used a FixedBitSet<ActionPB, ActionPB_ARRAYSIZE> 
instead of std::unordered_set here, though as we discussed, I'm glad to merge 
this without these improvements.



--
To view, visit http://gerrit.cloudera.org:8080/15206
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2e1ec19ed3aeb4d82ad38fe1fb655f57021c1a4
Gerrit-Change-Number: 15206
Gerrit-PatchSet: 26
Gerrit-Owner: Attila Bukor <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: Hao Hao <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 05 Mar 2020 02:28:10 +0000
Gerrit-HasComments: Yes

Reply via email to