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 95f8a84e4 [tools] fix backward compatibility in `kudu table delete`
95f8a84e4 is described below
commit 95f8a84e4a3a1c608c15a965450e2dc0a82d29e7
Author: Alexey Serbin <[email protected]>
AuthorDate: Tue Aug 23 15:05:19 2022 -0700
[tools] fix backward compatibility in `kudu table delete`
[1] introduced incompatibility into the `kudu table delete` CLI tool
by setting --reserve_seconds to non-zero by default. With that,
the tool started trying to soft-delete tables by default, which lead
to unexpected failures when HMS integration was enabled.
This patch addresses the issue by changing the default value of
--reserve_seconds to 0. In addition, this patch adds a few test
scenarios to check that dropping a table works as expected when HMS
integration is enabled after the soft-delete functionality for tables
has been introduced.
This is a follow-up to [1].
[1]
https://github.com/apache/kudu/commit/7b6b6b636818d3e22a3939fde77689dce84e88b2
Change-Id: I360d81c2f64ac07cd342ddb882aa72ee50b820bc
Reviewed-on: http://gerrit.cloudera.org:8080/18932
Tested-by: Kudu Jenkins
Reviewed-by: Abhishek Chennaka <[email protected]>
Reviewed-by: Attila Bukor <[email protected]>
---
src/kudu/integration-tests/master_hms-itest.cc | 36 +++++++++++++++++++++++---
src/kudu/tools/kudu-admin-test.cc | 1 +
src/kudu/tools/kudu-tool-test.cc | 25 +++++++++++++++---
src/kudu/tools/tool_action_table.cc | 5 ++--
4 files changed, 58 insertions(+), 9 deletions(-)
diff --git a/src/kudu/integration-tests/master_hms-itest.cc
b/src/kudu/integration-tests/master_hms-itest.cc
index 9b989c879..0dd0459cb 100644
--- a/src/kudu/integration-tests/master_hms-itest.cc
+++ b/src/kudu/integration-tests/master_hms-itest.cc
@@ -40,6 +40,7 @@
#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/integration-tests/hms_itest-base.h"
#include "kudu/mini-cluster/external_mini_cluster.h"
@@ -481,15 +482,18 @@ TEST_F(MasterHmsTest, TestDeleteTable) {
ASSERT_OK(harness_.hms_client()->DropTable("default", "externalTable"));
}
-TEST_F(MasterHmsTest, TestSoftDeleteTable) {
+// Test to verify that the soft-deletion of tables is not supported when
+// HMS is enabled.
+TEST_F(MasterHmsTest, TableSoftDeleteNotSupportedWithHmsEnabled) {
// TODO(kedeng) : change the test case when state sync to HMS
// Create a Kudu table, then soft delete it from Kudu.
ASSERT_OK(CreateKuduTable("default", "a"));
NO_FATALS(CheckTable("default", "a", /*user=*/ nullopt));
hive::Table hms_table;
- // Soft-delete related functions is not supported when HMS is enabled.
- // We set hive_metastore_uris for hack HMS enable.
- FLAGS_hive_metastore_uris = "thrift://127.0.0.1:0";
+ // Set --hive_metastore_uris to make HmsCatalog::IsEnabled() returns 'true'.
+ const auto& hms_uris = cluster_->hms()->uris();
+ ASSERT_TRUE(!hms_uris.empty());
+ FLAGS_hive_metastore_uris = hms_uris;
ASSERT_TRUE(hms::HmsCatalog::IsEnabled());
Status s = client_->SoftDeleteTable("default.a", 6000);
ASSERT_TRUE(s.IsNotSupported()) << s.ToString();
@@ -503,6 +507,30 @@ TEST_F(MasterHmsTest, TestSoftDeleteTable) {
ASSERT_OK(client_->OpenTable("default.a", &table));
}
+// This test makes sure that with the HMS integration enabled, both alter and
+// drop/delete for a table work as expected after the soft-delete feature for
+// tables has been introduced. There might be other scenarios elsewhere testing
+// for that implicitly, but this scenario is run explicitly to check for the
+// backward compatibility in that context.
+TEST_F(MasterHmsTest, AlterAndDeleteTableWhenHmsEnabled) {
+ // Create the database and Kudu table.
+ ASSERT_OK(CreateDatabase("db"));
+ ASSERT_OK(CreateKuduTable("db", "a"));
+ NO_FATALS(CheckTable("db", "a", /*user=*/nullopt));
+
+ const auto& hms_uris = cluster_->hms()->uris();
+ ASSERT_TRUE(!hms_uris.empty());
+ // Set --hive_metastore_uris to make HmsCatalog::IsEnabled() returns 'true'.
+ FLAGS_hive_metastore_uris = hms_uris;
+ ASSERT_TRUE(hms::HmsCatalog::IsEnabled());
+
+ unique_ptr<KuduTableAlterer> table_alterer(client_->NewTableAlterer("db.a"));
+ ASSERT_OK(table_alterer->RenameTo("db.b")->Alter());
+
+ ASSERT_OK(client_->DeleteTable("db.b"));
+ NO_FATALS(CheckTableDoesNotExist("db", "b"));
+}
+
TEST_F(MasterHmsTest, TestNotificationLogListener) {
// Create a Kudu table.
ASSERT_OK(CreateKuduTable("default", "a"));
diff --git a/src/kudu/tools/kudu-admin-test.cc
b/src/kudu/tools/kudu-admin-test.cc
index 803fb62bc..3dec5ebd3 100644
--- a/src/kudu/tools/kudu-admin-test.cc
+++ b/src/kudu/tools/kudu-admin-test.cc
@@ -1695,6 +1695,7 @@ TEST_F(AdminCliTest, TestListTables) {
ASSERT_OK(RunKuduTool({
"table",
"delete",
+ "--reserve_seconds=300",
cluster_->master()->bound_rpc_addr().ToString(),
kTableId
}, &stdout));
diff --git a/src/kudu/tools/kudu-tool-test.cc b/src/kudu/tools/kudu-tool-test.cc
index ba062551b..ab0417ad9 100644
--- a/src/kudu/tools/kudu-tool-test.cc
+++ b/src/kudu/tools/kudu-tool-test.cc
@@ -4624,10 +4624,10 @@ TEST_F(ToolTest, TestRecallTable) {
ASSERT_OK(client->OpenTable(kTableName, &table));
string table_id = table->id();
- // Delete the table.
+ // Soft-delete the table.
string out;
- NO_FATALS(RunActionStdoutNone(Substitute("table delete $0 $1",
- master_addr, kTableName)));
+ NO_FATALS(RunActionStdoutNone(Substitute(
+ "table delete $0 $1 --reserve_seconds=300", master_addr, kTableName)));
// List soft_deleted table.
vector<string> kudu_tables;
@@ -6493,6 +6493,25 @@ TEST_F(ToolTest, TestHmsIgnoresDifferentMasters) {
Substitute("hms check $0 --noignore-other-clusters", master_addrs_str)));
}
+// Make sure that `kudu table delete` works as expected when HMS integration
+// is enabled, keeping the behavior of the tool backward-compatible even after
+// introducing the "table soft-delete" feature.
+TEST_F(ToolTest, DropTableHmsEnabled) {
+ ExternalMiniClusterOptions opts;
+ opts.hms_mode = HmsMode::ENABLE_METASTORE_INTEGRATION;
+ NO_FATALS(StartExternalMiniCluster(std::move(opts)));
+ const auto& master_rpc_addr =
+ HostPort::ToCommaSeparatedString(cluster_->master_rpc_addrs());
+ string out;
+ NO_FATALS(RunActionStdoutString(
+ Substitute("perf loadgen --keep_auto_table $0", master_rpc_addr), &out));
+ NO_FATALS(RunActionStdoutString(
+ Substitute("table list $0", master_rpc_addr), &out));
+ const auto table_name = out;
+ NO_FATALS(RunActionStdoutNone(
+ Substitute("table delete $0 $1", master_rpc_addr, table_name)));
+}
+
TEST_F(ToolTest, TestHmsPrecheck) {
ExternalMiniClusterOptions opts;
opts.hms_mode = HmsMode::ENABLE_HIVE_METASTORE;
diff --git a/src/kudu/tools/tool_action_table.cc
b/src/kudu/tools/tool_action_table.cc
index fa683e8b4..85ff8f417 100644
--- a/src/kudu/tools/tool_action_table.cc
+++ b/src/kudu/tools/tool_action_table.cc
@@ -171,8 +171,9 @@ DEFINE_bool(show_avro_format_schema, false,
"Display the table schema in avro format. When enabled it only
outputs the "
"table schema in Avro format without any other information like "
"partition/owner/comments. It cannot be used in conjunction with
other flags");
-DEFINE_uint32(reserve_seconds, 604800,
- "Reserve seconds before purging a soft-deleted table.");
+DEFINE_uint32(reserve_seconds, 0,
+ "Grace period before purging a soft-deleted table, in seconds. "
+ "If set to 0, table is purged once it dropped/deleted.");
DECLARE_bool(create_table);
DECLARE_bool(fault_tolerant);
