You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by al...@apache.org on 2020/11/13 22:53:07 UTC

[kudu] branch master updated: [catalog] logging for HMS-related operations

This is an automated email from the ASF dual-hosted git repository.

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


The following commit(s) were added to refs/heads/master by this push:
     new 6d9f257  [catalog] logging for HMS-related operations
6d9f257 is described below

commit 6d9f257ad48c11a4546545dee096c484623cc392
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Fri Nov 13 11:23:33 2020 -0800

    [catalog] logging for HMS-related operations
    
    This patch adds more logging for results of HMS catalog's operations.
    Since the number of DDL operations is usually not very high, this new
    logging should not contribute significantly to the amount of information
    logged.
    
    The motivation for this patch is to increase the supportability of Kudu
    clusters where HMS integration is enabled.
    
    Change-Id: Iab09e9cdd9a994b62f728cdc218e5e905488a420
    Reviewed-on: http://gerrit.cloudera.org:8080/16722
    Tested-by: Kudu Jenkins
    Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
 src/kudu/master/catalog_manager.cc | 70 +++++++++++++++++++++++++++++---------
 1 file changed, 53 insertions(+), 17 deletions(-)

diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc
index 20b358c..4686933 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -1874,23 +1874,32 @@ Status CatalogManager::CreateTable(const CreateTableRequestPB* orig_req,
   // since this step validates that the table name is available in the HMS catalog.
   if (hms_catalog_ && is_user_table) {
     CHECK(rpc);
-    Status s = hms_catalog_->CreateTable(table->id(), normalized_table_name, GetClusterId(),
-        req.owner(), schema);
+    Status s = hms_catalog_->CreateTable(
+        table->id(), normalized_table_name, GetClusterId(), req.owner(), schema);
     if (!s.ok()) {
-      s = s.CloneAndPrepend(Substitute("an error occurred while creating table $0 in the HMS",
-                                       normalized_table_name));
+      s = s.CloneAndPrepend(Substitute(
+          "failed to create HMS catalog entry for table $0", table->ToString()));
       LOG(WARNING) << s.ToString();
       return SetupError(std::move(s), resp, MasterErrorPB::HIVE_METASTORE_ERROR);
     }
     TRACE("Created new table in HMS catalog");
+    LOG(INFO) << Substitute("created HMS catalog entry for table $0",
+                            table->ToString());
   }
   // Delete the new HMS entry if we exit early.
   auto abort_hms = MakeScopedCleanup([&] {
       // TODO(dan): figure out how to test this.
       if (hms_catalog_ && is_user_table) {
         TRACE("Rolling back HMS table creation");
-        WARN_NOT_OK(hms_catalog_->DropTable(table->id(), normalized_table_name),
-                    "an error occurred while attempting to delete orphaned HMS table entry");
+        auto s = hms_catalog_->DropTable(table->id(), normalized_table_name);
+        if (s.ok()) {
+          LOG(INFO) << Substitute(
+              "deleted orphaned HMS catalog entry for table $0", table->ToString());
+        } else {
+          LOG(WARNING) << Substitute(
+              "failed to delete orphaned HMS catalog entry for table $0: $1",
+              table->ToString(), s.ToString());
+        }
       }
   });
 
@@ -2182,9 +2191,17 @@ Status CatalogManager::DeleteTableRpc(const DeleteTableRequestPB& req,
     }
 
     // Drop the table from the HMS.
+    auto s = hms_catalog_->DropTable(table->id(), l.data().name());
+    if (PREDICT_TRUE(s.ok())) {
+      LOG(INFO) << Substitute(
+          "deleted HMS catalog entry for table $0", table->ToString());
+    } else {
+      LOG(WARNING) << Substitute(
+          "failed to delete HMS catalog entry for table $0: $1",
+          table->ToString(), s.ToString());
+    }
     RETURN_NOT_OK(SetupError(
-          hms_catalog_->DropTable(table->id(), l.data().name()),
-          resp, MasterErrorPB::HIVE_METASTORE_ERROR));
+        std::move(s), resp, MasterErrorPB::HIVE_METASTORE_ERROR));
 
     // Unlock the table, and wait for the notification log listener to handle
     // the delete table event.
@@ -2664,10 +2681,22 @@ Status CatalogManager::AlterTableRpc(const AlterTableRequestPB& req,
     RETURN_NOT_OK(SchemaFromPB(l.data().pb.schema(), &schema));
 
     // Rename the table in the HMS.
-    RETURN_NOT_OK(SetupError(hms_catalog_->AlterTable(
-            table->id(), l.data().name(), normalized_new_table_name,
-            GetClusterId(), l.data().owner(), schema),
-        resp, MasterErrorPB::HIVE_METASTORE_ERROR));
+    auto s = hms_catalog_->AlterTable(table->id(),
+                                      l.data().name(),
+                                      normalized_new_table_name,
+                                      GetClusterId(),
+                                      l.data().owner(),
+                                      schema);
+    if (PREDICT_TRUE(s.ok())) {
+      LOG(INFO) << Substitute("renamed table $0 in HMS: new name $1",
+                              table->ToString(), normalized_new_table_name);
+    } else {
+      LOG(WARNING) << Substitute(
+          "failed to rename table $0 in HMS: new name $1: $2",
+          table->ToString(), normalized_new_table_name, s.ToString());
+    }
+    RETURN_NOT_OK(SetupError(
+        std::move(s), resp, MasterErrorPB::HIVE_METASTORE_ERROR));
 
     // Unlock the table, and wait for the notification log listener to handle
     // the alter table event.
@@ -3035,11 +3064,18 @@ Status CatalogManager::AlterTable(const AlterTableRequestPB& req,
     // table rename, since we split out the rename portion into its own
     // 'transaction' which is serialized through the HMS.
     DCHECK(!req.has_new_table_name());
-    WARN_NOT_OK(hms_catalog_->AlterTable(
-          table->id(), normalized_table_name, normalized_table_name,
-          GetClusterId(), l.mutable_data()->owner(), new_schema),
-        Substitute("failed to alter HiveMetastore schema for table $0, "
-                   "HMS schema information will be stale", table->ToString()));
+    auto s = hms_catalog_->AlterTable(
+        table->id(), normalized_table_name, normalized_table_name,
+        GetClusterId(), l.mutable_data()->owner(), new_schema);
+    if (PREDICT_TRUE(s.ok())) {
+      LOG(INFO) << Substitute(
+          "altered HMS schema for table $0", table->ToString());
+    } else {
+      LOG(WARNING) << Substitute(
+          "failed to alter HMS schema for table $0, "
+          "HMS schema information will be stale: $1",
+          table->ToString(), s.ToString());
+    }
   }
 
   if (!tablets_to_add.empty() || has_metadata_changes) {