KUDU-2191 (6/n): Fixups to the C++ HMS client These are changes necessary for subsequent patches in the series, but they are easier to review in isolation.
Change-Id: I86a8d984e67fe6b4c004725828b4296476c2389e Reviewed-on: http://gerrit.cloudera.org:8080/9861 Reviewed-by: Adar Dembo <a...@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/fff4185e Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/fff4185e Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/fff4185e Branch: refs/heads/master Commit: fff4185e0f88954c67841a13b0924db4eab4c761 Parents: 4db748f Author: Dan Burkert <danburk...@apache.org> Authored: Wed Mar 28 16:35:45 2018 -0700 Committer: Dan Burkert <danburk...@apache.org> Committed: Mon Apr 2 21:39:49 2018 +0000 ---------------------------------------------------------------------- .../hive/metastore/TestKuduMetastorePlugin.java | 3 +- src/kudu/hms/hms_client-test.cc | 15 +++++-- src/kudu/hms/hms_client.cc | 44 ++++++++++++++++++-- src/kudu/hms/hms_client.h | 18 +++++++- src/kudu/hms/mini_hms.cc | 10 ++++- 5 files changed, 79 insertions(+), 11 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/fff4185e/java/kudu-hive/src/test/java/org/apache/kudu/hive/metastore/TestKuduMetastorePlugin.java ---------------------------------------------------------------------- diff --git a/java/kudu-hive/src/test/java/org/apache/kudu/hive/metastore/TestKuduMetastorePlugin.java b/java/kudu-hive/src/test/java/org/apache/kudu/hive/metastore/TestKuduMetastorePlugin.java index 8eb6927..1abcd6a 100644 --- a/java/kudu-hive/src/test/java/org/apache/kudu/hive/metastore/TestKuduMetastorePlugin.java +++ b/java/kudu-hive/src/test/java/org/apache/kudu/hive/metastore/TestKuduMetastorePlugin.java @@ -28,6 +28,7 @@ import org.apache.hadoop.hive.metastore.HiveMetaStoreClient; import org.apache.hadoop.hive.metastore.MetaStoreUtils; import org.apache.hadoop.hive.metastore.MockPartitionExpressionForMetastore; import org.apache.hadoop.hive.metastore.PartitionExpressionProxy; +import org.apache.hadoop.hive.metastore.TableType; import org.apache.hadoop.hive.metastore.api.EnvironmentContext; import org.apache.hadoop.hive.metastore.api.FieldSchema; import org.apache.hadoop.hive.metastore.api.SerDeInfo; @@ -77,7 +78,7 @@ public class TestKuduMetastorePlugin { Table table = new Table(); table.setDbName("default"); table.setTableName(name); - table.setTableType("MANAGED_TABLE"); + table.setTableType(TableType.MANAGED_TABLE.toString()); table.putToParameters(hive_metastoreConstants.META_TABLE_STORAGE, KuduMetastorePlugin.KUDU_STORAGE_HANDLER); table.putToParameters(KuduMetastorePlugin.KUDU_TABLE_ID_KEY, http://git-wip-us.apache.org/repos/asf/kudu/blob/fff4185e/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 87601eb..a4a6ffe 100644 --- a/src/kudu/hms/hms_client-test.cc +++ b/src/kudu/hms/hms_client-test.cc @@ -17,11 +17,16 @@ #include "kudu/hms/hms_client.h" +#include <algorithm> // IWYU pragma: keep #include <cstdint> +#include <map> // IWYU pragma: keep +#include <memory> // IWYU pragma: keep #include <string> +#include <type_traits> // IWYU pragma: keep #include <utility> #include <vector> +#include <boost/none_t.hpp> // IWYU pragma: keep #include <boost/optional/optional.hpp> #include <glog/stl_logging.h> // IWYU pragma: keep #include <gtest/gtest.h> @@ -59,7 +64,7 @@ class HmsClientTest : public KuduTest, hive::Table table; table.dbName = database_name; table.tableName = table_name; - table.tableType = "MANAGED_TABLE"; + table.tableType = HmsClient::kManagedTable; table.__set_parameters({ make_pair(HmsClient::kKuduTableIdKey, table_id), @@ -122,7 +127,9 @@ TEST_P(HmsClientTest, TestHmsOperations) { ASSERT_OK(hms.Start()); HmsClient client(hms.address(), hms_client_opts); + ASSERT_FALSE(client.IsConnected()); ASSERT_OK(client.Start()); + ASSERT_TRUE(client.IsConnected()); // Create a database. string database_name = "my_db"; @@ -161,7 +168,7 @@ TEST_P(HmsClientTest, TestHmsOperations) { EXPECT_EQ(table_id, my_table.parameters[HmsClient::kKuduTableIdKey]); EXPECT_EQ(HmsClient::kKuduStorageHandler, my_table.parameters[hive::g_hive_metastore_constants.META_TABLE_STORAGE]); - EXPECT_EQ("MANAGED_TABLE", my_table.tableType); + EXPECT_EQ(HmsClient::kManagedTable, my_table.tableType); string new_table_name = "my_altered_table"; @@ -186,7 +193,7 @@ TEST_P(HmsClientTest, TestHmsOperations) { EXPECT_EQ(table_id, renamed_table.parameters[HmsClient::kKuduTableIdKey]); EXPECT_EQ(HmsClient::kKuduStorageHandler, renamed_table.parameters[hive::g_hive_metastore_constants.META_TABLE_STORAGE]); - EXPECT_EQ("MANAGED_TABLE", renamed_table.tableType); + EXPECT_EQ(HmsClient::kManagedTable, renamed_table.tableType); // Create a table with an uppercase name. string uppercase_table_name = "my_UPPERCASE_Table"; @@ -279,7 +286,7 @@ TEST_P(HmsClientTest, TestLargeObjects) { hive::Table table; table.dbName = database_name; table.tableName = table_name; - table.tableType = "MANAGED_TABLE"; + table.tableType = HmsClient::kManagedTable; hive::FieldSchema partition_key; partition_key.name = "c1"; partition_key.type = "int"; http://git-wip-us.apache.org/repos/asf/kudu/blob/fff4185e/src/kudu/hms/hms_client.cc ---------------------------------------------------------------------- diff --git a/src/kudu/hms/hms_client.cc b/src/kudu/hms/hms_client.cc index 6e2f4c6..1e889ad 100644 --- a/src/kudu/hms/hms_client.cc +++ b/src/kudu/hms/hms_client.cc @@ -25,6 +25,7 @@ #include <utility> #include <vector> +#include <boost/algorithm/string/predicate.hpp> #include <glog/logging.h> #include <thrift/Thrift.h> #include <thrift/protocol/TBinaryProtocol.h> @@ -65,6 +66,7 @@ using std::make_shared; using std::shared_ptr; using std::string; using std::vector; +using strings::Substitute; namespace kudu { namespace hms { @@ -110,11 +112,17 @@ const char* const HmsClient::kKuduStorageHandler = "org.apache.kudu.hive.KuduSto const char* const HmsClient::kTransactionalEventListeners = "hive.metastore.transactional.event.listeners"; +const char* const HmsClient::kDisallowIncompatibleColTypeChanges = + "hive.metastore.disallow.incompatible.col.type.changes"; const char* const HmsClient::kDbNotificationListener = "org.apache.hive.hcatalog.listener.DbNotificationListener"; const char* const HmsClient::kKuduMetastorePlugin = "org.apache.kudu.hive.metastore.KuduMetastorePlugin"; +const char* const HmsClient::kManagedTable = "MANAGED_TABLE"; + +const uint16_t HmsClient::kDefaultHmsPort = 9083; + const int kSlowExecutionWarningThresholdMs = 1000; namespace { @@ -166,7 +174,8 @@ Status HmsClient::Start() { // the required event listeners. string event_listener_config; HMS_RET_NOT_OK(client_.get_config_value(event_listener_config, kTransactionalEventListeners, ""), - "failed to get Hive MetaStore transactional event listener configuration"); + Substitute("failed to get Hive MetaStore $0 configuration", + kTransactionalEventListeners)); // Parse the set of listeners from the configuration string. vector<string> listeners = strings::Split(event_listener_config, ",", strings::SkipWhitespace()); @@ -177,12 +186,35 @@ Status HmsClient::Start() { for (const auto& required_listener : { kDbNotificationListener, kKuduMetastorePlugin }) { if (std::find(listeners.begin(), listeners.end(), required_listener) == listeners.end()) { return Status::IllegalState( - strings::Substitute("Hive Metastore configuration is missing required " - "transactional event listener ($0): $1", - kTransactionalEventListeners, required_listener)); + Substitute("Hive Metastore configuration is missing required " + "transactional event listener ($0): $1", + kTransactionalEventListeners, required_listener)); } } + // Also check that the HMS is configured to allow changing the type of + // columns, which is required to support dropping columns. File-based Hive + // tables handle columns by offset, and removing a column simply shifts all + // remaining columns down by one. The actual data is not changed, but instead + // reassigned to the next column. If the HMS is configured to check column + // type changes it will validate that the existing data matches the shifted + // column layout. This is overly strict for Kudu, since we handle DDL + // operations properly. + // + // See org.apache.hadoop.hive.metastore.MetaStoreUtils.throwExceptionIfIncompatibleColTypeChange. + string disallow_incompatible_column_type_changes; + HMS_RET_NOT_OK(client_.get_config_value(disallow_incompatible_column_type_changes, + kDisallowIncompatibleColTypeChanges, + "false"), + Substitute("failed to get Hive MetaStore $0 configuration", + kDisallowIncompatibleColTypeChanges)); + + if (boost::iequals(disallow_incompatible_column_type_changes, "true")) { + return Status::IllegalState(Substitute( + "Hive Metastore configuration is invalid: $0 must be set to false", + kDisallowIncompatibleColTypeChanges)); + } + return Status::OK(); } @@ -193,6 +225,10 @@ Status HmsClient::Stop() { return Status::OK(); } +bool HmsClient::IsConnected() { + return client_.getInputProtocol()->getTransport()->isOpen(); +} + Status HmsClient::CreateDatabase(const hive::Database& database) { SCOPED_LOG_SLOW_EXECUTION(WARNING, kSlowExecutionWarningThresholdMs, "create HMS database"); HMS_RET_NOT_OK(client_.create_database(database), "failed to create Hive MetaStore database"); http://git-wip-us.apache.org/repos/asf/kudu/blob/fff4185e/src/kudu/hms/hms_client.h ---------------------------------------------------------------------- diff --git a/src/kudu/hms/hms_client.h b/src/kudu/hms/hms_client.h index 21f18f9..cafa417 100644 --- a/src/kudu/hms/hms_client.h +++ b/src/kudu/hms/hms_client.h @@ -23,11 +23,18 @@ #include "kudu/gutil/port.h" #include "kudu/hms/ThriftHiveMetastore.h" -#include "kudu/hms/hive_metastore_types.h" #include "kudu/util/monotime.h" #include "kudu/util/slice.h" #include "kudu/util/status.h" +namespace hive { +class Database; +class EnvironmentContext; +class NotificationEvent; // IWYU pragma: keep +class Partition; // IWYU pragma: keep +class Table; +} + namespace kudu { class HostPort; @@ -85,9 +92,15 @@ class HmsClient { static const char* const kKuduStorageHandler; static const char* const kTransactionalEventListeners; + static const char* const kDisallowIncompatibleColTypeChanges; static const char* const kDbNotificationListener; static const char* const kKuduMetastorePlugin; + // See org.apache.hadoop.hive.metastore.TableType. + static const char* const kManagedTable; + + static const uint16_t kDefaultHmsPort; + // Create an HmsClient connection to the proided HMS Thrift RPC address. // // The individual timeouts may be set to enforce per-operation @@ -109,6 +122,9 @@ class HmsClient { // This is optional; if not called the destructor will stop the client. Status Stop() WARN_UNUSED_RESULT; + // Returns 'true' if the client is connected to the remote server. + bool IsConnected() WARN_UNUSED_RESULT; + // Creates a new database in the HMS. // // If a database already exists by the same name an AlreadyPresent status is http://git-wip-us.apache.org/repos/asf/kudu/blob/fff4185e/src/kudu/hms/mini_hms.cc ---------------------------------------------------------------------- diff --git a/src/kudu/hms/mini_hms.cc b/src/kudu/hms/mini_hms.cc index 078341d..f571b6c 100644 --- a/src/kudu/hms/mini_hms.cc +++ b/src/kudu/hms/mini_hms.cc @@ -181,6 +181,9 @@ Status MiniHms::CreateHiveSite(const string& tmp_dir) const { // - hive.metastore.kerberos.keytab.file // - hive.metastore.kerberos.principal // Configures the HMS to use Kerberos for its Thrift RPC interface. + // + // - hive.metastore.disallow.incompatible.col.type.changes + // Configures the HMS to allow altering and dropping columns. static const string kFileTemplate = R"( <configuration> <property> @@ -208,7 +211,7 @@ Status MiniHms::CreateHiveSite(const string& tmp_dir) const { <property> <name>javax.jdo.option.ConnectionURL</name> - <value>jdbc:derby:memory:$1/metadb;create=true</value> + <value>jdbc:derby:$1/metadb;create=true</value> </property> <property> @@ -235,6 +238,11 @@ Status MiniHms::CreateHiveSite(const string& tmp_dir) const { <name>hadoop.rpc.protection</name> <value>$5</value> </property> + + <property> + <name>hive.metastore.disallow.incompatible.col.type.changes</name> + <value>false</value> + </property> </configuration> )";
