Hao Hao has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13552 )
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 an enum 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 when 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:418] Time spent Listing tables: real 24.735s user 0.033s sys 0.009s 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:418] Time spent Listing tables: real 47.543s user 0.040s sys 0.013s Change-Id: Icec75ae9e5626c887af37568a6f64a8361d888b7 Reviewed-on: http://gerrit.cloudera.org:8080/13552 Reviewed-by: Adar Dembo <a...@cloudera.com> Reviewed-by: Alexey Serbin <aser...@cloudera.com> Tested-by: Kudu Jenkins --- 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, 106 insertions(+), 63 deletions(-) Approvals: Adar Dembo: Looks good to me, but someone else must approve Alexey Serbin: Looks good to me, approved Kudu Jenkins: Verified -- 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: merged Gerrit-Change-Id: Icec75ae9e5626c887af37568a6f64a8361d888b7 Gerrit-Change-Number: 13552 Gerrit-PatchSet: 8 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)
