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 7b1e17991 Do not enter ALTERING state just for replication factor
changed.
7b1e17991 is described below
commit 7b1e179915a32b7c338025eae6225005befc9f43
Author: 宋家成 <songjiach...@thinkingdata.cn>
AuthorDate: Fri Dec 22 17:46:38 2023 +0800
Do not enter ALTERING state just for replication factor changed.
If we change the replication factor of a table without any tablets,
the table will stay in ALTERING state since exiting this state needs
tablet reports from the tablets of this table.
for now the condition of entering ALTERING state is as below:
has_metadata_changes && (table->num_tablets() >
tablets_to_drop.size() || num_replicas_changed);
Actually even if the number of replicas has been changed, the former
condition is also needed. So rollback the code to:
has_metadata_changes && table->num_tablets() >
tablets_to_drop.size();
Change-Id: I1d20da7c0dd5912790aaa46e9fff366b2973d7a4
Reviewed-on: http://gerrit.cloudera.org:8080/20830
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <ale...@apache.org>
---
src/kudu/integration-tests/alter_table-test.cc | 40 ++++++++++++++++++++++++++
src/kudu/master/catalog_manager.cc | 3 +-
2 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/src/kudu/integration-tests/alter_table-test.cc
b/src/kudu/integration-tests/alter_table-test.cc
index cf75a5fdc..509f954f6 100644
--- a/src/kudu/integration-tests/alter_table-test.cc
+++ b/src/kudu/integration-tests/alter_table-test.cc
@@ -2639,4 +2639,44 @@ TEST_F(AlterTableTest, AddAndRemoveImmutableAttribute) {
EXPECT_EQ("(int32 c0=50331648, int32 c1=3, int32 c2=3)", rows[3]);
}
+TEST_F(ReplicatedAlterTableTest, CheckTableStateAfterReplicatedAlter) {
+ ASSERT_OK(client_->DeleteTable(kTableName));
+ unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator());
+ ASSERT_OK(table_creator->table_name(kTableName)
+ .schema(&schema_)
+ .set_range_partition_columns({ "c0" })
+ .num_replicas(1)
+ .Create());
+
+ // Drop the partition.
+ unique_ptr<KuduTableAlterer>
table_alterer(client_->NewTableAlterer(kTableName));
+ ASSERT_OK(table_alterer->DropRangePartition(schema_.NewRow(),
schema_.NewRow())
+ ->wait(true)->Alter());
+
+ // Check that there are not any tablets in the table.
+ ASSERT_EVENTUALLY([&] {
+ master::GetTableLocationsResponsePB table_locations;
+ ASSERT_OK(itest::GetTableLocations(
+ cluster_->master_proxy(),
+ kTableName,
+ MonoDelta::FromSeconds(30), master::VOTER_REPLICA,
+ /*table_id=*/std::nullopt,
+ &table_locations));
+ ASSERT_EQ(0, table_locations.tablet_locations_size());
+ });
+
+ // Set replication factor to 3.
+ ASSERT_OK(SetReplicationFactor(kTableName, 3));
+ ASSERT_EVENTUALLY([&] {
+ shared_ptr<KuduTable> table;
+ ASSERT_OK(client_->OpenTable(kTableName, &table));
+ ASSERT_EQ(3, table->num_replicas());
+ });
+
+ // The state of the table should not be ALTERING.
+ bool is_altering;
+ ASSERT_OK(client_->IsAlterTableInProgress(kTableName, &is_altering));
+ ASSERT_FALSE(is_altering);
+}
+
} // namespace kudu
diff --git a/src/kudu/master/catalog_manager.cc
b/src/kudu/master/catalog_manager.cc
index 292f39005..01acfcdfe 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -3744,8 +3744,7 @@ Status CatalogManager::AlterTable(const
AlterTableRequestPB& req,
partition_schema_updated;
// Set to true if metadata changes need to be applied to existing tablets.
const bool has_metadata_changes_for_existing_tablets =
- has_metadata_changes &&
- (table->num_tablets() > tablets_to_drop.size() || num_replicas_changed);
+ has_metadata_changes && table->num_tablets() > tablets_to_drop.size();
// Skip empty requests...
if (!has_metadata_changes && !has_partitioning_changes) {
