BroadAuthzScopeBenchmark.diff

diff --git a/src/kudu/master/sentry_authz_provider-test.cc b/src/kudu/master/sentry_authz_provider-test.cc
index e7304ab..42ad8f0 100644
--- a/src/kudu/master/sentry_authz_provider-test.cc
+++ b/src/kudu/master/sentry_authz_provider-test.cc
@@ -59,6 +59,7 @@
 #include "kudu/util/random.h"
 #include "kudu/util/random_util.h"
 #include "kudu/util/status.h"
+#include "kudu/util/stopwatch.h"
 #include "kudu/util/test_macros.h"
 #include "kudu/util/test_util.h"
 #include "kudu/util/ttl_cache.h"
@@ -69,6 +70,7 @@ DECLARE_uint32(sentry_privileges_cache_capacity_mb);
 DECLARE_string(sentry_service_rpc_addresses);
 DECLARE_string(server_name);
 DECLARE_string(trusted_user_acl);
+DECLARE_bool(sentry_narrow_fetch);
 
 METRIC_DECLARE_counter(sentry_client_tasks_successful);
 METRIC_DECLARE_counter(sentry_client_tasks_failed_fatal);
@@ -208,7 +210,7 @@ class SentryAuthzProviderTest : public SentryTestBase {
   }
 
   virtual bool CachingEnabled() const {
-    return true;
+    return false;
   }
 
   Status StopSentry() {
@@ -727,6 +729,30 @@ TEST_F(SentryAuthzProviderTest, TestPrivilegeCaseSensitivity) {
   ASSERT_OK(sentry_authz_provider_->AuthorizeCreateTable("DB.table", kTestUser, kTestUser));
 }
 
+// Benchmark to test the time it takes to evaluate privileges when requesting
+// privileges for braod authorization scopes (e.g. SERVER, DATABASE).
+TEST_F(SentryAuthzProviderTest, BroadAuthzScopeBenchmark) {
+  ASSERT_OK(CreateRoleAndAddToGroups());
+  // Create a database with a bunch tables in it.
+  int kNumTables = 20000;
+  for (int i = 0; i < kNumTables; i++) {
+    KLOG_EVERY_N_SECS(INFO, 3) << Substitute("num tables granted: $0", i);
+    ASSERT_OK(AlterRoleGrantPrivilege(
+        GetTablePrivilege("dbwithalongname", Substitute("tablewithalongname$0", i), "OWNER")));
+  }
+  FLAGS_sentry_narrow_fetch = false;
+  LOG_TIMING(INFO, "Getting database privileges") {
+    Status s = sentry_authz_provider_->AuthorizeCreateTable(
+        "dbwithalongname.table", kTestUser, kTestUser);
+  }
+
+  FLAGS_sentry_narrow_fetch = true;
+  LOG_TIMING(INFO, "Getting database privileges") {
+    Status s = sentry_authz_provider_->AuthorizeCreateTable(
+        "dbwithalongname.table", kTestUser, kTestUser);
+  }
+}
+
 // Verify the behavior of the SentryAuthzProvider's cache upon fetching
 // privilege information on authorizables of the TABLE scope in
 // 'adjacent branches' of the authz hierarchy.
diff --git a/src/kudu/master/sentry_privileges_fetcher.cc b/src/kudu/master/sentry_privileges_fetcher.cc
index 5d54999..b5f29c2 100644
--- a/src/kudu/master/sentry_privileges_fetcher.cc
+++ b/src/kudu/master/sentry_privileges_fetcher.cc
@@ -137,6 +137,8 @@ DEFINE_uint32(sentry_privileges_cache_scrubbing_period_sec, 20,
               "cache.");
 TAG_FLAG(sentry_privileges_cache_scrubbing_period_sec, advanced);
 
+DEFINE_bool(sentry_narrow_fetch, false, "");
+
 DEFINE_uint32(sentry_privileges_cache_max_scrubbed_entries_per_pass, 32,
               "Maximum number of entries in the privileges cache to process "
               "in one pass of the periodic scrubbing task. A value of 0 means "
@@ -208,28 +210,37 @@ GROUP_FLAG_VALIDATOR(sentry_service_rpc_addresses,
 
 namespace {
 
-// Returns an authorizable based on the table identifier (in the format
-// <database-name>.<table-name>) and the given scope.
-Status GetAuthorizable(const string& table_ident,
+// Fetching privileges from Sentry gets more expensive the broader the scope of
+// the authorizable is, since the API used in a fetch returns all ancestors and
+// all descendents of an authorizable in its hierarchy tree.
+//
+// Even if requesting privileges at a relatively broad scope, e.g. DATABASE,
+// fill in the authorizable to request a narrower scope, since the broader
+// privileges (i.e. the ancestors) will be returned from Sentry anyway.
+void NarrowAuthzScopeForFetch(const string& db, const string& table,
+                              TSentryAuthorizable* authorizable) {
+  if (authorizable->db.empty()) {
+    authorizable->__set_db(db);
+  }
+  if (authorizable->table.empty()) {
+    authorizable->__set_table(table);
+  }
+}
+
+// Returns an authorizable based on the given database and table name and the
+// given scope.
+Status GetAuthorizable(const string& db, const string& table,
                        SentryAuthorizableScope::Scope scope,
                        TSentryAuthorizable* authorizable) {
-  Slice database;
-  Slice table;
   // We should only ever request privileges from Sentry for authorizables of
   // scope equal to or higher than 'TABLE'.
   DCHECK_NE(scope, SentryAuthorizableScope::Scope::COLUMN);
   switch (scope) {
     case SentryAuthorizableScope::Scope::TABLE:
-      RETURN_NOT_OK(ParseHiveTableIdentifier(table_ident, &database, &table));
-      DCHECK(!table.empty());
-      authorizable->__set_table(table.ToString());
+      authorizable->__set_table(table);
       FALLTHROUGH_INTENDED;
     case SentryAuthorizableScope::Scope::DATABASE:
-      if (database.empty() && table.empty()) {
-        RETURN_NOT_OK(ParseHiveTableIdentifier(table_ident, &database, &table));
-      }
-      DCHECK(!database.empty());
-      authorizable->__set_db(database.ToString());
+      authorizable->__set_db(db);
       FALLTHROUGH_INTENDED;
     case SentryAuthorizableScope::Scope::SERVER:
       authorizable->__set_server(FLAGS_server_name);
@@ -585,11 +596,19 @@ Status SentryPrivilegesFetcher::ResetCache() {
 
 Status SentryPrivilegesFetcher::GetSentryPrivileges(
     SentryAuthorizableScope::Scope requested_scope,
-    const string& table_name,
+    const string& table_ident,
     const string& user,
     SentryPrivilegesBranch* privileges) {
+  Slice db_slice;
+  Slice table_slice;
+  RETURN_NOT_OK(ParseHiveTableIdentifier(table_ident, &db_slice, &table_slice));
+  DCHECK(!table_slice.empty());
+  DCHECK(!db_slice.empty());
+  const string table = table_slice.ToString();
+  const string db = db_slice.ToString();
+
   TSentryAuthorizable authorizable;
-  RETURN_NOT_OK(GetAuthorizable(table_name, requested_scope, &authorizable));
+  RETURN_NOT_OK(GetAuthorizable(db, table, requested_scope, &authorizable));
 
   if (PREDICT_FALSE(requested_scope == SentryAuthorizableScope::SERVER &&
                     !IsGTest())) {
@@ -599,7 +618,7 @@ Status SentryPrivilegesFetcher::GetSentryPrivileges(
     // of the SERVER scope unless this method is called from test code.
     LOG(DFATAL) << Substitute(
         "requesting privileges of the SERVER scope from Sentry "
-        "on authorizable '$0' for user '$1'", table_name, user);
+        "on authorizable '$0' for user '$1'", table_ident, user);
   }
 
   // Not expecting requests for authorizables of the scope narrower than TABLE,
@@ -673,6 +692,9 @@ Status SentryPrivilegesFetcher::GetSentryPrivileges(
     return Status::OK();
   }
 
+  if (FLAGS_sentry_narrow_fetch) {
+    NarrowAuthzScopeForFetch(db, table, &authorizable);
+  }
   TRACE("Fetching privileges from Sentry");
   const auto s = FetchPrivilegesFromSentry(FLAGS_kudu_service_name,
                                            user, authorizable,
diff --git a/src/kudu/master/sentry_privileges_fetcher.h b/src/kudu/master/sentry_privileges_fetcher.h
index 05b5f3a..a1b704c 100644
--- a/src/kudu/master/sentry_privileges_fetcher.h
+++ b/src/kudu/master/sentry_privileges_fetcher.h
@@ -169,7 +169,7 @@ class SentryPrivilegesFetcher {
   // in the cache.
   Status GetSentryPrivileges(
       sentry::SentryAuthorizableScope::Scope requested_scope,
-      const std::string& table_name,
+      const std::string& table_ident,
       const std::string& user,
       SentryPrivilegesBranch* privileges);