Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Adar Dembo, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13566
to look at the new patch set (#3).
Change subject: authz: refactor authorization for ListTables
......................................................................
authz: refactor authorization for ListTables
Authorization for ListTables isn't very performant because it requires
authorizing each table while holding its table lock. This invariant is
held to guarantee that the authorization applies to the correct table in
Kudu (e.g. even amidst a concurrent rename).
If we authorize multiple tables at once, we can improve performance, but
keeping the guarantee is less straightforward. This patch takes a stab
at this by refactoring ListTables to prepare for the following
authorization sequence:
1. With table locks held, put together maps from
{ table_name => TableInfo } and { table_name => table_id }.
2. Authorize the tables (potentially in bulk in the future), keeping
track of the authorized table names.
3. Iterate through the authorized table names and, using the maps from
Step 1, with table locks held, check that the table name hasn't
changed. If it has, this implies there was a concurrent rename (or
equivalent) and the authorization call was not actually made for this
table, and it shouldn't be returned.
4. With confirmation that the tables that were authorized have the IDs
we expect, return the authorized tables.
If the catalog manager isn't set up to authorize tables (e.g.
authorization isn't enabled), steps 2 and 3 are skipped. This patch
refactors ListTables to match this sequence of events without bulk
authorization.
While this is mainly a refactor, the user-facing change is that if there
is a concurrent rename during a ListTables operation, there is a chance
that neither the old nor new table will show up in the listed tables.
This isn't ideal UX, but is conservative w.r.t security. A test is added
exercising this scenario.
Change-Id: I568e1be7b909768a99f0f4f13775e34a01ccd160
---
M src/kudu/integration-tests/master_sentry-itest.cc
M src/kudu/master/authz_provider.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/default_authz_provider.h
M src/kudu/master/sentry_authz_provider.cc
M src/kudu/master/sentry_authz_provider.h
7 files changed, 117 insertions(+), 19 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/13566/3
--
To view, visit http://gerrit.cloudera.org:8080/13566
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I568e1be7b909768a99f0f4f13775e34a01ccd160
Gerrit-Change-Number: 13566
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <hao....@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
