kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a...@apache.org
Subject [kudu] 01/05: hms: followup to 6a8d1d2
Date Fri, 24 May 2019 04:35:57 GMT
This is an automated email from the ASF dual-hosted git repository.

adar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit 4fed496872dfd57e2022d9f8c387988f82d5e203
Author: Andrew Wong <awong@apache.org>
AuthorDate: Wed May 22 19:36:57 2019 -0700

    hms: followup to 6a8d1d2
    
    Commit 6a8d1d2 left room for a little bit of refactoring and testing.
    This patch fills this room:
    - minor refactoring in tool_action_hms
    - tested that altering and dropping external tables won't affect
      underlying Kudu data
    
    There are no functional changes in this patch.
    
    Change-Id: I0d624258e1d177363fceba6b40d6aee77b142b54
    Reviewed-on: http://gerrit.cloudera.org:8080/13410
    Reviewed-by: Alexey Serbin <aserbin@cloudera.com>
    Tested-by: Andrew Wong <awong@cloudera.com>
    Reviewed-by: Hao Hao <hao.hao@cloudera.com>
---
 src/kudu/integration-tests/hms_itest-base.cc   |  9 ++++-
 src/kudu/integration-tests/hms_itest-base.h    |  5 ++-
 src/kudu/integration-tests/master_hms-itest.cc | 32 +++++++++++++++++
 src/kudu/tools/tool_action_hms.cc              | 49 ++++++++++++--------------
 4 files changed, 66 insertions(+), 29 deletions(-)

diff --git a/src/kudu/integration-tests/hms_itest-base.cc b/src/kudu/integration-tests/hms_itest-base.cc
index 547c2ab..3078ede 100644
--- a/src/kudu/integration-tests/hms_itest-base.cc
+++ b/src/kudu/integration-tests/hms_itest-base.cc
@@ -114,7 +114,8 @@ Status HmsITestBase::CreateKuduTable(const string& database_name,
 
 Status HmsITestBase::CreateHmsTable(const string& database_name,
                                     const string& table_name,
-                                    const string& table_type) {
+                                    const string& table_type,
+                                    boost::optional<const string&> kudu_table_name)
{
   hive::Table hms_table;
   hms_table.dbName = database_name;
   hms_table.tableName = table_name;
@@ -123,6 +124,12 @@ Status HmsITestBase::CreateHmsTable(const string& database_name,
   if (table_type == HmsClient::kExternalTable) {
     hms_table.parameters[HmsClient::kExternalTableKey] = "TRUE";
   }
+  if (kudu_table_name) {
+    hms_table.parameters[HmsClient::kKuduTableNameKey] = *kudu_table_name;
+  } else {
+    hms_table.parameters[HmsClient::kKuduTableNameKey] =
+        Substitute("$0.$1", database_name, table_name);
+  }
   return hms_client_->CreateTable(hms_table);
 }
 
diff --git a/src/kudu/integration-tests/hms_itest-base.h b/src/kudu/integration-tests/hms_itest-base.h
index b3fb011..ca30c9a 100644
--- a/src/kudu/integration-tests/hms_itest-base.h
+++ b/src/kudu/integration-tests/hms_itest-base.h
@@ -45,9 +45,12 @@ class HmsITestBase : public ExternalMiniClusterITestBase {
                          MonoDelta timeout = {}) WARN_UNUSED_RESULT;
 
   // Creates a table in the HMS catalog.
+  // If supplied, 'kudu_table_name' will be used for the 'kudu.table_name'
+  // field of the HMS entry.
   Status CreateHmsTable(const std::string& database_name,
                         const std::string& table_name,
-                        const std::string& table_type = hms::HmsClient::kManagedTable);
+                        const std::string& table_type = hms::HmsClient::kManagedTable,
+                        boost::optional<const std::string&> kudu_table_name = boost::none);
 
   // Renames a table entry in the HMS catalog.
   Status RenameHmsTable(const std::string& database_name,
diff --git a/src/kudu/integration-tests/master_hms-itest.cc b/src/kudu/integration-tests/master_hms-itest.cc
index 6704fec..4c68fce 100644
--- a/src/kudu/integration-tests/master_hms-itest.cc
+++ b/src/kudu/integration-tests/master_hms-itest.cc
@@ -16,6 +16,7 @@
 // under the License.
 
 #include <algorithm>
+#include <map>
 #include <memory>
 #include <string>
 #include <vector>
@@ -389,6 +390,37 @@ TEST_F(MasterHmsTest, TestNotificationLogListener) {
   NO_FATALS(CheckTableDoesNotExist("default", "a"));
 }
 
+TEST_F(MasterHmsTest, TestIgnoreExternalTables) {
+  // Set up a managed table, and a couple of external tables to point at it.
+  ASSERT_OK(CreateKuduTable("default", "managed"));
+  const string kManagedTableName = "default.managed";
+  ASSERT_OK(CreateHmsTable("default", "ext1", HmsClient::kExternalTable, kManagedTableName));
+  ASSERT_OK(CreateHmsTable("default", "ext2", HmsClient::kExternalTable, kManagedTableName));
+
+  // Drop a table in the HMS and check that it didn't affect the underlying
+  // Kudu table.
+  ASSERT_OK(hms_client_->DropTable("default", "ext1"));
+  shared_ptr<KuduTable> table;
+  ASSERT_OK(client_->OpenTable(kManagedTableName, &table));
+
+  // Do the same, but rename the HMS table.
+  hive::Table ext;
+  ASSERT_OK(hms_client_->GetTable("default", "ext2", &ext));
+  ext.tableName = "ext3";
+  ASSERT_OK(hms_client_->AlterTable("default", "ext2", ext));
+  ASSERT_OK(client_->OpenTable(kManagedTableName, &table));
+  Status s = hms_client_->GetTable("default", "ext2", &ext);
+  ASSERT_TRUE(s.IsNotFound()) << s.ToString();
+
+  // Alter the table in Kudu, the external tables in the HMS will not be
+  // altered.
+  unique_ptr<KuduTableAlterer> table_alterer(
+      client_->NewTableAlterer(kManagedTableName)->RenameTo("default.other"));
+  ASSERT_OK(table_alterer->Alter());
+  ASSERT_OK(hms_client_->GetTable("default", "ext3", &ext));
+  ASSERT_EQ(kManagedTableName, ext.parameters[HmsClient::kKuduTableNameKey]);
+}
+
 TEST_F(MasterHmsTest, TestUppercaseIdentifiers) {
   ASSERT_OK(CreateKuduTable("default", "MyTable"));
   NO_FATALS(CheckTable("default", "MyTable", /*user=*/none));
diff --git a/src/kudu/tools/tool_action_hms.cc b/src/kudu/tools/tool_action_hms.cc
index 5ca4d9d..84ad849 100644
--- a/src/kudu/tools/tool_action_hms.cc
+++ b/src/kudu/tools/tool_action_hms.cc
@@ -304,39 +304,34 @@ Status AnalyzeCatalogs(const string& master_addrs,
     vector<hive::Table> hms_tables;
     RETURN_NOT_OK(hms_catalog->GetKuduTables(&hms_tables));
     for (hive::Table& hms_table : hms_tables) {
-      const string &storage_handler = hms_table.parameters[HmsClient::kStorageHandlerKey];
-      // If this is a legacy table or external table, lookup the Kudu table by name.
-      if (storage_handler == HmsClient::kLegacyKuduStorageHandler ||
-          hms_table.tableType == HmsClient::kExternalTable) {
-        const string& hms_table_name = hms_table.parameters[HmsClient::kKuduTableNameKey];
-        shared_ptr<KuduTable>* kudu_table = FindOrNull(kudu_tables_by_name,
-                                                       hms_table_name);
-        if (kudu_table) {
-          if (hms_table.tableType == HmsClient::kExternalTable) {
-            external_hms_tables_by_id[(*kudu_table)->id()].emplace_back(
-                std::move(hms_table));
-          } else {
-            managed_hms_tables_by_id[(*kudu_table)->id()].emplace_back(
-                std::move(hms_table));
-          }
-        } else {
-          // External tables don't always point at valid Kudu tables.
-          if (hms_table.tableType != HmsClient::kExternalTable) {
-            orphan_hms_tables.emplace_back(std::move(hms_table));
-          }
-        }
-      } else if (storage_handler == HmsClient::kKuduStorageHandler) {
+      // If this is a non-legacy, managed table, we expect a table ID to exist
+      // in the HMS entry; look up the Kudu table by ID. Otherwise, look it up
+      // by table name.
+      shared_ptr<KuduTable>* kudu_table;
+      const string& storage_handler = hms_table.parameters[HmsClient::kStorageHandlerKey];
+      if (storage_handler == HmsClient::kKuduStorageHandler &&
+          hms_table.tableType == HmsClient::kManagedTable) {
         // TODO(ghenke): Also lookup by name to support tables created when HMS sync is off.
         // TODO(ghenke): Also lookup by name to support repairing id's out of sync.
         const string& hms_table_id = hms_table.parameters[HmsClient::kKuduTableIdKey];
-        shared_ptr<KuduTable>* kudu_table = FindOrNull(kudu_tables_by_id,
-                                                       hms_table_id);
-        if (kudu_table) {
+        kudu_table = FindOrNull(kudu_tables_by_id, hms_table_id);
+      } else {
+        const string& hms_table_name = hms_table.parameters[HmsClient::kKuduTableNameKey];
+        kudu_table = FindOrNull(kudu_tables_by_name, hms_table_name);
+      }
+
+      if (kudu_table) {
+        if (hms_table.tableType == HmsClient::kExternalTable) {
+          external_hms_tables_by_id[(*kudu_table)->id()].emplace_back(
+              std::move(hms_table));
+        } else if (hms_table.tableType == HmsClient::kManagedTable) {
           managed_hms_tables_by_id[(*kudu_table)->id()].emplace_back(
               std::move(hms_table));
-        } else {
-          orphan_hms_tables.emplace_back(std::move(hms_table));
         }
+      } else if (hms_table.tableType == HmsClient::kManagedTable) {
+        // Note: we only consider managed HMS table entries "orphans" because
+        // external tables don't always point at valid Kudu tables.
+        orphan_hms_tables.emplace_back(std::move(hms_table));
       }
     }
   }


Mime
View raw message