This is an automated email from the ASF dual-hosted git repository. alexey pushed a commit to branch branch-1.17.x in repository https://gitbox.apache.org/repos/asf/kudu.git
commit effa52b1a95f97731313fb1099ae87021c190d5d Author: Marton Greber <greber...@gmail.com> AuthorDate: Wed Feb 15 22:34:15 2023 +0100 KUDU-1945 Auto-incrementing column feature flag This patch adds the flag "--master_support_auto_incrementing_column" to guard the auto-incrementing column feature. It prohibits users to create table with non-unique primary key. The default value for the flag is set to true in this patch. The verification happens on the RPC level. If the server has the feature turned off, or the server is older compared to the client - doesn't have the auto-incrementing feature -, the request gets rejected in both cases. Change-Id: I39c3dde3705c25c36d3ad787c0db6ed03f6c2cfd Reviewed-on: http://gerrit.cloudera.org:8080/19523 Tested-by: Kudu Jenkins Reviewed-by: Wenzhe Zhou <wz...@cloudera.com> Reviewed-by: Alexey Serbin <ale...@apache.org> (cherry picked from commit 7fafdd2803a2c747faa6789a1921d51186edfb6e) Reviewed-on: http://gerrit.cloudera.org:8080/19676 Reviewed-by: Yingchun Lai <laiyingc...@apache.org> --- .../org/apache/kudu/client/CreateTableOptions.java | 4 ++++ .../java/org/apache/kudu/client/TestKuduTable.java | 19 ++++++++++++++++++ src/kudu/client/client-internal.cc | 6 +++++- src/kudu/client/client-internal.h | 3 ++- src/kudu/client/client-test.cc | 23 ++++++++++++++++++++-- src/kudu/client/client.cc | 4 +++- src/kudu/master/master.proto | 2 ++ src/kudu/master/master_service.cc | 8 ++++++++ 8 files changed, 64 insertions(+), 5 deletions(-) diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java b/java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java index 5e3547eea..d577a590f 100644 --- a/java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java +++ b/java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java @@ -337,6 +337,10 @@ public class CreateTableOptions { List<Integer> getRequiredFeatureFlags(Schema schema) { List<Integer> requiredFeatureFlags = new ArrayList<>(); + if (schema.hasAutoIncrementingColumn()) { + requiredFeatureFlags.add( + Integer.valueOf(Master.MasterFeatures.AUTO_INCREMENTING_COLUMN_VALUE)); + } if (schema.hasImmutableColumns()) { requiredFeatureFlags.add( Integer.valueOf(Master.MasterFeatures.IMMUTABLE_COLUMN_ATTRIBUTE_VALUE)); diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java index 6f5469241..035b68a18 100644 --- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java +++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java @@ -24,6 +24,7 @@ import static org.apache.kudu.client.KuduPredicate.ComparisonOp.LESS; import static org.apache.kudu.client.KuduPredicate.ComparisonOp.LESS_EQUAL; import static org.apache.kudu.test.ClientTestUtil.createBasicSchemaInsert; import static org.apache.kudu.test.ClientTestUtil.createSchemaWithImmutableColumns; +import static org.apache.kudu.test.ClientTestUtil.createSchemaWithNonUniqueKey; import static org.apache.kudu.test.ClientTestUtil.getBasicCreateTableOptions; import static org.apache.kudu.test.ClientTestUtil.getBasicSchema; import static org.apache.kudu.test.ClientTestUtil.getBasicTableOptionsWithNonCoveredRange; @@ -2662,4 +2663,22 @@ public class TestKuduTable { "More than one columns are set as auto-incrementing columns")); } } + + @Test(timeout = 100000) + @KuduTestHarness.MasterServerConfig(flags = { + "--master_support_auto_incrementing_column=false" + }) + public void testCreateTableWithAutoIncrementingColWhenMasterNotSupport() throws Exception { + try { + CreateTableOptions builder = getBasicCreateTableOptions(); + client.createTable(tableName, createSchemaWithNonUniqueKey(), builder); + fail("shouldn't be able to create a table with auto-incrementing column " + + "when server side doesn't support required AUTO_INCREMENTING_COLUMN feature"); + } catch (KuduException ex) { + final String errmsg = ex.getMessage(); + assertTrue(errmsg, ex.getStatus().isRemoteError()); + assertTrue(errmsg, errmsg.matches( + ".* server sent error unsupported feature flags")); + } + } } diff --git a/src/kudu/client/client-internal.cc b/src/kudu/client/client-internal.cc index ca6bd53d8..e4faa05db 100644 --- a/src/kudu/client/client-internal.cc +++ b/src/kudu/client/client-internal.cc @@ -319,7 +319,8 @@ Status KuduClient::Data::CreateTable(KuduClient* client, const MonoTime& deadline, bool has_range_partition_bounds, bool has_range_specific_hash_schema, - bool has_immutable_column_schema) { + bool has_immutable_column_schema, + bool has_auto_incrementing_column) { vector<uint32_t> features; if (has_range_partition_bounds) { features.push_back(MasterFeatures::RANGE_PARTITION_BOUNDS); @@ -330,6 +331,9 @@ Status KuduClient::Data::CreateTable(KuduClient* client, if (has_immutable_column_schema) { features.push_back(MasterFeatures::IMMUTABLE_COLUMN_ATTRIBUTE); } + if (has_auto_incrementing_column) { + features.push_back(MasterFeatures::AUTO_INCREMENTING_COLUMN); + } Synchronizer sync; AsyncLeaderMasterRpc<CreateTableRequestPB, CreateTableResponsePB> rpc( deadline, client, BackoffType::EXPONENTIAL, req, resp, diff --git a/src/kudu/client/client-internal.h b/src/kudu/client/client-internal.h index b29b77d91..56c0965c0 100644 --- a/src/kudu/client/client-internal.h +++ b/src/kudu/client/client-internal.h @@ -108,7 +108,8 @@ class KuduClient::Data { const MonoTime& deadline, bool has_range_partition_bounds, bool has_range_specific_hash_schema, - bool has_immutable_column_schema); + bool has_immutable_column_schema, + bool has_auto_incrementing_column); static Status IsCreateTableInProgress(KuduClient* client, master::TableIdentifierPB table, diff --git a/src/kudu/client/client-test.cc b/src/kudu/client/client-test.cc index 104d2a213..5070c67c4 100644 --- a/src/kudu/client/client-test.cc +++ b/src/kudu/client/client-test.cc @@ -147,6 +147,7 @@ DECLARE_bool(fail_dns_resolution); DECLARE_bool(location_mapping_by_uuid); DECLARE_bool(log_inject_latency); DECLARE_bool(master_client_location_assignment_enabled); +DECLARE_bool(master_support_auto_incrementing_column); DECLARE_bool(master_support_connect_to_master_rpc); DECLARE_bool(master_support_immutable_column_attribute); DECLARE_bool(mock_table_metrics_for_testing); @@ -9840,7 +9841,6 @@ TEST_F(ReplicationFactorLimitsTest, MaxReplicationFactor) { class ClientTestAutoIncrementingColumn : public ClientTest { public: void SetUp() override { - // TODO:achennaka Enable Feature flag here once implemented KuduTest::SetUp(); @@ -9912,7 +9912,6 @@ TEST_F(ClientTestAutoIncrementingColumn, ReadAndWrite) { } } - TEST_F(ClientTestAutoIncrementingColumn, ConcurrentWrites) { const string kTableName = "concurrent_writes_auto_incrementing_column"; KuduSchemaBuilder b; @@ -10127,5 +10126,25 @@ TEST_F(ClientTestAutoIncrementingColumn, InsertOperationNegatives) { "with auto-incrementing column"); } } + +TEST_F(ClientTestAutoIncrementingColumn, CreateTableFeatureFlag) { + FLAGS_master_support_auto_incrementing_column = false; + const string kTableName = "create_table_with_auto_incrementing_column_feature_flag"; + KuduSchemaBuilder b; + b.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->NonUniquePrimaryKey(); + ASSERT_OK(b.Build(&schema_)); + + unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator()); + Status s = table_creator->table_name(kTableName) + .schema(&schema_) + .add_hash_partitions({"key"}, 2) + .num_replicas(1) + .Create(); + ASSERT_TRUE(s.IsNotSupported()) << s.ToString(); + ASSERT_STR_CONTAINS( + s.ToString(), + Substitute("Error creating table $0 on the master: cluster does not support " + "CreateTable with feature(s) AUTO_INCREMENTING_COLUMN", kTableName)); +} } // namespace client } // namespace kudu diff --git a/src/kudu/client/client.cc b/src/kudu/client/client.cc index 1d767cf22..4fb407d4c 100644 --- a/src/kudu/client/client.cc +++ b/src/kudu/client/client.cc @@ -1060,6 +1060,7 @@ Status KuduTableCreator::Create() { break; } } + bool has_auto_incrementing_column = data_->schema_->schema_->has_auto_incrementing(); if (data_->table_type_) { req.set_table_type(*data_->table_type_); @@ -1080,7 +1081,8 @@ Status KuduTableCreator::Create() { deadline, !data_->range_partitions_.empty(), has_range_with_custom_hash_schema, - has_immutable_column_schema), + has_immutable_column_schema, + has_auto_incrementing_column), Substitute("Error creating table $0 on the master", data_->table_name_)); // Spin until the table is fully created, if requested. if (data_->wait_) { diff --git a/src/kudu/master/master.proto b/src/kudu/master/master.proto index d527ec11c..0493be6e2 100644 --- a/src/kudu/master/master.proto +++ b/src/kudu/master/master.proto @@ -1176,6 +1176,8 @@ enum MasterFeatures { UPSERT_IGNORE = 9; // Whether master supports immutable attribute on column schema. IMMUTABLE_COLUMN_ATTRIBUTE = 10; + // Whether master supports auto incrementing column. + AUTO_INCREMENTING_COLUMN = 11; } service MasterService { diff --git a/src/kudu/master/master_service.cc b/src/kudu/master/master_service.cc index 234008826..f5944aa3b 100644 --- a/src/kudu/master/master_service.cc +++ b/src/kudu/master/master_service.cc @@ -118,6 +118,12 @@ DEFINE_bool(master_support_immutable_column_attribute, true, TAG_FLAG(master_support_immutable_column_attribute, hidden); TAG_FLAG(master_support_immutable_column_attribute, runtime); +DEFINE_bool(master_support_auto_incrementing_column, true, + "Whether the cluster supports auto-incrementing columns. " + "This in turn enables the usage of non-unique primary key."); +TAG_FLAG(master_support_auto_incrementing_column, unsafe); +TAG_FLAG(master_support_auto_incrementing_column, experimental); +TAG_FLAG(master_support_auto_incrementing_column, runtime); using google::protobuf::Message; using kudu::consensus::ReplicaManagementInfoPB; @@ -956,6 +962,8 @@ bool MasterServiceImpl::SupportsFeature(uint32_t feature) const { return FLAGS_master_support_upsert_ignore_operations; case MasterFeatures::IMMUTABLE_COLUMN_ATTRIBUTE: return FLAGS_master_support_immutable_column_attribute; + case MasterFeatures::AUTO_INCREMENTING_COLUMN: + return FLAGS_master_support_auto_incrementing_column; default: return false; }