Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13552
to look at the new patch set (#2).
Change subject: sentry: allow caching of COLUMN/TABLE privileges when checking
higher scopes
......................................................................
sentry: allow caching of COLUMN/TABLE privileges when checking higher scopes
Currently, Kudu will not cache COLUMN/TABLE when checking for
DATABASE/SERVER Sentry privileges. This is because, when written, Kudu
would send requests that would yield significantly more privileges than
required. Commit 0040a731e4741bde6d8c9d81e796cb000cd24817 changed this
behavior, and as such, this patch gates the optimization behind a
boolean in the Sentry provider's Authorize() signature.
The effect of this is that when checking for DATABASE/SERVER privileges,
two entries will be added to the privilege cache instead of one: one for
the DATABASE/SERVER privileges, and one for the TABLE/COLUMN privileges.
This is optional because in some cases, it isn't necessarily desirable
to cache table-level privileges. E.g. when creating a table, we
shouldn't cache table-level privileges because Sentry may create OWNER
privileges for the new table that caching may miss out on when first
authorizing the create request.
This improves the worst case performance of ListTables where there are
many databases on which the user has no privileges, and a single table
per database. In this scenario, without this patch, there would be
2 * (# tables) calls to Sentry, and with this patch, there would be
1 * (# tables) calls.
With this patch:
./bin/sentry_authz_provider-test --gtest_filter=*ListTablesBench*
--num_databases=300 --num_tables_per_db=1 --has-db-privileges=false
sentry_authz_provider-test.cc:424] Time spent Listing tables: real 23.203s
user 0.028s sys 0.004s
Without this patch:
./bin/sentry_authz_provider-test --gtest_filter=*ListTablesBench*
--num_databases=300 --num_tables_per_db=1 --has-db-privileges=false
sentry_authz_provider-test.cc:420] Time spent Listing tables: real 44.448s
user 0.037s sys 0.018s
Change-Id: Icec75ae9e5626c887af37568a6f64a8361d888b7
---
M src/kudu/master/sentry_authz_provider-test.cc
M src/kudu/master/sentry_authz_provider.cc
M src/kudu/master/sentry_authz_provider.h
M src/kudu/master/sentry_privileges_fetcher.cc
M src/kudu/master/sentry_privileges_fetcher.h
5 files changed, 65 insertions(+), 45 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/13552/2
--
To view, visit http://gerrit.cloudera.org:8080/13552
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icec75ae9e5626c887af37568a6f64a8361d888b7
Gerrit-Change-Number: 13552
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Hao Hao <hao....@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
