kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From danburk...@apache.org
Subject kudu git commit: KUDU-2191: support table-name identifiers with upper case chars
Date Tue, 03 Jul 2018 23:23:47 GMT
Repository: kudu
Updated Branches:
  refs/heads/master 1ae050e4d -> 7b048b8db


KUDU-2191: support table-name identifiers with upper case chars

Summary: When the HMS integration is enabled, Kudu now preserves table
name casing, but uses case-insensitive lookups to retrieve tables.

Background: The HMS lowercases all database (table) identifiers during database
(table) creation, only storing the lowercased version. On database and
table lookup the HMS automatically does a case-insensitive compare.
During table creation Kudu checks that table names are valid UTF-8, and
does no transformations on identiers. During table lookups Kudu requires
that the table name match exactly, including case.

As a result of these behavior differences and the design of the
notification log listener, tables with upper-case characters can not be
altered or deleted when the HMS integration is enabled. This commit
fixes this by changing how the Catalog Manager handles identifiers when
the HMS integration is enabled:

* During table creation, the Catalog Manager preserves the case of table
  names.
* On table lookup, the Catalog Manager does a case-insensitive
  comparison to find the table.

This is implemented by storing the preserved case in the table's
sys-catalog metadata entry, and storing a 'normalized' (down-cased)
identifier in the ephemeral by-name table map. The various parts of the
catalog manager which deal with the by-name map are converted to use the
normalized version of the name. When the HMS integration is not
configured, normalized table names are equal to the original table name,
so the behavior changes that this patch introduces are entirely opt-in.

There is one edge case that complicates turning on the HMS integration
in rare circumstances: if there are existing (legacy) tables with names
which map to the same normalized form (e.g. differ only in case), the
catalog manager will fail to startup and instruct the operator to rename
the offending tables before trying again. Additionally, this check only
applies to tables that otherwise follow the Hive table naming rules
(matching regex '[\w_/]+\.[\w_/]+').

Change-Id: I18977d6fe7b2999a36681a728ac0d1e54b7f38cd
Reviewed-on: http://gerrit.cloudera.org:8080/10817
Reviewed-by: Adar Dembo <adar@cloudera.com>
Tested-by: Kudu Jenkins


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/7b048b8d
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/7b048b8d
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/7b048b8d

Branch: refs/heads/master
Commit: 7b048b8dbe2562e68dd2b2b7f1a4db83b1ae10bf
Parents: 1ae050e
Author: Dan Burkert <danburkert@apache.org>
Authored: Fri Jun 22 14:19:38 2018 -0700
Committer: Dan Burkert <danburkert@apache.org>
Committed: Tue Jul 3 23:23:32 2018 +0000

----------------------------------------------------------------------
 src/kudu/hms/hms_catalog-test.cc                |  62 +++++---
 src/kudu/hms/hms_catalog.cc                     |  97 +++++++++----
 src/kudu/hms/hms_catalog.h                      |  26 +++-
 src/kudu/hms/hms_client-test.cc                 |  28 ++++
 .../integration-tests/master-stress-test.cc     |   2 +-
 src/kudu/integration-tests/master_hms-itest.cc  | 126 +++++++++++++++--
 src/kudu/master/catalog_manager.cc              | 141 +++++++++++++------
 src/kudu/master/catalog_manager.h               |  34 +++--
 src/kudu/mini-cluster/external_mini_cluster.cc  |  15 ++
 src/kudu/mini-cluster/external_mini_cluster.h   |   8 +-
 10 files changed, 422 insertions(+), 117 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/7b048b8d/src/kudu/hms/hms_catalog-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/hms/hms_catalog-test.cc b/src/kudu/hms/hms_catalog-test.cc
index 12066f6..472f3a8 100644
--- a/src/kudu/hms/hms_catalog-test.cc
+++ b/src/kudu/hms/hms_catalog-test.cc
@@ -37,6 +37,7 @@
 #include "kudu/rpc/sasl_common.h"
 #include "kudu/security/test/mini_kdc.h"
 #include "kudu/util/net/net_util.h"
+#include "kudu/util/slice.h"
 #include "kudu/util/status.h"
 #include "kudu/util/test_macros.h"
 #include "kudu/util/test_util.h"
@@ -55,35 +56,64 @@ namespace kudu {
 namespace hms {
 
 TEST(HmsCatalogStaticTest, TestParseTableName) {
-  string db;
-  string tbl;
+  Slice db;
+  Slice tbl;
+  string table;
 
-  EXPECT_OK(HmsCatalog::ParseTableName("foo.bar", &db, &tbl));
+  table = "foo.bar";
+  ASSERT_OK(HmsCatalog::ParseTableName(table, &db, &tbl));
   EXPECT_EQ("foo", db);
   EXPECT_EQ("bar", tbl);
 
-  EXPECT_OK(HmsCatalog::ParseTableName("99bottles.my_awesome/table/22", &db, &tbl));
+  table = "99bottles.my_awesome/table/22";
+  ASSERT_OK(HmsCatalog::ParseTableName(table, &db, &tbl));
   EXPECT_EQ("99bottles", db);
   EXPECT_EQ("my_awesome/table/22", tbl);
 
-  EXPECT_OK(HmsCatalog::ParseTableName("_leading_underscore.trailing_underscore_", &db, &tbl));
+  table = "_leading_underscore.trailing_underscore_";
+  ASSERT_OK(HmsCatalog::ParseTableName(table, &db, &tbl));
   EXPECT_EQ("_leading_underscore", db);
   EXPECT_EQ("trailing_underscore_", tbl);
 
-  EXPECT_OK(HmsCatalog::ParseTableName("unicode ☃ tables?.maybe/one_day", &db, &tbl));
-  EXPECT_EQ("unicode ☃ tables?", db);
-  EXPECT_EQ("maybe/one_day", tbl);
+  EXPECT_TRUE(HmsCatalog::ParseTableName(".", &db, &tbl).IsInvalidArgument());
+  EXPECT_TRUE(HmsCatalog::ParseTableName("no-table", &db, &tbl).IsInvalidArgument());
+  EXPECT_TRUE(HmsCatalog::ParseTableName("lots.of.tables", &db, &tbl).IsInvalidArgument());
+  EXPECT_TRUE(HmsCatalog::ParseTableName("no-table", &db, &tbl).IsInvalidArgument());
+  EXPECT_TRUE(HmsCatalog::ParseTableName("lots.of.tables", &db, &tbl).IsInvalidArgument());
+  EXPECT_TRUE(HmsCatalog::ParseTableName(".no_table", &db, &tbl).IsInvalidArgument());
+  EXPECT_TRUE(HmsCatalog::ParseTableName(".no_database", &db, &tbl).IsInvalidArgument());
+  EXPECT_TRUE(HmsCatalog::ParseTableName("punctuation?.no", &db, &tbl).IsInvalidArgument());
+  EXPECT_TRUE(HmsCatalog::ParseTableName("white space.no", &db, &tbl).IsInvalidArgument());
+  EXPECT_TRUE(HmsCatalog::ParseTableName("unicode☃tables.no", &db, &tbl).IsInvalidArgument());
+  EXPECT_TRUE(HmsCatalog::ParseTableName(string("\0.\0", 3), &db, &tbl).IsInvalidArgument());
+}
 
-  EXPECT_OK(HmsCatalog::ParseTableName(".", &db, &tbl));
-  EXPECT_EQ("", db);
-  EXPECT_EQ("", tbl);
+TEST(HmsCatalogStaticTest, TestNormalizeTableName) {
+  string table = "foo.bar";
+  ASSERT_OK(HmsCatalog::NormalizeTableName(&table));
+  ASSERT_EQ("foo.bar", table);
 
-  EXPECT_OK(HmsCatalog::ParseTableName(string("\0.\0", 3), &db, &tbl));
-  EXPECT_EQ(string("\0", 1), db);
-  EXPECT_EQ(string("\0", 1), tbl);
+  table = "fOo.BaR";
+  ASSERT_OK(HmsCatalog::NormalizeTableName(&table));
+  EXPECT_EQ("foo.bar", table);
 
-  EXPECT_TRUE(HmsCatalog::ParseTableName("no-table", &db, &tbl).IsInvalidArgument());
-  EXPECT_TRUE(HmsCatalog::ParseTableName("lots.of.tables", &db, &tbl).IsInvalidArgument());
+  table = "A.B";
+  ASSERT_OK(HmsCatalog::NormalizeTableName(&table));
+  EXPECT_EQ("a.b", table);
+
+  table = "__/A__.buzz";
+  ASSERT_OK(HmsCatalog::NormalizeTableName(&table));
+  EXPECT_EQ("__/a__.buzz", table);
+
+  table = "THE/QUICK/BROWN/FOX/JUMPS/OVER/THE/LAZY/DOG."
+          "the_quick_brown_fox_jumps_over_the_lazy_dog";
+  ASSERT_OK(HmsCatalog::NormalizeTableName(&table));
+  EXPECT_EQ("the/quick/brown/fox/jumps/over/the/lazy/dog."
+            "the_quick_brown_fox_jumps_over_the_lazy_dog", table);
+
+  table = "default.MyTable";
+  ASSERT_OK(HmsCatalog::NormalizeTableName(&table));
+  EXPECT_EQ("default.mytable", table);
 }
 
 TEST(HmsCatalogStaticTest, TestParseUris) {

http://git-wip-us.apache.org/repos/asf/kudu/blob/7b048b8d/src/kudu/hms/hms_catalog.cc
----------------------------------------------------------------------
diff --git a/src/kudu/hms/hms_catalog.cc b/src/kudu/hms/hms_catalog.cc
index 0bc44bb..fe904cd 100644
--- a/src/kudu/hms/hms_catalog.cc
+++ b/src/kudu/hms/hms_catalog.cc
@@ -26,6 +26,7 @@
 #include <utility>
 #include <vector>
 
+#include <boost/optional/optional.hpp>
 #include <gflags/gflags.h>
 #include <glog/logging.h>
 
@@ -33,6 +34,8 @@
 #include "kudu/common/schema.h"
 #include "kudu/common/types.h"
 #include "kudu/gutil/macros.h"
+#include "kudu/gutil/strings/ascii_ctype.h"
+#include "kudu/gutil/strings/charset.h"
 #include "kudu/gutil/strings/split.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/hms/hive_metastore_types.h"
@@ -43,6 +46,7 @@
 #include "kudu/util/slice.h"
 #include "kudu/util/threadpool.h"
 
+using boost::optional;
 using std::move;
 using std::string;
 using std::vector;
@@ -97,8 +101,8 @@ namespace kudu {
 namespace hms {
 
 const char* const HmsCatalog::kInvalidTableError = "when the Hive Metastore integration "
-    "is enabled, Kudu table names must be a period "
-    "('.') separated database and table name pair";
+    "is enabled, Kudu table names must be a period ('.') separated database and table name "
+    "identifier pair, each containing only ASCII alphanumeric characters, '_', and '/'";
 
 HmsCatalog::HmsCatalog(string master_addresses)
     : master_addresses_(std::move(master_addresses)),
@@ -147,15 +151,15 @@ Status HmsCatalog::CreateTable(const string& id,
 }
 
 Status HmsCatalog::DropTable(const string& id, const string& name) {
-  string hms_database;
-  string hms_table;
+  Slice hms_database;
+  Slice hms_table;
   RETURN_NOT_OK(ParseTableName(name, &hms_database, &hms_table));
 
   hive::EnvironmentContext env_ctx = EnvironmentContext();
   env_ctx.properties.insert(make_pair(HmsClient::kKuduTableIdKey, id));
 
   return Execute([&] (HmsClient* client) {
-    return client->DropTable(hms_database, hms_table, env_ctx);
+    return client->DropTable(hms_database.ToString(), hms_table.ToString(), env_ctx);
   });
 }
 
@@ -179,11 +183,12 @@ Status HmsCatalog::UpgradeLegacyImpalaTable(const string& id,
 
 Status HmsCatalog::DowngradeToLegacyImpalaTable(const std::string& name) {
   return Execute([&] (HmsClient* client) {
-    string hms_database;
-    string hms_table;
+    Slice hms_database;
+    Slice hms_table;
     RETURN_NOT_OK(ParseTableName(name, &hms_database, &hms_table));
+
     hive::Table table;
-    RETURN_NOT_OK(client->GetTable(hms_database, hms_table, &table));
+    RETURN_NOT_OK(client->GetTable(hms_database.ToString(), hms_table.ToString(), &table));
     if (table.parameters[HmsClient::kStorageHandlerKey] !=
         HmsClient::kKuduStorageHandler) {
       return Status::IllegalState("non-Kudu table cannot be downgraded");
@@ -200,7 +205,7 @@ Status HmsCatalog::DowngradeToLegacyImpalaTable(const std::string& name) {
     // Remove the Kudu-specific field 'kudu.table_id'.
     EraseKeyReturnValuePtr(&table.parameters, HmsClient::kKuduTableIdKey);
 
-    return client->AlterTable(hms_database, hms_table, table, EnvironmentContext());
+    return client->AlterTable(table.dbName, table.tableName, table, EnvironmentContext());
   });
 }
 
@@ -243,12 +248,12 @@ Status HmsCatalog::AlterTable(const string& id,
       // - The original table does not exist in the HMS
       // - The original table doesn't match the Kudu table being altered
 
-      string hms_database;
-      string hms_table;
+      Slice hms_database;
+      Slice hms_table;
       RETURN_NOT_OK(ParseTableName(name, &hms_database, &hms_table));
 
       hive::Table table;
-      RETURN_NOT_OK(client->GetTable(hms_database, hms_table, &table));
+      RETURN_NOT_OK(client->GetTable(hms_database.ToString(), hms_table.ToString(), &table));
 
       // Check that the HMS entry belongs to the table being altered.
       if (table.parameters[HmsClient::kStorageHandlerKey] != HmsClient::kKuduStorageHandler ||
@@ -260,7 +265,8 @@ Status HmsCatalog::AlterTable(const string& id,
 
       // Overwrite fields in the table that have changed, including the new name.
       RETURN_NOT_OK(PopulateTable(id, new_name, schema, master_addresses_, &table));
-      return client->AlterTable(hms_database, hms_table, table, EnvironmentContext());
+      return client->AlterTable(hms_database.ToString(), hms_table.ToString(),
+                                table, EnvironmentContext());
   });
 }
 
@@ -447,6 +453,12 @@ hive::FieldSchema column_to_field(const ColumnSchema& column) {
   return field;
 }
 
+// Convert an ASCII encoded string to lowercase in place.
+void ToLowerCase(Slice s) {
+  for (int i = 0; i < s.size(); i++) {
+    s.mutable_data()[i] = ascii_tolower(s[i]);
+  }
+}
 } // anonymous namespace
 
 Status HmsCatalog::PopulateTable(const string& id,
@@ -454,7 +466,11 @@ Status HmsCatalog::PopulateTable(const string& id,
                                  const Schema& schema,
                                  const string& master_addresses,
                                  hive::Table* table) {
-  RETURN_NOT_OK(ParseTableName(name, &table->dbName, &table->tableName));
+  Slice hms_database_name;
+  Slice hms_table_name;
+  RETURN_NOT_OK(ParseTableName(name, &hms_database_name, &hms_table_name));
+  table->dbName = hms_database_name.ToString();
+  table->tableName = hms_table_name.ToString();
 
   // Add the Kudu-specific parameters. This intentionally avoids overwriting
   // other parameters.
@@ -483,24 +499,45 @@ Status HmsCatalog::PopulateTable(const string& id,
   return Status::OK();
 }
 
-Status HmsCatalog::ParseTableName(const string& table,
-                                  string* hms_database,
-                                  string* hms_table) {
-  // We do minimal parsing or validating of the identifiers, since Hive has
-  // different validation rules based on configuration (and probably version).
-  // The only rule we enforce is that there be exactly one period to separate
-  // the database and table names, we leave checking of everything else to the
-  // HMS.
-  //
-  // See org.apache.hadoop.hive.metastore.MetaStoreUtils.validateName.
-
-  vector<string> identifiers = strings::Split(table, ".");
-  if (identifiers.size() != 2) {
-    return Status::InvalidArgument(kInvalidTableError, table);
+Status HmsCatalog::NormalizeTableName(string* table_name) {
+  CHECK_NOTNULL(table_name);
+  Slice hms_database;
+  Slice hms_table;
+  RETURN_NOT_OK(ParseTableName(*table_name, &hms_database, &hms_table));
+
+  ToLowerCase(hms_database);
+  ToLowerCase(hms_table);
+
+  return Status::OK();
+}
+
+Status HmsCatalog::ParseTableName(const string& table_name,
+                                  Slice* hms_database,
+                                  Slice* hms_table) {
+  const char kSeparator = '.';
+  strings::CharSet charset("abcdefghijklmnopqrstuvwxyz"
+                           "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+                           "0123456789"
+                           "_/");
+
+  optional<int> separator_idx;
+  for (int idx = 0; idx < table_name.size(); idx++) {
+    char c = table_name[idx];
+    if (!charset.Test(c)) {
+      if (c == kSeparator && !separator_idx) {
+        separator_idx = idx;
+      } else {
+        return Status::InvalidArgument(kInvalidTableError, table_name);
+      }
+    }
+  }
+  if (!separator_idx || *separator_idx == 0 || *separator_idx == table_name.size() - 1) {
+    return Status::InvalidArgument(kInvalidTableError, table_name);
   }
 
-  *hms_database = std::move(identifiers[0]);
-  *hms_table = std::move(identifiers[1]);
+  *hms_database = Slice(table_name.data(), *separator_idx);
+  *hms_table = Slice(table_name.data() + *separator_idx + 1,
+                     table_name.size() - *separator_idx - 1);
   return Status::OK();
 }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/7b048b8d/src/kudu/hms/hms_catalog.h
----------------------------------------------------------------------
diff --git a/src/kudu/hms/hms_catalog.h b/src/kudu/hms/hms_catalog.h
index b9f61f1..d40fb3c 100644
--- a/src/kudu/hms/hms_catalog.h
+++ b/src/kudu/hms/hms_catalog.h
@@ -30,6 +30,8 @@
 #include "kudu/util/net/net_util.h"
 #include "kudu/util/status.h"
 
+class StringPiece;
+
 namespace hive {
 class NotificationEvent;
 class Table;
@@ -129,9 +131,26 @@ class HmsCatalog {
                               const std::string& master_addresses,
                               hive::Table* table) WARN_UNUSED_RESULT;
 
+  // Validates and canonicalizes the provided table name according to HMS rules.
+  // If the table name is not valid it will not be modified. If the table name
+  // is valid, it will be canonicalized.
+  //
+  // Valid Kudu/HMS table names consist of a period ('.') separated database and
+  // table name pair. The database and table names must contain only the ASCII
+  // alphanumeric, '_', and '/' characters.
+  //
+  // Normalized Kudu/HMS table names are downcased so that they contain no
+  // upper-case (A-Z) ASCII characters.
+  //
+  // Hive handles validating and canonicalizing table names in
+  // org.apache.hadoop.hive.metastore.MetaStoreUtils.validateName and
+  // org.apache.hadoop.hive.common.util.normalizeIdentifier.
+  static Status NormalizeTableName(std::string* table_name);
+
  private:
 
   FRIEND_TEST(HmsCatalogStaticTest, TestParseTableName);
+  FRIEND_TEST(HmsCatalogStaticTest, TestParseTableNameSlices);
   FRIEND_TEST(HmsCatalogStaticTest, TestParseUris);
 
   // Synchronously executes a task with exclusive access to the HMS client.
@@ -148,9 +167,10 @@ class HmsCatalog {
 
   // Parses a Kudu table name into a Hive database and table name.
   // Returns an error if the Kudu table name is not correctly formatted.
-  static Status ParseTableName(const std::string& table,
-                               std::string* hms_database,
-                               std::string* hms_table) WARN_UNUSED_RESULT;
+  // The returned HMS database and table slices must not outlive 'table_name'.
+  static Status ParseTableName(const std::string& table_name,
+                               Slice* hms_database,
+                               Slice* hms_table) WARN_UNUSED_RESULT;
 
   // Parses a Hive Metastore URI string into a sequence of HostPorts.
   static Status ParseUris(const std::string& metastore_uris, std::vector<HostPort>* hostports);

http://git-wip-us.apache.org/repos/asf/kudu/blob/7b048b8d/src/kudu/hms/hms_client-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/hms/hms_client-test.cc b/src/kudu/hms/hms_client-test.cc
index 8e1dcd7..2cb6a4c 100644
--- a/src/kudu/hms/hms_client-test.cc
+++ b/src/kudu/hms/hms_client-test.cc
@@ -403,5 +403,33 @@ TEST_F(HmsClientTest, TestDeserializeJsonTable) {
   ASSERT_EQ("database_name", table.dbName);
 }
 
+TEST_F(HmsClientTest, TestCaseSensitivity) {
+  MiniKdc kdc;
+  MiniHms hms;
+  ASSERT_OK(hms.Start());
+
+  HmsClient client(hms.address(), HmsClientOptions());
+  ASSERT_OK(client.Start());
+
+  // Create a database.
+  hive::Database db;
+  db.name = "my_db";
+  ASSERT_OK(client.CreateDatabase(db));
+
+  // Create a table.
+  ASSERT_OK(CreateTable(&client, "my_db", "Foo", "abc123"));
+
+  hive::Table table;
+  ASSERT_OK(client.GetTable("my_db", "Foo", &table));
+  ASSERT_EQ(table.tableName, "foo");
+
+  ASSERT_OK(client.GetTable("my_db", "foo", &table));
+  ASSERT_EQ(table.tableName, "foo");
+
+  ASSERT_OK(client.GetTable("MY_DB", "FOO", &table));
+  ASSERT_EQ(table.dbName, "my_db");
+  ASSERT_EQ(table.tableName, "foo");
+}
+
 } // namespace hms
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/7b048b8d/src/kudu/integration-tests/master-stress-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/master-stress-test.cc b/src/kudu/integration-tests/master-stress-test.cc
index 53040d6..38aa71f 100644
--- a/src/kudu/integration-tests/master-stress-test.cc
+++ b/src/kudu/integration-tests/master-stress-test.cc
@@ -396,7 +396,7 @@ class MasterStressTest : public KuduTest,
 
  private:
   string GenerateTableName() {
-    return Substitute("default.table_$0", oid_generator_.Next());
+    return Substitute("default.Table_$0", oid_generator_.Next());
   }
 
   bool BlockingGetTableName(string* chosen_table) {

http://git-wip-us.apache.org/repos/asf/kudu/blob/7b048b8d/src/kudu/integration-tests/master_hms-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/master_hms-itest.cc b/src/kudu/integration-tests/master_hms-itest.cc
index 8a7c4a1..4b639f5 100644
--- a/src/kudu/integration-tests/master_hms-itest.cc
+++ b/src/kudu/integration-tests/master_hms-itest.cc
@@ -32,10 +32,12 @@
 #include "kudu/common/common.pb.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/hms/hive_metastore_types.h"
+#include "kudu/hms/hms_catalog.h"
 #include "kudu/hms/hms_client.h"
 #include "kudu/hms/mini_hms.h"
 #include "kudu/integration-tests/external_mini_cluster-itest-base.h"
 #include "kudu/mini-cluster/external_mini_cluster.h"
+#include "kudu/mini-cluster/mini_cluster.h"
 #include "kudu/util/decimal_util.h"
 #include "kudu/util/net/net_util.h"
 #include "kudu/util/status.h"
@@ -63,11 +65,15 @@ using strings::Substitute;
 class MasterHmsTest : public ExternalMiniClusterITestBase {
  public:
 
+  virtual HmsMode GetHmsMode() {
+    return HmsMode::ENABLE_METASTORE_INTEGRATION;
+  }
+
   void SetUp() override {
     ExternalMiniClusterITestBase::SetUp();
 
     ExternalMiniClusterOptions opts;
-    opts.hms_mode = HmsMode::ENABLE_METASTORE_INTEGRATION;
+    opts.hms_mode = GetHmsMode();
     opts.num_masters = 1;
     opts.num_tablet_servers = 1;
     // Tune down the notification log poll period in order to speed up catalog convergence.
@@ -234,7 +240,7 @@ TEST_F(MasterHmsTest, TestCreateTable) {
   // Attempt to create a Kudu table to an invalid table name.
   s = CreateKuduTable(hms_database_name, "☃");
   ASSERT_TRUE(s.IsInvalidArgument()) << s.ToString();
-  ASSERT_STR_CONTAINS(s.ToString(), "☃ is not a valid object name");
+  ASSERT_STR_CONTAINS(s.ToString(), "create_db.☃");
 
   // Drop the HMS entry and create the table through Kudu.
   ASSERT_OK(hms_client_->DropTable(hms_database_name, hms_table_name));
@@ -284,8 +290,7 @@ TEST_F(MasterHmsTest, TestRenameTable) {
   table_alterer.reset(client_->NewTableAlterer("db.a"));
   s = table_alterer->RenameTo("foo")->Alter();
   ASSERT_TRUE(s.IsInvalidArgument()) << s.ToString();
-  ASSERT_STR_CONTAINS(s.ToString(), "Kudu table names must be a period ('.') separated "
-                                    "database and table name pair");
+  ASSERT_STR_CONTAINS(s.ToString(), hms::HmsCatalog::kInvalidTableError);
 
   // Attempt to rename the Kudu table to a non-existent database.
   table_alterer.reset(client_->NewTableAlterer("db.a"));
@@ -296,8 +301,8 @@ TEST_F(MasterHmsTest, TestRenameTable) {
   // Attempt to rename the Kudu table to an invalid table name.
   table_alterer.reset(client_->NewTableAlterer("db.a"));
   s = table_alterer->RenameTo("db.☃")->Alter();
-  ASSERT_TRUE(s.IsIllegalState()) << s.ToString();
-  ASSERT_STR_CONTAINS(s.ToString(), "☃ is not a valid object name");
+  ASSERT_TRUE(s.IsInvalidArgument()) << s.ToString();
+  ASSERT_STR_CONTAINS(s.ToString(), "db.☃");
 
   // Shutdown the HMS and try to rename the table.
   ASSERT_OK(StopHms());
@@ -467,13 +472,118 @@ TEST_F(MasterHmsTest, TestNotificationLogListener) {
   ASSERT_OK(hms_client_->DropTable("default", "a"));
   Status s = client_->DeleteTable("default.a");
   ASSERT_TRUE(s.IsNotFound()) << s.ToString();
-  CheckTableDoesNotExist("default", "a");
+  NO_FATALS(CheckTableDoesNotExist("default", "a"));
 
   // Scenario 2: drop from Kudu first.
   ASSERT_OK(CreateKuduTable("default", "a"));
   ASSERT_OK(client_->DeleteTable("default.a"));
   s = hms_client_->DropTable("default", "a");
   ASSERT_TRUE(s.IsNotFound()) << s.ToString();
-  CheckTableDoesNotExist("default", "a");
+  NO_FATALS(CheckTableDoesNotExist("default", "a"));
+}
+
+TEST_F(MasterHmsTest, TestUppercaseIdentifiers) {
+  ASSERT_OK(CreateKuduTable("default", "MyTable"));
+  NO_FATALS(CheckTable("default", "MyTable"));
+  NO_FATALS(CheckTable("default", "mytable"));
+  NO_FATALS(CheckTable("default", "MYTABLE"));
+
+  // Kudu table schema lookups should be case-insensitive.
+  for (const auto& name : { "default.MyTable",
+                            "default.mytable",
+                            "DEFAULT.MYTABLE",
+                            "default.mYtABLE" }) {
+    shared_ptr<KuduTable> table;
+    ASSERT_OK(client_->OpenTable(name, &table));
+    // The client uses the requested name as the table object's name.
+    ASSERT_EQ(name, table->name());
+  }
+
+  // Listing tables shows the preserved case.
+  vector<string> tables;
+  ASSERT_OK(client_->ListTables(&tables));
+  ASSERT_EQ(tables, vector<string>({ "default.MyTable" }));
+
+  // Rename the table to the same normalized name, but with a different case.
+  unique_ptr<KuduTableAlterer> table_alterer;
+  table_alterer.reset(client_->NewTableAlterer("default.mytable"));
+  ASSERT_OK(table_alterer->RenameTo("DEFAULT.MYTABLE")->Alter());
+  NO_FATALS(CheckTable("default", "MyTable"));
+  NO_FATALS(CheckTable("default", "mytable"));
+  NO_FATALS(CheckTable("default", "MYTABLE"));
+
+  // The master should retain the new case.
+  tables.clear();
+  ASSERT_OK(client_->ListTables(&tables));
+  ASSERT_EQ(tables, vector<string>({ "DEFAULT.MYTABLE" }));
+
+  // Rename the table to something different.
+  table_alterer.reset(client_->NewTableAlterer("DEFAULT.MYTABLE"));
+  ASSERT_OK(table_alterer->RenameTo("default.T_1/1")->Alter());
+  NO_FATALS(CheckTable("default", "T_1/1"));
+  NO_FATALS(CheckTable("default", "t_1/1"));
+  NO_FATALS(CheckTable("DEFAULT", "T_1/1"));
+
+  // Rename the table through the HMS.
+  ASSERT_OK(RenameHmsTable("default", "T_1/1", "AbC"));
+  ASSERT_EVENTUALLY([&] {
+    NO_FATALS(CheckTable("default", "AbC"));
+  });
+
+  // Listing tables shows the preserved case.
+  tables.clear();
+  ASSERT_OK(client_->ListTables(&tables));
+  ASSERT_EQ(tables, vector<string>({ "default.AbC" }));
+
+  // Drop the table.
+  ASSERT_OK(client_->DeleteTable("DEFAULT.abc"));
+  NO_FATALS(CheckTableDoesNotExist("default", "AbC"));
+  NO_FATALS(CheckTableDoesNotExist("default", "abc"));
+}
+
+class MasterHmsUpgradeTest : public MasterHmsTest {
+ public:
+  HmsMode GetHmsMode() override {
+    return HmsMode::ENABLE_HIVE_METASTORE;
+  }
+};
+
+TEST_F(MasterHmsUpgradeTest, TestConflictingNormalizedNames) {
+  ASSERT_OK(CreateKuduTable("default", "MyTable"));
+  ASSERT_OK(CreateKuduTable("default", "mytable"));
+
+  // Shutdown the masters and turn on the HMS integration. The masters should
+  // fail to startup because of conflicting normalized table names.
+  cluster_->ShutdownNodes(cluster::ClusterNodes::MASTERS_ONLY);
+  cluster_->EnableMetastoreIntegration();
+  // Typically, restarting the cluster will fail because the master will
+  // immediately try to elect itself leader, and CHECK fail upon seeing the
+  // conflicting table names. However, in TSAN or otherwise slow situations the
+  // master may be able to register the tablet server before attempting to elect
+  // itself leader, in which case ExternalMiniCluster::Restart() can succeed. In
+  // this situation a fallback to a leader-only API will deterministically fail.
+  Status s = cluster_->Restart();
+  if (s.ok()) {
+    vector<string> tables;
+    s = client_->ListTables(&tables);
+  }
+  ASSERT_TRUE(s.IsNetworkError()) << s.ToString();
+
+  // Disable the metastore integration and rename one of the conflicting tables.
+  cluster_->ShutdownNodes(cluster::ClusterNodes::MASTERS_ONLY);
+  cluster_->DisableMetastoreIntegration();
+  ASSERT_OK(cluster_->Restart());
+  unique_ptr<KuduTableAlterer> table_alterer(client_->NewTableAlterer("default.mytable"));
+  ASSERT_OK(table_alterer->RenameTo("default.mytable-renamed")->Alter());
+
+  // Try again to enable the integration.
+  cluster_->ShutdownNodes(cluster::ClusterNodes::MASTERS_ONLY);
+  cluster_->EnableMetastoreIntegration();
+  ASSERT_OK(cluster_->Restart());
+
+  vector<string> tables;
+  client_->ListTables(&tables);
+  std::sort(tables.begin(), tables.end());
+  ASSERT_EQ(tables, vector<string>({ "default.MyTable", "default.mytable-renamed" }));
 }
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/7b048b8d/src/kudu/master/catalog_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc
index 7094a70..8f58924 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -315,7 +315,18 @@ class TableLoader : public TableVisitor {
     bool is_deleted = l.mutable_data()->is_deleted();
     catalog_manager_->table_ids_map_[table->id()] = table;
     if (!is_deleted) {
-      catalog_manager_->table_names_map_[l.data().name()] = table;
+      auto* existing = InsertOrReturnExisting(&catalog_manager_->normalized_table_names_map_,
+                                              CatalogManager::NormalizeTableName(l.data().name()),
+                                              table);
+      if (existing) {
+        // Return an HMS-specific error message, since this error currently only
+        // occurs when the HMS is enabled.
+        return Status::IllegalState(
+            "when the Hive Metastore integration is enabled, Kudu table names must not differ "
+            "only by case; restart the master(s) with the Hive Metastore integration disabled and "
+            "rename one of the conflicting tables",
+            Substitute("$0 or $1 [id=$2]", (*existing)->ToString(), l.data().name(), table_id));
+      }
     }
     l.Commit();
 
@@ -1126,7 +1137,7 @@ Status CatalogManager::VisitTablesAndTabletsUnlocked() {
   AbortAndWaitForAllTasks(tables);
 
   // Clear the existing state.
-  table_names_map_.clear();
+  normalized_table_names_map_.clear();
   table_ids_map_.clear();
   tablet_map_.clear();
 
@@ -1353,6 +1364,7 @@ Status CatalogManager::CreateTable(const CreateTableRequestPB* orig_req,
   Schema client_schema;
   RETURN_NOT_OK(SchemaFromPB(req.schema(), &client_schema));
   const string& table_name = req.name();
+  string normalized_table_name = NormalizeTableName(table_name);
 
   RETURN_NOT_OK(SetupError(ValidateClientSchema(table_name, client_schema),
                            resp, MasterErrorPB::INVALID_SCHEMA));
@@ -1494,7 +1506,7 @@ Status CatalogManager::CreateTable(const CreateTableRequestPB* orig_req,
     TRACE("Acquired catalog manager lock");
 
     // b. Verify that the table does not exist.
-    table = FindPtrOrNull(table_names_map_, table_name);
+    table = FindPtrOrNull(normalized_table_names_map_, normalized_table_name);
     if (table != nullptr) {
       return SetupError(Status::AlreadyPresent(Substitute(
               "table $0 already exists with id $1", table_name, table->id())),
@@ -1502,7 +1514,7 @@ Status CatalogManager::CreateTable(const CreateTableRequestPB* orig_req,
     }
 
     // c. Reserve the table name if possible.
-    if (!InsertIfNotPresent(&reserved_table_names_, table_name)) {
+    if (!InsertIfNotPresent(&reserved_normalized_table_names_, normalized_table_name)) {
       // ServiceUnavailable will cause the client to retry the create table
       // request. We don't want to outright fail the request with
       // 'AlreadyPresent', because a table name reservation can be rolled back
@@ -1517,7 +1529,7 @@ Status CatalogManager::CreateTable(const CreateTableRequestPB* orig_req,
   // Ensure that we drop the name reservation upon return.
   SCOPED_CLEANUP({
     std::lock_guard<LockType> l(lock_);
-    CHECK_EQ(1, reserved_table_names_.erase(table_name));
+    CHECK_EQ(1, reserved_normalized_table_names_.erase(normalized_table_name));
   });
 
   // d. Create the in-memory representation of the new table and its tablets.
@@ -1610,7 +1622,7 @@ Status CatalogManager::CreateTable(const CreateTableRequestPB* orig_req,
     std::lock_guard<LockType> l(lock_);
 
     table_ids_map_[table->id()] = table;
-    table_names_map_[table_name] = table;
+    normalized_table_names_map_[normalized_table_name] = table;
     for (const auto& tablet : tablets) {
       InsertOrDie(&tablet_map_, tablet->id(), tablet);
     }
@@ -1695,11 +1707,13 @@ Status CatalogManager::FindAndLockTable(const ReqClass& request,
       // If the request contains both a table ID and table name, ensure that
       // both match the same table.
       if (table_identifier.has_table_name() &&
-          table.get() != FindPtrOrNull(table_names_map_, table_identifier.table_name()).get()) {
+          table.get() != FindPtrOrNull(normalized_table_names_map_,
+                                       NormalizeTableName(table_identifier.table_name())).get()) {
         return tnf_error();
       }
     } else if (table_identifier.has_table_name()) {
-      table = FindPtrOrNull(table_names_map_, table_identifier.table_name());
+      table = FindPtrOrNull(normalized_table_names_map_,
+                            NormalizeTableName(table_identifier.table_name()));
     } else {
       return SetupError(Status::InvalidArgument("missing table ID or table name"),
                         response, MasterErrorPB::UNKNOWN_ERROR);
@@ -1714,7 +1728,8 @@ Status CatalogManager::FindAndLockTable(const ReqClass& request,
   // Acquire the table lock.
   TableMetadataLock lock(table.get(), lock_mode);
 
-  if (table_identifier.has_table_name() && table_identifier.table_name() != lock.data().name()) {
+  if (table_identifier.has_table_name() &&
+      NormalizeTableName(table_identifier.table_name()) != NormalizeTableName(lock.data().name())) {
     // We've encountered the table while it's in the process of being renamed;
     // pretend it doesn't yet exist.
     return tnf_error();
@@ -1843,7 +1858,7 @@ Status CatalogManager::DeleteTable(const DeleteTableRequestPB& req,
     {
       TRACE("Removing table from by-name map");
       std::lock_guard<LockType> l_map(lock_);
-      if (table_names_map_.erase(l.data().name()) != 1) {
+      if (normalized_table_names_map_.erase(NormalizeTableName(l.data().name())) != 1) {
         LOG(FATAL) << "Could not remove table " << table->ToString()
                    << " from map in response to DeleteTable request: "
                    << SecureShortDebugString(req);
@@ -2143,17 +2158,16 @@ Status CatalogManager::AlterTableRpc(const AlterTableRequestPB& req,
   // that event to the catalog. By 'serializing' the rename through the
   // HMS, race conditions are avoided.
   if (hms_catalog_ && req.has_new_table_name() && req.alter_external_catalogs()) {
-    // Look up the table, lock it, and mark it as removed.
+    // Look up the table and lock it.
     scoped_refptr<TableInfo> table;
     TableMetadataLock l;
     RETURN_NOT_OK(FindAndLockTable(req, resp, LockMode::READ, &table, &l));
     RETURN_NOT_OK(CheckIfTableDeletedOrNotRunning(&l, resp));
 
-    // The HMS allows renaming a table to the same name (ALTER TABLE t RENAME TO t;),
-    // however Kudu does not, so we must validate this case ourselves. This
-    // invariant is also checked in CatalogManager::AlterTable, but when the HMS
-    // integration is enabled that check does not bubble up to the client (it's
-    // called by the notification log listener).
+    // The HMS allows renaming a table to the same name (ALTER TABLE t RENAME TO t),
+    // however Kudu does not, so we must enforce this constraint ourselves before
+    // altering the table in the HMS. The comparison is on the non-normalized
+    // table names, since we want to allow changing the case of a table name.
     if (l.data().name() == req.new_table_name()) {
       return SetupError(
           Status::AlreadyPresent(Substitute("table $0 already exists with id $1",
@@ -2248,6 +2262,7 @@ Status CatalogManager::AlterTable(const AlterTableRequestPB& req,
   }
 
   string table_name = l.data().name();
+  string normalized_table_name = NormalizeTableName(table_name);
   *resp->mutable_table_id() = table->id();
 
   // 3. Calculate and validate new schema for the on-disk state, not persisted yet.
@@ -2272,33 +2287,47 @@ Status CatalogManager::AlterTable(const AlterTableRequestPB& req,
         resp, MasterErrorPB::INVALID_SCHEMA));
 
   // 4. Validate and try to acquire the new table name.
+  bool do_cleanup = false;
+  string normalized_new_table_name = NormalizeTableName(req.new_table_name());
   if (req.has_new_table_name()) {
     RETURN_NOT_OK(SetupError(
           ValidateIdentifier(req.new_table_name()).CloneAndPrepend("invalid table name"),
           resp, MasterErrorPB::INVALID_SCHEMA));
 
-    std::lock_guard<LockType> catalog_lock(lock_);
-    TRACE("Acquired catalog manager lock");
-
-    // Verify that the table does not exist.
-    scoped_refptr<TableInfo> other_table = FindPtrOrNull(table_names_map_, req.new_table_name());
-    if (other_table != nullptr) {
-      return SetupError(
-          Status::AlreadyPresent(Substitute("table $0 already exists with id $1",
-              req.new_table_name(), table->id())),
-          resp, MasterErrorPB::TABLE_ALREADY_PRESENT);
-    }
+    // Validate the new table name.
+    //
+    // Special case: the new table name and existing table names are different,
+    // but map to the same normalized name. In this case we don't need to
+    // reserve the new table name, since it's already reserved by way of
+    // existing in the by-name map.
+    if (!(table_name != req.new_table_name() &&
+          normalized_table_name == normalized_new_table_name)) {
+      std::lock_guard<LockType> catalog_lock(lock_);
+      TRACE("Acquired catalog manager lock");
+
+      // Verify that the table does not exist.
+      scoped_refptr<TableInfo> other_table = FindPtrOrNull(normalized_table_names_map_,
+                                                           normalized_new_table_name);
+
+      if (other_table != nullptr) {
+        return SetupError(
+            Status::AlreadyPresent(Substitute("table $0 already exists with id $1",
+                req.new_table_name(), table->id())),
+            resp, MasterErrorPB::TABLE_ALREADY_PRESENT);
+      }
 
-    // Reserve the new table name if possible.
-    if (!InsertIfNotPresent(&reserved_table_names_, req.new_table_name())) {
-      // ServiceUnavailable will cause the client to retry the create table
-      // request. We don't want to outright fail the request with
-      // 'AlreadyPresent', because a table name reservation can be rolled back
-      // in the case of an error. Instead, we force the client to retry at a
-      // later time.
-      return SetupError(Status::ServiceUnavailable(Substitute(
-              "table name $0 is already reserved", req.new_table_name())),
-          resp, MasterErrorPB::TABLE_ALREADY_PRESENT);
+      // Reserve the new table name if possible.
+      if (!InsertIfNotPresent(&reserved_normalized_table_names_, normalized_new_table_name)) {
+        // ServiceUnavailable will cause the client to retry the create table
+        // request. We don't want to outright fail the request with
+        // 'AlreadyPresent', because a table name reservation can be rolled back
+        // in the case of an error. Instead, we force the client to retry at a
+        // later time.
+        return SetupError(Status::ServiceUnavailable(Substitute(
+                "table name $0 is already reserved", req.new_table_name())),
+            resp, MasterErrorPB::TABLE_ALREADY_PRESENT);
+      }
+      do_cleanup = true;
     }
 
     l.mutable_data()->pb.set_name(req.new_table_name());
@@ -2306,9 +2335,9 @@ Status CatalogManager::AlterTable(const AlterTableRequestPB& req,
 
   // Ensure that we drop our reservation upon return.
   auto cleanup = MakeScopedCleanup([&] () {
-    if (req.has_new_table_name()) {
+    if (do_cleanup) {
       std::lock_guard<LockType> l(lock_);
-      CHECK_EQ(1, reserved_table_names_.erase(req.new_table_name()));
+      CHECK_EQ(1, reserved_normalized_table_names_.erase(normalized_new_table_name));
     }
   });
 
@@ -2405,12 +2434,12 @@ Status CatalogManager::AlterTable(const AlterTableRequestPB& req,
     // and tablets indices.
     std::lock_guard<LockType> lock(lock_);
     if (req.has_new_table_name()) {
-      if (table_names_map_.erase(table_name) != 1) {
+      if (normalized_table_names_map_.erase(normalized_table_name) != 1) {
         LOG(FATAL) << "Could not remove table " << table->ToString()
                    << " from map in response to AlterTable request: "
                    << SecureShortDebugString(req);
       }
-      InsertOrDie(&table_names_map_, req.new_table_name(), table);
+      InsertOrDie(&normalized_table_names_map_, normalized_new_table_name, table);
     }
 
     // Insert new tablets into the global tablet map. After this, the tablets
@@ -2532,7 +2561,7 @@ Status CatalogManager::ListTables(const ListTablesRequestPB* req,
 
   shared_lock<LockType> l(lock_);
 
-  for (const TableInfoMap::value_type& entry : table_names_map_) {
+  for (const TableInfoMap::value_type& entry : normalized_table_names_map_) {
     TableMetadataLock ltm(entry.second.get(), LockMode::READ);
     if (!ltm.data().is_running()) continue; // implies !is_deleted() too
 
@@ -2543,7 +2572,7 @@ Status CatalogManager::ListTables(const ListTablesRequestPB* req,
       }
     }
 
-    ListTablesResponsePB::TableInfo *table = resp->add_tables();
+    ListTablesResponsePB::TableInfo* table = resp->add_tables();
     table->set_id(entry.second->id());
     table->set_name(ltm.data().name());
   }
@@ -2576,7 +2605,7 @@ Status CatalogManager::TableNameExists(const string& table_name, bool* exists) {
   RETURN_NOT_OK(CheckOnline());
 
   shared_lock<LockType> l(lock_);
-  *exists = ContainsKey(table_names_map_, table_name);
+  *exists = ContainsKey(normalized_table_names_map_, NormalizeTableName(table_name));
   return Status::OK();
 }
 
@@ -4674,7 +4703,7 @@ void CatalogManager::DumpState(std::ostream* out) const {
   {
     shared_lock<LockType> l(lock_);
     ids_copy = table_ids_map_;
-    names_copy = table_names_map_;
+    names_copy = normalized_table_names_map_;
     tablets_copy = tablet_map_;
     // TODO(aserbin): add information about root CA certs, if any
   }
@@ -4759,6 +4788,28 @@ Status CatalogManager::WaitForNotificationLogListenerCatchUp(RespClass* resp,
   return Status::OK();
 }
 
+string CatalogManager::NormalizeTableName(const string& table_name) {
+  // Force a deep copy on platforms with reference counted strings.
+  string normalized_table_name(table_name.data(), table_name.size());
+  if (hms::HmsCatalog::IsEnabled()) {
+    // If HmsCatalog::NormalizeTableName returns an error, the table name is not
+    // modified. In this case the table is guaranteed to be a legacy table which
+    // has survived since before the cluster was configured to integrate with
+    // the HMS. It's safe to use the unmodified table name as the normalized
+    // name in this case, since there cannot be a name conflict with a table in
+    // the HMS. When the table gets 'upgraded' to be included in the HMS it will
+    // need to be renamed with a Hive compatible name.
+    //
+    // Note: not all legacy tables will fail normalization; if a table happens
+    // to be named with a Hive compatible name ("Legacy.Table"), it will be
+    // normalized according to the Hive rules ("legacy.table"). We check in
+    // TableLoader::VisitTables that such legacy tables do not have conflicting
+    // names when normalized.
+    ignore_result(hms::HmsCatalog::NormalizeTableName(&normalized_table_name));
+  }
+  return normalized_table_name;
+}
+
 const char* CatalogManager::StateToString(State state) {
   switch (state) {
     case CatalogManager::kConstructed: return "Constructed";

http://git-wip-us.apache.org/repos/asf/kudu/blob/7b048b8d/src/kudu/master/catalog_manager.h
----------------------------------------------------------------------
diff --git a/src/kudu/master/catalog_manager.h b/src/kudu/master/catalog_manager.h
index d27975a..3f13e38 100644
--- a/src/kudu/master/catalog_manager.h
+++ b/src/kudu/master/catalog_manager.h
@@ -683,6 +683,16 @@ class CatalogManager : public tserver::TabletReplicaLookupIf {
     return hms_catalog_.get();
   }
 
+  // Returns the normalized form of the provided table name.
+  //
+  // If the HMS integration is configured and the table name is a valid HMS
+  // table name, the normalized form is returned (see HmsCatalog::NormalizeTableName),
+  // otherwise a copy of the original table name is returned.
+  //
+  // If the HMS integration is not configured then a copy of the original table
+  // name is returned.
+  static std::string NormalizeTableName(const std::string& table_name);
+
  private:
   // These tests call ElectedAsLeaderCb() directly.
   FRIEND_TEST(MasterTest, TestShutdownDuringTableVisit);
@@ -753,9 +763,8 @@ class CatalogManager : public tserver::TabletReplicaLookupIf {
   // of authn tokens.
   Status PrepareFollowerTokenVerifier();
 
-  // Clears out the existing metadata ('table_names_map_', 'table_ids_map_',
-  // and 'tablet_map_'), loads tables metadata into memory and if successful
-  // loads the tablets metadata.
+  // Clears out the existing metadata (by-name map, table-id map, and tablet
+  // map), and loads table and tablet metadata into memory.
   Status VisitTablesAndTabletsUnlocked();
   // This is called by tests only.
   Status VisitTablesAndTablets();
@@ -953,26 +962,25 @@ class CatalogManager : public tserver::TabletReplicaLookupIf {
   typedef rw_spinlock LockType;
   mutable LockType lock_;
 
-  // Table maps: table-id -> TableInfo and table-name -> TableInfo
+  // Table maps: table-id -> TableInfo and normalized-table-name -> TableInfo
   TableInfoMap table_ids_map_;
-  TableInfoMap table_names_map_;
+  TableInfoMap normalized_table_names_map_;
 
   // Tablet maps: tablet-id -> TabletInfo
   TabletInfoMap tablet_map_;
 
-  // Names of tables that are currently reserved by CreateTable() or
-  // AlterTable().
+  // Names of tables that are currently reserved by CreateTable() or AlterTable().
   //
   // As a rule, operations that add new table names should do so as follows:
   // 1. Acquire lock_.
-  // 2. Ensure table_names_map_ does not contain the new name.
-  // 3. Ensure reserved_table_names_ does not contain the new name.
-  // 4. Add the new name to reserved_table_names_.
+  // 2. Ensure normalized_table_names_map_ does not contain the new normalized name.
+  // 3. Ensure reserved_normalized_table_names_ does not contain the new normalized name.
+  // 4. Add the new normalized name to reserved_normalized_table_names_.
   // 5. Release lock_.
   // 6. Perform the operation.
-  // 7. If it succeeded, add the name to table_names_map_ with lock_ held.
-  // 8. Remove the new name from reserved_table_names_ with lock_ held.
-  std::unordered_set<std::string> reserved_table_names_;
+  // 7. If it succeeded, add the normalized name to normalized_table_names_map_ with lock_ held.
+  // 8. Remove the new normalized name from reserved_normalized_table_names_ with lock_ held.
+  std::unordered_set<std::string> reserved_normalized_table_names_;
 
   Master *master_;
   ObjectIdGenerator oid_generator_;

http://git-wip-us.apache.org/repos/asf/kudu/blob/7b048b8d/src/kudu/mini-cluster/external_mini_cluster.cc
----------------------------------------------------------------------
diff --git a/src/kudu/mini-cluster/external_mini_cluster.cc b/src/kudu/mini-cluster/external_mini_cluster.cc
index 7db8edb..a188ad5 100644
--- a/src/kudu/mini-cluster/external_mini_cluster.cc
+++ b/src/kudu/mini-cluster/external_mini_cluster.cc
@@ -37,6 +37,7 @@
 #include "kudu/common/wire_protocol.pb.h"
 #include "kudu/gutil/basictypes.h"
 #include "kudu/gutil/strings/join.h"
+#include "kudu/gutil/strings/stringpiece.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/gutil/strings/util.h"
 #include "kudu/hms/mini_hms.h"
@@ -260,6 +261,20 @@ void ExternalMiniCluster::EnableMetastoreIntegration() {
   opts_.hms_mode = HmsMode::ENABLE_METASTORE_INTEGRATION;
 }
 
+void ExternalMiniCluster::DisableMetastoreIntegration() {
+  for (const auto& master : masters_) {
+    CHECK(master->IsShutdown()) << "Call Shutdown() before changing the HMS mode";
+    master->mutable_flags()->erase(
+        std::remove_if(
+          master->mutable_flags()->begin(), master->mutable_flags()->end(),
+          [] (const string& flag) {
+            return StringPiece(flag).starts_with("--hive_metastore");
+          }),
+        master->mutable_flags()->end());
+  }
+  opts_.hms_mode = HmsMode::ENABLE_HIVE_METASTORE;
+}
+
 void ExternalMiniCluster::SetDaemonBinPath(string daemon_bin_path) {
   opts_.daemon_bin_path = std::move(daemon_bin_path);
   for (auto& master : masters_) {

http://git-wip-us.apache.org/repos/asf/kudu/blob/7b048b8d/src/kudu/mini-cluster/external_mini_cluster.h
----------------------------------------------------------------------
diff --git a/src/kudu/mini-cluster/external_mini_cluster.h b/src/kudu/mini-cluster/external_mini_cluster.h
index 15c9b02..7b29e00 100644
--- a/src/kudu/mini-cluster/external_mini_cluster.h
+++ b/src/kudu/mini-cluster/external_mini_cluster.h
@@ -320,9 +320,15 @@ class ExternalMiniCluster : public MiniCluster {
                  const std::string& value) WARN_UNUSED_RESULT;
 
   // Enable Hive Metastore integration.
-  // Overrides 'enable_metastore_integration' set by ExternalMiniClusterOptions.
+  // Overrides HMS integration options set by ExternalMiniClusterOptions.
+  // The cluster must be shut down before calling this method.
   void EnableMetastoreIntegration();
 
+  // Disable Hive Metastore integration.
+  // Overrides HMS integration options set by ExternalMiniClusterOptions.
+  // The cluster must be shut down before calling this method.
+  void DisableMetastoreIntegration();
+
   // Set the path where daemon binaries can be found.
   // Overrides 'daemon_bin_path' set by ExternalMiniClusterOptions.
   // The cluster must be shut down before calling this method.


Mime
View raw message