You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by gr...@apache.org on 2019/06/06 23:57:29 UTC
[kudu] branch master updated (1865bfa -> 5c87afd)
This is an automated email from the ASF dual-hosted git repository.
granthenke pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git.
from 1865bfa memrowset: skip all uncommitted mutations when iterating
new a76177f hms: allow for tooling to run without Kudu plugin
new 60cc7f3 sentry: don't send requests for DATABASE/SERVER privileges
new 5c87afd KUDU-2785: Add splitSizeBytes to the backup job
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../org/apache/kudu/backup/KuduBackupRDD.scala | 4 ++
.../scala/org/apache/kudu/backup/Options.scala | 13 +++++-
.../org/apache/kudu/backup/TestKuduBackup.scala | 34 +++++++++++++++
src/kudu/common/common.proto | 14 +++++-
src/kudu/hms/hms_catalog.cc | 3 +-
src/kudu/hms/hms_catalog.h | 7 ++-
src/kudu/hms/hms_client-test.cc | 4 ++
src/kudu/hms/hms_client.cc | 7 ++-
src/kudu/hms/hms_client.h | 1 +
src/kudu/hms/mini_hms.cc | 27 +++++++-----
src/kudu/hms/mini_hms.h | 6 +++
src/kudu/master/catalog_manager.cc | 3 +-
src/kudu/master/master.cc | 22 ----------
src/kudu/master/master_main.cc | 25 +++++++++++
src/kudu/master/sentry_authz_provider-test.cc | 33 ++++++++++++++
src/kudu/master/sentry_privileges_fetcher.cc | 50 +++++++++++++++-------
src/kudu/master/sentry_privileges_fetcher.h | 2 +-
src/kudu/mini-cluster/external_mini_cluster.cc | 7 ++-
src/kudu/server/server_base.cc | 1 -
src/kudu/thrift/client.h | 4 ++
src/kudu/tools/kudu-tool-test.cc | 49 +++++++++++----------
21 files changed, 236 insertions(+), 80 deletions(-)
[kudu] 03/03: KUDU-2785: Add splitSizeBytes to the backup job
Posted by gr...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
granthenke pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 5c87afd4f2160344d651948e90234ee512adcfd8
Author: Grant Henke <gr...@apache.org>
AuthorDate: Tue Jun 4 10:46:51 2019 -0500
KUDU-2785: Add splitSizeBytes to the backup job
This patch adds an experimental and hidden option
to use the new scanner splitSizeBytes feature in the
backup job.
Change-Id: If6b8d02b71b1f463e4d0d9e04203d8edbd5e016b
Reviewed-on: http://gerrit.cloudera.org:8080/13511
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mp...@apache.org>
---
.../org/apache/kudu/backup/KuduBackupRDD.scala | 4 +++
.../scala/org/apache/kudu/backup/Options.scala | 13 ++++++++-
.../org/apache/kudu/backup/TestKuduBackup.scala | 34 ++++++++++++++++++++++
3 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackupRDD.scala b/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackupRDD.scala
index c9837ce..a9a7072 100644
--- a/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackupRDD.scala
+++ b/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackupRDD.scala
@@ -57,6 +57,10 @@ class KuduBackupRDD private[kudu] (
.prefetching(options.scanPrefetching)
.keepAlivePeriodMs(options.keepAlivePeriodMs)
+ options.splitSizeBytes.foreach { size =>
+ builder.setSplitSizeBytes(size)
+ }
+
// Set a hybrid time for the scan to ensure application consistency.
val toMicros = TimeUnit.MILLISECONDS.toMicros(options.toMs)
val toHTT =
diff --git a/java/kudu-backup/src/main/scala/org/apache/kudu/backup/Options.scala b/java/kudu-backup/src/main/scala/org/apache/kudu/backup/Options.scala
index 67cd261..1a8b3b9 100644
--- a/java/kudu-backup/src/main/scala/org/apache/kudu/backup/Options.scala
+++ b/java/kudu-backup/src/main/scala/org/apache/kudu/backup/Options.scala
@@ -39,7 +39,8 @@ case class BackupOptions(
scanPrefetching: Boolean = BackupOptions.DefaultScanPrefetching,
keepAlivePeriodMs: Long = BackupOptions.DefaultKeepAlivePeriodMs,
failOnFirstError: Boolean = BackupOptions.DefaultFailOnFirstError,
- numParallelBackups: Int = BackupOptions.DefaultNumParallelBackups)
+ numParallelBackups: Int = BackupOptions.DefaultNumParallelBackups,
+ splitSizeBytes: Option[Long] = None)
object BackupOptions {
val DefaultForceFull: Boolean = false
@@ -54,6 +55,7 @@ object BackupOptions {
val DefaultKeepAlivePeriodMs: Long = AsyncKuduClient.DEFAULT_KEEP_ALIVE_PERIOD_MS
val DefaultFailOnFirstError: Boolean = false
val DefaultNumParallelBackups = 1
+ val DefaultSplitSizeBytes: Option[Long] = None
// We use the program name to make the help output show a the spark invocation required.
val ClassName: String = KuduBackup.getClass.getCanonicalName.dropRight(1) // Remove trailing `$`
@@ -146,6 +148,15 @@ object BackupOptions {
.hidden()
.optional()
+ opt[Long]("splitSizeBytes")
+ .action((v, o) => o.copy(splitSizeBytes = Some(v)))
+ .text(
+ "Sets the target number of bytes per spark task. If set, tablet's primary key range " +
+ "will be split to generate uniform task sizes instead of the default of 1 task per " +
+ "tablet. This option is experimental.")
+ .hidden()
+ .optional()
+
help("help").text("prints this usage text")
arg[String]("<table>...")
diff --git a/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala b/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
index 566e630..2e278bb 100644
--- a/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
+++ b/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
@@ -29,8 +29,10 @@ import org.apache.kudu.Schema
import org.apache.kudu.Type
import org.apache.kudu.ColumnSchema.ColumnSchemaBuilder
import org.apache.kudu.spark.kudu.SparkListenerUtil.withJobDescriptionCollector
+import org.apache.kudu.spark.kudu.SparkListenerUtil.withJobTaskCounter
import org.apache.kudu.spark.kudu._
import org.apache.kudu.test.CapturingLogAppender
+import org.apache.kudu.test.KuduTestHarness.TabletServerConfig
import org.apache.kudu.test.RandomUtils
import org.apache.kudu.util.DataGenerator.DataGeneratorBuilder
import org.apache.kudu.util.HybridTimeUtil
@@ -323,6 +325,38 @@ class TestKuduBackup extends KuduTestSuite {
}
}
+ @TabletServerConfig(
+ flags = Array(
+ "--flush_threshold_mb=1",
+ "--flush_threshold_secs=1",
+ // Disable rowset compact to prevent DRSs being merged because they are too small.
+ "--enable_rowset_compaction=false"
+ ))
+ @Test
+ def testBackupWithSplitSizeBytes() {
+ // Create a table with a single partition.
+ val tableName = "split-size-table"
+ val options = new CreateTableOptions().setRangePartitionColumns(List("key").asJava)
+ val table = kuduClient.createTable(tableName, schema, options)
+
+ // Insert enough data into the test table so we can split it.
+ val rowCount = 1000
+ upsertRowsWithRowDataSize(table, rowCount, 32 * 1024)
+
+ // Wait for mrs flushed.
+ Thread.sleep(5 * 1000)
+
+ // Run a backup job with custom splitSizeBytes and count the tasks.
+ val backupOptions = createBackupOptions(Seq(tableName)).copy(splitSizeBytes = Some(1024))
+ val actualNumTasks = withJobTaskCounter(ss.sparkContext) { () =>
+ assertEquals(0, runBackup(backupOptions))
+ }
+ validateBackup(backupOptions, rowCount, false)
+
+ // Verify there were more tasks than there are partitions.
+ assertTrue(actualNumTasks > 1)
+ }
+
@Test
def testBackupAndRestoreTableWithManyPartitions(): Unit = {
val kNumPartitions = 100
[kudu] 01/03: hms: allow for tooling to run without Kudu plugin
Posted by gr...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
granthenke pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit a76177f615ab1936ff37679e551986526ba062bd
Author: Andrew Wong <aw...@apache.org>
AuthorDate: Mon Jun 3 21:36:59 2019 -0400
hms: allow for tooling to run without Kudu plugin
Currently, whenever an HMS client successfully connects to the HMS, it
checks the various HMS service configurations (e.g. that it is using the
Kudu-HMS plugin), returning an error if any are misconfigured. This is
important to make it much more obvious when the Kudu Master's HMS
synchronization is misconfigured.
It is still useful for other HMS clients (e.g. that used by the HMS
tooling) to operate on an HMS instance that is not configured with the
Kudu-HMS plugin et al.
This patch removes the requirement by plumbing the option to the HMS
client as a member of ThriftOptions. This was the most straightforward
way to plumb this option from the HmsCatalog to the HmsClient, given the
templating layered in between them for HA. Besides, we can use this
option in the future if we ever want to verify the configuration of
Thrift-based clients for other services (e.g. Sentry).
This patch additionally allows the -hive_metastore_sasl_enabled flag to
be used without the -keytab_file flag if not running kudu-master. To get
this behavior, I've moved the gflag validator into master_main.cc, which
is not built by tooling. I manually tested that it works, i.e. that
tooling will not validate and that a master will.
To test, I added an HmsMode that starts the HMS without the Kudu-HMS
plugin installed and used it in a couple of HMS tooling tests. I
considered reusing the ENABLE_HIVE_METASTORE HmsMode, but opted not to
since some tests are greatly simplified by ENABLE_HIVE_METASTORE having
the Kudu-HMS plugin installed (e.g. restarting the HMS isn't required to
enable the Kudu-HMS integration).
Change-Id: I9b9968bf0f8a55859a14421beda05cab3496b6c0
Reviewed-on: http://gerrit.cloudera.org:8080/13510
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
src/kudu/common/common.proto | 14 +++++++-
src/kudu/hms/hms_catalog.cc | 3 +-
src/kudu/hms/hms_catalog.h | 7 +++-
src/kudu/hms/hms_client-test.cc | 4 +++
src/kudu/hms/hms_client.cc | 7 +++-
src/kudu/hms/hms_client.h | 1 +
src/kudu/hms/mini_hms.cc | 27 ++++++++------
src/kudu/hms/mini_hms.h | 6 ++++
src/kudu/master/catalog_manager.cc | 3 +-
src/kudu/master/master.cc | 22 ------------
src/kudu/master/master_main.cc | 25 +++++++++++++
src/kudu/mini-cluster/external_mini_cluster.cc | 7 +++-
src/kudu/server/server_base.cc | 1 -
src/kudu/thrift/client.h | 4 +++
src/kudu/tools/kudu-tool-test.cc | 49 ++++++++++++++------------
15 files changed, 118 insertions(+), 62 deletions(-)
diff --git a/src/kudu/common/common.proto b/src/kudu/common/common.proto
index a51e806..0e2e77e 100644
--- a/src/kudu/common/common.proto
+++ b/src/kudu/common/common.proto
@@ -70,9 +70,21 @@ enum EncodingType {
BIT_SHUFFLE = 6;
}
+// Enums that specify the HMS-related configurations for a Kudu mini-cluster.
enum HmsMode {
+ // No HMS will be started.
NONE = 0;
+
+ // The HMS will be started, but will not be configured to use the Kudu
+ // plugin.
+ DISABLE_HIVE_METASTORE = 3;
+
+ // The HMS will be started and configured to use the Kudu plugin, but the
+ // Kudu mini-cluster will not be configured to synchronize with it.
ENABLE_HIVE_METASTORE = 1;
+
+ // The HMS will be started and configured to use the Kudu plugin, and the
+ // Kudu mini-cluster will be configured to synchronize with it.
ENABLE_METASTORE_INTEGRATION = 2;
};
@@ -425,4 +437,4 @@ message KeyRangePB {
optional bytes stop_primary_key = 2 [(kudu.REDACT) = true];
// Number of bytes in chunk.
required uint64 size_bytes_estimates = 3;
-}
\ No newline at end of file
+}
diff --git a/src/kudu/hms/hms_catalog.cc b/src/kudu/hms/hms_catalog.cc
index 7c2219a..6f03f5c 100644
--- a/src/kudu/hms/hms_catalog.cc
+++ b/src/kudu/hms/hms_catalog.cc
@@ -110,7 +110,7 @@ HmsCatalog::~HmsCatalog() {
Stop();
}
-Status HmsCatalog::Start() {
+Status HmsCatalog::Start(HmsClientVerifyKuduSyncConfig verify_service_config) {
vector<HostPort> addresses;
RETURN_NOT_OK(ParseUris(FLAGS_hive_metastore_uris, &addresses));
@@ -122,6 +122,7 @@ Status HmsCatalog::Start() {
options.service_principal = FLAGS_hive_metastore_kerberos_principal;
options.max_buf_size = FLAGS_hive_metastore_max_message_size_bytes;
options.retry_count = FLAGS_hive_metastore_retry_count;
+ options.verify_service_config = verify_service_config == VERIFY;
return ha_client_.Start(std::move(addresses), std::move(options));
}
diff --git a/src/kudu/hms/hms_catalog.h b/src/kudu/hms/hms_catalog.h
index f4e92fd..e878f4c 100644
--- a/src/kudu/hms/hms_catalog.h
+++ b/src/kudu/hms/hms_catalog.h
@@ -37,6 +37,11 @@ class Schema;
namespace hms {
+enum HmsClientVerifyKuduSyncConfig {
+ DONT_VERIFY,
+ VERIFY,
+};
+
// A high-level API above the HMS which handles converting to and from
// Kudu-specific types, retries, reconnections, HA, error handling, and
// concurrent requests.
@@ -49,7 +54,7 @@ class HmsCatalog {
~HmsCatalog();
// Starts the HmsCatalog instance.
- Status Start();
+ Status Start(HmsClientVerifyKuduSyncConfig verify_service_config = DONT_VERIFY);
// Stops the HmsCatalog instance.
void Stop();
diff --git a/src/kudu/hms/hms_client-test.cc b/src/kudu/hms/hms_client-test.cc
index ca64c6b..6fbc121 100644
--- a/src/kudu/hms/hms_client-test.cc
+++ b/src/kudu/hms/hms_client-test.cc
@@ -397,6 +397,10 @@ TEST_F(HmsClientTest, TestHmsConnect) {
options.send_timeout = MonoDelta::FromMilliseconds(100);
options.conn_timeout = MonoDelta::FromMilliseconds(100);
+ // This test will attempt to connect and transfer data upon starting the
+ // client.
+ options.verify_service_config = true;
+
auto start_client = [&options] (Sockaddr addr) -> Status {
HmsClient client(HostPort(addr), options);
return client.Start();
diff --git a/src/kudu/hms/hms_client.cc b/src/kudu/hms/hms_client.cc
index 61d87db..13e2237 100644
--- a/src/kudu/hms/hms_client.cc
+++ b/src/kudu/hms/hms_client.cc
@@ -126,7 +126,8 @@ const int kSlowExecutionWarningThresholdMs = 1000;
const char* const HmsClient::kServiceName = "Hive Metastore";
HmsClient::HmsClient(const HostPort& address, const ClientOptions& options)
- : client_(hive::ThriftHiveMetastoreClient(CreateClientProtocol(address, options))) {
+ : verify_kudu_sync_config_(options.verify_service_config),
+ client_(hive::ThriftHiveMetastoreClient(CreateClientProtocol(address, options))) {
}
HmsClient::~HmsClient() {
@@ -138,6 +139,10 @@ Status HmsClient::Start() {
HMS_RET_NOT_OK(client_.getOutputProtocol()->getTransport()->open(),
"failed to open Hive Metastore connection");
+ if (!verify_kudu_sync_config_) {
+ return Status::OK();
+ }
+
// Immediately after connecting to the HMS, check that it is configured with
// the required event listeners.
string event_listener_config;
diff --git a/src/kudu/hms/hms_client.h b/src/kudu/hms/hms_client.h
index 67dcbed..c79f401 100644
--- a/src/kudu/hms/hms_client.h
+++ b/src/kudu/hms/hms_client.h
@@ -190,6 +190,7 @@ class HmsClient {
static Status DeserializeJsonTable(Slice json, hive::Table* table) WARN_UNUSED_RESULT;
private:
+ bool verify_kudu_sync_config_;
hive::ThriftHiveMetastoreClient client_;
};
} // namespace hms
diff --git a/src/kudu/hms/mini_hms.cc b/src/kudu/hms/mini_hms.cc
index 7ccb4a9..1400f60 100644
--- a/src/kudu/hms/mini_hms.cc
+++ b/src/kudu/hms/mini_hms.cc
@@ -83,6 +83,10 @@ void MiniHms::EnableSentry(const HostPort& sentry_address,
sentry_client_rpc_retry_interval_ms_ = sentry_client_rpc_retry_interval_ms;
}
+void MiniHms::EnableKuduPlugin(bool enable) {
+ enable_kudu_plugin_ = enable;
+}
+
void MiniHms::SetDataRoot(string data_root) {
CHECK(!hms_process_);
data_root_ = std::move(data_root);
@@ -208,6 +212,9 @@ bool MiniHms::IsAuthorizationEnabled() const {
Status MiniHms::CreateHiveSite() const {
+ const string listeners = Substitute("org.apache.hive.hcatalog.listener.DbNotificationListener$0",
+ enable_kudu_plugin_ ? ",org.apache.kudu.hive.metastore.KuduMetastorePlugin" : "");
+
// - datanucleus.schema.autoCreateAll
// - hive.metastore.schema.verification
// Allow Hive to startup and run without first running the schemaTool.
@@ -237,8 +244,7 @@ Status MiniHms::CreateHiveSite() const {
<property>
<name>hive.metastore.transactional.event.listeners</name>
<value>
- org.apache.hive.hcatalog.listener.DbNotificationListener,
- org.apache.kudu.hive.metastore.KuduMetastorePlugin
+ $0
</value>
</property>
@@ -254,37 +260,37 @@ Status MiniHms::CreateHiveSite() const {
<property>
<name>hive.metastore.warehouse.dir</name>
- <value>file://$1/warehouse/</value>
+ <value>file://$2/warehouse/</value>
</property>
<property>
<name>javax.jdo.option.ConnectionURL</name>
- <value>jdbc:derby:$1/metadb;create=true</value>
+ <value>jdbc:derby:$2/metadb;create=true</value>
</property>
<property>
<name>hive.metastore.event.db.listener.timetolive</name>
- <value>$0s</value>
+ <value>$1s</value>
</property>
<property>
<name>hive.metastore.sasl.enabled</name>
- <value>$2</value>
+ <value>$3</value>
</property>
<property>
<name>hive.metastore.kerberos.keytab.file</name>
- <value>$3</value>
+ <value>$4</value>
</property>
<property>
<name>hive.metastore.kerberos.principal</name>
- <value>$4</value>
+ <value>$5</value>
</property>
<property>
<name>hadoop.rpc.protection</name>
- <value>$5</value>
+ <value>$6</value>
</property>
<property>
@@ -302,7 +308,7 @@ Status MiniHms::CreateHiveSite() const {
<value>true</value>
</property>
- $6
+ $7
</configuration>
)";
@@ -354,6 +360,7 @@ Status MiniHms::CreateHiveSite() const {
}
string hive_file_contents = Substitute(kHiveFileTemplate,
+ listeners,
notification_log_ttl_.ToSeconds(),
data_root_,
IsKerberosEnabled(),
diff --git a/src/kudu/hms/mini_hms.h b/src/kudu/hms/mini_hms.h
index 7fb1c22..73ac2df 100644
--- a/src/kudu/hms/mini_hms.h
+++ b/src/kudu/hms/mini_hms.h
@@ -59,6 +59,9 @@ class MiniHms {
int sentry_client_rpc_retry_num = 3,
int sentry_client_rpc_retry_interval_ms = 500);
+ // Configures the mini HMS to enable or disable the Kudu plugin.
+ void EnableKuduPlugin(bool enable);
+
// Configures the mini HMS to store its data in the provided path. If not set,
// it uses a test-only temporary directory.
void SetDataRoot(std::string data_root);
@@ -124,6 +127,9 @@ class MiniHms {
std::string sentry_service_principal_;
int sentry_client_rpc_retry_num_;
int sentry_client_rpc_retry_interval_ms_;
+
+ // Whether to enable the Kudu listener plugin.
+ bool enable_kudu_plugin_ = true;
};
} // namespace hms
diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc
index b933ad5..eeb351d 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -280,6 +280,7 @@ using kudu::consensus::RaftConsensus;
using kudu::consensus::RaftPeerPB;
using kudu::consensus::StartTabletCopyRequestPB;
using kudu::consensus::kMinimumTerm;
+using kudu::hms::HmsClientVerifyKuduSyncConfig;
using kudu::pb_util::SecureDebugString;
using kudu::pb_util::SecureShortDebugString;
using kudu::rpc::RpcContext;
@@ -752,7 +753,7 @@ Status CatalogManager::Init(bool is_first_run) {
std::lock_guard<RWMutex> leader_lock_guard(leader_lock_);
hms_catalog_.reset(new hms::HmsCatalog(std::move(master_addresses)));
- RETURN_NOT_OK_PREPEND(hms_catalog_->Start(),
+ RETURN_NOT_OK_PREPEND(hms_catalog_->Start(HmsClientVerifyKuduSyncConfig::VERIFY),
"failed to start Hive Metastore catalog");
hms_notification_log_listener_.reset(new HmsNotificationLogListenerTask(this));
diff --git a/src/kudu/master/master.cc b/src/kudu/master/master.cc
index 5d90381..4fb8b8f 100644
--- a/src/kudu/master/master.cc
+++ b/src/kudu/master/master.cc
@@ -25,7 +25,6 @@
#include <vector>
#include <gflags/gflags.h>
-#include <gflags/gflags_declare.h>
#include <glog/logging.h>
#include "kudu/cfile/block_cache.h"
@@ -56,7 +55,6 @@
#include "kudu/tserver/tablet_copy_service.h"
#include "kudu/tserver/tablet_service.h"
#include "kudu/util/flag_tags.h"
-#include "kudu/util/flag_validators.h"
#include "kudu/util/maintenance_manager.h"
#include "kudu/util/monotime.h"
#include "kudu/util/net/net_util.h"
@@ -95,8 +93,6 @@ DEFINE_string(location_mapping_cmd, "",
"characters from the set [a-zA-Z0-9_-.]. If the cluster is not "
"using location awareness features this flag should not be set.");
-DECLARE_bool(hive_metastore_sasl_enabled);
-DECLARE_string(keytab_file);
using std::min;
using std::shared_ptr;
@@ -113,24 +109,6 @@ using strings::Substitute;
namespace kudu {
namespace master {
-namespace {
-// Validates that if the HMS is configured with SASL enabled, the server has a
-// keytab available. This is located in master.cc because the HMS module (where
-// --hive_metastore_sasl_enabled is defined) doesn't link to the server module
-// (where --keytab_file is defined), and vice-versa. The master module is the
-// first module which links to both.
-bool ValidateHiveMetastoreSaslEnabled() {
- if (FLAGS_hive_metastore_sasl_enabled && FLAGS_keytab_file.empty()) {
- LOG(ERROR) << "When the Hive Metastore has SASL enabled "
- "(--hive_metastore_sasl_enabled), Kudu must be configured with "
- "a keytab (--keytab_file).";
- return false;
- }
- return true;
-}
-GROUP_FLAG_VALIDATOR(hive_metastore_sasl_enabled, ValidateHiveMetastoreSaslEnabled);
-} // anonymous namespace
-
Master::Master(const MasterOptions& opts)
: KuduServer("Master", opts, "kudu.master"),
state_(kStopped),
diff --git a/src/kudu/master/master_main.cc b/src/kudu/master/master_main.cc
index 583a185..d2f784a 100644
--- a/src/kudu/master/master_main.cc
+++ b/src/kudu/master/master_main.cc
@@ -25,6 +25,7 @@
#include "kudu/gutil/strings/substitute.h"
#include "kudu/master/master.h"
#include "kudu/master/master_options.h"
+#include "kudu/util/flag_validators.h"
#include "kudu/util/flags.h"
#include "kudu/util/init.h"
#include "kudu/util/logging.h"
@@ -38,9 +39,33 @@ DECLARE_bool(evict_failed_followers);
DECLARE_int32(webserver_port);
DECLARE_string(rpc_bind_addresses);
+DECLARE_bool(hive_metastore_sasl_enabled);
+DECLARE_string(keytab_file);
+
namespace kudu {
namespace master {
+namespace {
+// Validates that if the HMS is configured with SASL enabled, the server has a
+// keytab available. This is located in master.cc because the HMS module (where
+// -hive_metastore_sasl_enabled is defined) doesn't link to the server module
+// (where --keytab_file is defined), and vice-versa. The master module is the
+// first module which links to both.
+// Note: this check only needs to be run on a server. E.g. tools that run with
+// the HMS don't need to pass in a keytab.
+bool ValidateHiveMetastoreSaslEnabled() {
+ if (FLAGS_hive_metastore_sasl_enabled &&
+ FLAGS_keytab_file.empty()) {
+ LOG(ERROR) << "When the Hive Metastore has SASL enabled "
+ "(--hive_metastore_sasl_enabled), Kudu must be configured with "
+ "a keytab (--keytab_file).";
+ return false;
+ }
+ return true;
+}
+GROUP_FLAG_VALIDATOR(hive_metastore_sasl_enabled, ValidateHiveMetastoreSaslEnabled);
+} // anonymous namespace
+
static int MasterMain(int argc, char** argv) {
InitKuduOrDie();
diff --git a/src/kudu/mini-cluster/external_mini_cluster.cc b/src/kudu/mini-cluster/external_mini_cluster.cc
index b60fcb2..bffb048 100644
--- a/src/kudu/mini-cluster/external_mini_cluster.cc
+++ b/src/kudu/mini-cluster/external_mini_cluster.cc
@@ -252,11 +252,16 @@ Status ExternalMiniCluster::Start() {
}
// Start the HMS.
- if (opts_.hms_mode == HmsMode::ENABLE_HIVE_METASTORE ||
+ if (opts_.hms_mode == HmsMode::DISABLE_HIVE_METASTORE ||
+ opts_.hms_mode == HmsMode::ENABLE_HIVE_METASTORE ||
opts_.hms_mode == HmsMode::ENABLE_METASTORE_INTEGRATION) {
hms_.reset(new hms::MiniHms());
hms_->SetDataRoot(opts_.cluster_root);
+ if (opts_.hms_mode == HmsMode::DISABLE_HIVE_METASTORE) {
+ hms_->EnableKuduPlugin(false);
+ }
+
if (opts_.enable_kerberos) {
string spn = Substitute("hive/$0", hms_->address().host());
string ktpath;
diff --git a/src/kudu/server/server_base.cc b/src/kudu/server/server_base.cc
index 7a57f33..cdc3bc9 100644
--- a/src/kudu/server/server_base.cc
+++ b/src/kudu/server/server_base.cc
@@ -130,7 +130,6 @@ TAG_FLAG(principal, experimental);
// See KUDU-1884.
TAG_FLAG(principal, unsafe);
-
DEFINE_string(keytab_file, "",
"Path to the Kerberos Keytab file for this server. Specifying a "
"keytab file will cause the server to kinit, and enable Kerberos "
diff --git a/src/kudu/thrift/client.h b/src/kudu/thrift/client.h
index d01f213..49ac621 100644
--- a/src/kudu/thrift/client.h
+++ b/src/kudu/thrift/client.h
@@ -76,6 +76,10 @@ struct ClientOptions {
// Number of times an RPC is retried by the HA client after encountering
// retriable failures, such as network failures.
int32_t retry_count = 1;
+
+ // Whether the client should, upon connecting, verify the Thrift service
+ // configuration is correct.
+ bool verify_service_config = false;
};
std::shared_ptr<apache::thrift::protocol::TProtocol> CreateClientProtocol(
diff --git a/src/kudu/tools/kudu-tool-test.cc b/src/kudu/tools/kudu-tool-test.cc
index 65829e9..3fe573e 100644
--- a/src/kudu/tools/kudu-tool-test.cc
+++ b/src/kudu/tools/kudu-tool-test.cc
@@ -3402,7 +3402,7 @@ TEST_P(ToolTestKerberosParameterized, TestHmsDowngrade) {
TEST_P(ToolTestKerberosParameterized, TestCheckAndAutomaticFixHmsMetadata) {
string kUsername = "alice";
ExternalMiniClusterOptions opts;
- opts.hms_mode = HmsMode::ENABLE_HIVE_METASTORE;
+ opts.hms_mode = HmsMode::DISABLE_HIVE_METASTORE;
opts.enable_kerberos = EnableKerberos();
NO_FATALS(StartExternalMiniCluster(std::move(opts)));
@@ -3410,6 +3410,7 @@ TEST_P(ToolTestKerberosParameterized, TestCheckAndAutomaticFixHmsMetadata) {
thrift::ClientOptions hms_opts;
hms_opts.enable_kerberos = EnableKerberos();
hms_opts.service_principal = "hive";
+ hms_opts.verify_service_config = false;
HmsClient hms_client(cluster_->hms()->address(), hms_opts);
ASSERT_OK(hms_client.Start());
ASSERT_TRUE(hms_client.IsConnected());
@@ -3521,11 +3522,6 @@ TEST_P(ToolTestKerberosParameterized, TestCheckAndAutomaticFixHmsMetadata) {
ASSERT_OK(hms_client.CreateDatabase(db));
ASSERT_OK(CreateKuduTable(kudu_client, "my_db.table"));
- // Enable the HMS integration.
- cluster_->ShutdownNodes(cluster::ClusterNodes::MASTERS_ONLY);
- cluster_->EnableMetastoreIntegration();
- ASSERT_OK(cluster_->Restart());
-
unordered_set<string> consistent_tables = {
"default.control",
};
@@ -3554,12 +3550,16 @@ TEST_P(ToolTestKerberosParameterized, TestCheckAndAutomaticFixHmsMetadata) {
consistent_tables.insert(tables.begin(), tables.end());
};
+ const string hms_flags = Substitute("-hive_metastore_uris=$0 -hive_metastore_sasl_enabled=$1",
+ FLAGS_hive_metastore_uris, FLAGS_hive_metastore_sasl_enabled);
+
// Run the HMS check tool and verify that the consistent tables are not
// reported, and the inconsistent tables are reported.
auto check = [&] () {
string out;
string err;
- Status s = RunActionStdoutStderrString(Substitute("hms check $0", master_addr), &out, &err);
+ Status s = RunActionStdoutStderrString(Substitute("hms check $0 $1", master_addr, hms_flags),
+ &out, &err);
SCOPED_TRACE(strings::CUnescapeOrDie(out));
if (inconsistent_tables.empty()) {
ASSERT_OK(s);
@@ -3581,13 +3581,14 @@ TEST_P(ToolTestKerberosParameterized, TestCheckAndAutomaticFixHmsMetadata) {
// 'hms fix --dryrun should not change the output of 'hms check'.
NO_FATALS(RunActionStdoutNone(
- Substitute("hms fix $0 --dryrun --drop_orphan_hms_tables", master_addr)));
+ Substitute("hms fix $0 --dryrun --drop_orphan_hms_tables $1", master_addr, hms_flags)));
NO_FATALS(check());
// Drop orphan hms tables.
NO_FATALS(RunActionStdoutNone(
Substitute("hms fix $0 --drop_orphan_hms_tables --nocreate_missing_hms_tables "
- "--noupgrade_hms_tables --nofix_inconsistent_tables", master_addr)));
+ "--noupgrade_hms_tables --nofix_inconsistent_tables $1",
+ master_addr, hms_flags)));
make_consistent({
"default.orphan_hms_table",
"default.orphan_hms_table_legacy_managed",
@@ -3596,7 +3597,8 @@ TEST_P(ToolTestKerberosParameterized, TestCheckAndAutomaticFixHmsMetadata) {
// Create missing hms tables.
NO_FATALS(RunActionStdoutNone(
- Substitute("hms fix $0 --noupgrade_hms_tables --nofix_inconsistent_tables", master_addr)));
+ Substitute("hms fix $0 --noupgrade_hms_tables --nofix_inconsistent_tables $1",
+ master_addr, hms_flags)));
make_consistent({
"default.kudu_orphan",
"my_db.table",
@@ -3605,7 +3607,7 @@ TEST_P(ToolTestKerberosParameterized, TestCheckAndAutomaticFixHmsMetadata) {
// Upgrade legacy HMS tables.
NO_FATALS(RunActionStdoutNone(
- Substitute("hms fix $0 --nofix_inconsistent_tables", master_addr)));
+ Substitute("hms fix $0 --nofix_inconsistent_tables $1", master_addr, hms_flags)));
make_consistent({
"default.legacy_managed",
"legacy_external",
@@ -3615,7 +3617,7 @@ TEST_P(ToolTestKerberosParameterized, TestCheckAndAutomaticFixHmsMetadata) {
NO_FATALS(check());
// Refresh stale HMS tables.
- NO_FATALS(RunActionStdoutNone(Substitute("hms fix $0", master_addr)));
+ NO_FATALS(RunActionStdoutNone(Substitute("hms fix $0 $1", master_addr, hms_flags)));
make_consistent({
"default.UPPERCASE",
"default.inconsistent_schema",
@@ -3679,7 +3681,7 @@ TEST_P(ToolTestKerberosParameterized, TestCheckAndAutomaticFixHmsMetadata) {
TEST_P(ToolTestKerberosParameterized, TestCheckAndManualFixHmsMetadata) {
string kUsername = "alice";
ExternalMiniClusterOptions opts;
- opts.hms_mode = HmsMode::ENABLE_HIVE_METASTORE;
+ opts.hms_mode = HmsMode::DISABLE_HIVE_METASTORE;
opts.enable_kerberos = EnableKerberos();
NO_FATALS(StartExternalMiniCluster(std::move(opts)));
@@ -3687,6 +3689,7 @@ TEST_P(ToolTestKerberosParameterized, TestCheckAndManualFixHmsMetadata) {
thrift::ClientOptions hms_opts;
hms_opts.enable_kerberos = EnableKerberos();
hms_opts.service_principal = "hive";
+ hms_opts.verify_service_config = false;
HmsClient hms_client(cluster_->hms()->address(), hms_opts);
ASSERT_OK(hms_client.Start());
ASSERT_TRUE(hms_client.IsConnected());
@@ -3727,16 +3730,15 @@ TEST_P(ToolTestKerberosParameterized, TestCheckAndManualFixHmsMetadata) {
ASSERT_OK(CreateKuduTable(kudu_client, "impala::default.conflicting_legacy_table"));
ASSERT_OK(CreateKuduTable(kudu_client, "default.conflicting_legacy_table"));
- // Enable the HMS integration.
- cluster_->ShutdownNodes(cluster::ClusterNodes::MASTERS_ONLY);
- cluster_->EnableMetastoreIntegration();
- ASSERT_OK(cluster_->Restart());
+ const string hms_flags = Substitute("-hive_metastore_uris=$0 -hive_metastore_sasl_enabled=$1",
+ FLAGS_hive_metastore_uris, FLAGS_hive_metastore_sasl_enabled);
// Run the HMS check tool and verify that the inconsistent tables are reported.
auto check = [&] () {
string out;
string err;
- Status s = RunActionStdoutStderrString(Substitute("hms check $0", master_addr), &out, &err);
+ Status s = RunActionStdoutStderrString(Substitute("hms check $0 $1", master_addr, hms_flags),
+ &out, &err);
SCOPED_TRACE(strings::CUnescapeOrDie(out));
for (const string& table : vector<string>({
"duplicate_hms_tables",
@@ -3757,7 +3759,8 @@ TEST_P(ToolTestKerberosParameterized, TestCheckAndManualFixHmsMetadata) {
{
string out;
string err;
- Status s = RunActionStdoutStderrString(Substitute("hms fix $0", master_addr), &out, &err);
+ Status s = RunActionStdoutStderrString(Substitute("hms fix $0 $1", master_addr, hms_flags),
+ &out, &err);
SCOPED_TRACE(strings::CUnescapeOrDie(out));
ASSERT_FALSE(s.ok());
}
@@ -3783,14 +3786,14 @@ TEST_P(ToolTestKerberosParameterized, TestCheckAndManualFixHmsMetadata) {
// Rename the conflicting table.
NO_FATALS(RunActionStdoutNone(Substitute(
- "table rename-table --nomodify-external-catalogs $0 "
- "default.conflicting_legacy_table default.non_conflicting_legacy_table", master_addr)));
+ "table rename-table --nomodify-external-catalogs $0 "
+ "default.conflicting_legacy_table default.non_conflicting_legacy_table", master_addr)));
// Run the automatic fixer to create missing HMS table entries.
- NO_FATALS(RunActionStdoutNone(Substitute("hms fix $0", master_addr)));
+ NO_FATALS(RunActionStdoutNone(Substitute("hms fix $0 $1", master_addr, hms_flags)));
// Check should now be clean.
- NO_FATALS(RunActionStdoutNone(Substitute("hms check $0", master_addr)));
+ NO_FATALS(RunActionStdoutNone(Substitute("hms check $0 $1", master_addr, hms_flags)));
// Ensure the tables are available.
vector<string> kudu_tables;
[kudu] 02/03: sentry: don't send requests for DATABASE/SERVER
privileges
Posted by gr...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
granthenke pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 60cc7f3c83e52bc58b9a31d222a2f86e3ccc4059
Author: Andrew Wong <aw...@apache.org>
AuthorDate: Wed May 29 13:42:42 2019 -0700
sentry: don't send requests for DATABASE/SERVER privileges
The API used to request privileges from Sentry will return more
privileges than required for a given request. For example, if requesting
privileges for a specific database, the API will return privileges for
all ancestors and all descendents of that database, i.e. privileges for
the server, the database, all tables in the database, and all columns
therein.
To remediate this, this patch narrows the scope of requests sent to
Sentry to the table-level. Table-level seems fitting since every request
for privileges Kudu makes to Sentry thus far is naturally scoped to a
table anyway.
Note: this patch doesn't touch any of the caching logic; it only tweaks
the request sent to Sentry.
I wrote a small benchmark[1] to gauge the effects of this patch. The
time spent checking database-level privileges with 20K privilege entries
in the database was 1.989 without this patch, and 0.491s with this
patch.
W0602 07:44:03.110574 13909 sentry_client.cc:188] Time spent alter Sentry role grant privileges: real 3.861s user 0.000s sys 0.000s
I0602 07:44:03.110697 13909 sentry_authz_provider-test.cc:739] num tables granted: 19998
W0602 07:44:06.829799 13909 sentry_client.cc:188] Time spent alter Sentry role grant privileges: real 3.719s user 0.000s sys 0.000s
I0602 07:44:06.829936 13909 sentry_authz_provider-test.cc:739] num tables granted: 19999
W0602 07:44:10.521934 13909 sentry_client.cc:188] Time spent alter Sentry role grant privileges: real 3.692s user 0.000s sys 0.000s
W0602 07:44:12.375082 14301 sasl_client_transport.cc:160] Received large Thrift SASL frame: 2.33M
W0602 07:44:12.438380 14301 sentry_client.cc:170] Time spent list Sentry privileges by user: real 1.892s user 0.054s sys 0.009s
W0602 07:44:12.509138 13909 sentry_authz_provider.cc:275] Action <CREATE> on table <dbwithalongname.table> with authorizable scope <DATABASE> is not permitted for user <test-user>
I0602 07:44:12.510848 13909 sentry_authz_provider-test.cc:744] Time spent Getting database privileges: real 1.989s user 0.070s sys 0.002s
W0602 07:44:13.002019 13909 sentry_authz_provider.cc:275] Action <CREATE> on table <dbwithalongname.table> with authorizable scope <DATABASE> is not permitted for user <test-user>
I0602 07:44:13.002111 13909 sentry_authz_provider-test.cc:750] Time spent Getting database privileges: real 0.491s user 0.000s sys 0.000s
While the test numbers themselves may not be representative of a real
Sentry deployment (since the test uses a Sentry minicluster instead of a
real cluster) the numbers still point to reasonable improvement.
A version of this benchmark is included in this patch. I opted to not
gate the behavior behind a gflag, so rather than running successive
privileges with different behavior, the included test only checks
privileges once.
[1] https://gist.github.com/andrwng/88f3a760441b4b37b3ff011e656683ab
Change-Id: Ic0025e3bacc8449dfffe99a1fc062a9e6787eb78
Reviewed-on: http://gerrit.cloudera.org:8080/13494
Tested-by: Kudu Jenkins
Reviewed-by: Hao Hao <ha...@cloudera.com>
---
src/kudu/master/sentry_authz_provider-test.cc | 33 ++++++++++++++++++
src/kudu/master/sentry_privileges_fetcher.cc | 50 ++++++++++++++++++---------
src/kudu/master/sentry_privileges_fetcher.h | 2 +-
3 files changed, 68 insertions(+), 17 deletions(-)
diff --git a/src/kudu/master/sentry_authz_provider-test.cc b/src/kudu/master/sentry_authz_provider-test.cc
index e7304ab..1b37927 100644
--- a/src/kudu/master/sentry_authz_provider-test.cc
+++ b/src/kudu/master/sentry_authz_provider-test.cc
@@ -27,6 +27,7 @@
#include <unordered_set>
#include <vector>
+#include <gflags/gflags.h>
#include <gflags/gflags_declare.h>
#include <glog/logging.h>
#include <google/protobuf/stubs/port.h>
@@ -52,17 +53,24 @@
#include "kudu/sentry/sentry_client.h"
#include "kudu/sentry/sentry_policy_service_types.h"
#include "kudu/util/barrier.h"
+#include "kudu/util/flag_tags.h"
#include "kudu/util/hdr_histogram.h"
+#include "kudu/util/logging.h"
#include "kudu/util/metrics.h"
#include "kudu/util/net/net_util.h"
#include "kudu/util/pb_util.h"
#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"
+DEFINE_int32(num_table_privileges, 100,
+ "Number of table privileges to use in testing");
+TAG_FLAG(num_table_privileges, hidden);
+
DECLARE_int32(sentry_service_recv_timeout_seconds);
DECLARE_int32(sentry_service_send_timeout_seconds);
DECLARE_uint32(sentry_privileges_cache_capacity_mb);
@@ -339,6 +347,31 @@ constexpr const char* kColumn = "column";
} // anonymous namespace
+// 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) {
+ const char* kLongDb = "DbWithLongName";
+ const char* kLongTable = "TableWithLongName";
+ ASSERT_OK(CreateRoleAndAddToGroups());
+
+ // Create a database with a bunch tables in it.
+ int kNumTables = FLAGS_num_table_privileges;
+ for (int i = 0; i < kNumTables; i++) {
+ KLOG_EVERY_N_SECS(INFO, 3) << Substitute("num tables granted: $0", i);
+ ASSERT_OK(AlterRoleGrantPrivilege(
+ GetTablePrivilege(kLongDb, Substitute("$0_$1", kLongTable, i), "OWNER")));
+ }
+
+ // Time how long it takes to get the database privileges via authorizing a
+ // create table request.
+ Status s;
+ LOG_TIMING(INFO, "Getting database privileges") {
+ s = sentry_authz_provider_->AuthorizeCreateTable(
+ Substitute("$0.$1_$2", kLongDb, kLongTable, 0) , kTestUser, kTestUser);
+ }
+ ASSERT_TRUE(s.IsNotAuthorized());
+}
+
class SentryAuthzProviderFilterPrivilegesTest : public SentryAuthzProviderTest {
public:
SentryAuthzProviderFilterPrivilegesTest()
diff --git a/src/kudu/master/sentry_privileges_fetcher.cc b/src/kudu/master/sentry_privileges_fetcher.cc
index 5d54999..ff04a1f 100644
--- a/src/kudu/master/sentry_privileges_fetcher.cc
+++ b/src/kudu/master/sentry_privileges_fetcher.cc
@@ -208,28 +208,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 +594,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 +616,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 +690,7 @@ Status SentryPrivilegesFetcher::GetSentryPrivileges(
return Status::OK();
}
+ 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);