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 76f475aba KUDU-2671 introduce RANGE_SPECIFIC_HASH_SCHEMA feature flag
76f475aba is described below

commit 76f475abad0194464fd2e46383be7467f50aedd3
Author: Alexey Serbin <ale...@apache.org>
AuthorDate: Thu Jun 16 19:15:43 2022 -0700

    KUDU-2671 introduce RANGE_SPECIFIC_HASH_SCHEMA feature flag
    
    This patch introduces a new RANGE_SPECIFIC_HASH_SCHEMA flag for master
    to signal that a Kudu cluster is able to work with tables having
    range-specific hash schemas (a.k.a. custom hash schemas per range).
    
    In addition, now C++ client requires the new flag to be present at
    the server side when creating a table having at least one range
    partition with custom hash schema or when adding a new range partition
    with custom hash schema.
    
    The rationale for introducing the flag is the following: if there were
    no RANGE_SPECIFIC_HASH_SCHEMA flag and a newer client were not requiring
    the server to have such a flag, the client would not get an error while
    trying to perform the following operations against tablet servers
    of prior versions:
      * Creating a table having a range partition with custom hash schema
      * Adding a range partition with custom hash schema to existing table
    That's because the information on custom hash schemas is provided via
    newly added flags in corresponding protobuf structures, and the old
    server would simply ignore the fields, assuming all the ranges to be
    created have the table-wide hash schema.
    
    A follow-up patch will add similar functionality for Kudu Java client.
    
    Change-Id: I256d32003e869939e7aa581b21bbe1e77c1e3aba
    Reviewed-on: http://gerrit.cloudera.org:8080/18633
    Reviewed-by: Mahesh Reddy <mre...@cloudera.com>
    Reviewed-by: Abhishek Chennaka <achenn...@cloudera.com>
    Tested-by: Alexey Serbin <ale...@apache.org>
    Reviewed-by: Attila Bukor <abu...@apache.org>
---
 src/kudu/client/client-internal.cc               |  12 +-
 src/kudu/client/client-internal.h                |   6 +-
 src/kudu/client/client.cc                        |  21 +++-
 src/kudu/client/flex_partitioning_client-test.cc | 138 +++++++++++++++++++++++
 src/kudu/client/table_alterer-internal.h         |   3 +
 src/kudu/master/master.proto                     |   2 +
 src/kudu/master/master_service.cc                |   3 +
 7 files changed, 175 insertions(+), 10 deletions(-)

diff --git a/src/kudu/client/client-internal.cc 
b/src/kudu/client/client-internal.cc
index 99934e28f..5c650599f 100644
--- a/src/kudu/client/client-internal.cc
+++ b/src/kudu/client/client-internal.cc
@@ -316,11 +316,15 @@ Status KuduClient::Data::CreateTable(KuduClient* client,
                                      const CreateTableRequestPB& req,
                                      CreateTableResponsePB* resp,
                                      const MonoTime& deadline,
-                                     bool has_range_partition_bounds) {
+                                     bool has_range_partition_bounds,
+                                     bool has_range_specific_hash_schema) {
   vector<uint32_t> features;
   if (has_range_partition_bounds) {
     features.push_back(MasterFeatures::RANGE_PARTITION_BOUNDS);
   }
+  if (has_range_specific_hash_schema) {
+    features.push_back(MasterFeatures::RANGE_SPECIFIC_HASH_SCHEMA);
+  }
   Synchronizer sync;
   AsyncLeaderMasterRpc<CreateTableRequestPB, CreateTableResponsePB> rpc(
       deadline, client, BackoffType::EXPONENTIAL, req, resp,
@@ -387,11 +391,15 @@ Status KuduClient::Data::AlterTable(KuduClient* client,
                                     const AlterTableRequestPB& req,
                                     AlterTableResponsePB* resp,
                                     const MonoTime& deadline,
-                                    bool has_add_drop_partition) {
+                                    bool has_add_drop_partition,
+                                    bool adding_range_with_custom_hash_schema) 
{
   vector<uint32_t> required_feature_flags;
   if (has_add_drop_partition) {
     
required_feature_flags.push_back(MasterFeatures::ADD_DROP_RANGE_PARTITIONS);
   }
+  if (adding_range_with_custom_hash_schema) {
+    
required_feature_flags.push_back(MasterFeatures::RANGE_SPECIFIC_HASH_SCHEMA);
+  }
   Synchronizer sync;
   AsyncLeaderMasterRpc<AlterTableRequestPB, AlterTableResponsePB> rpc(
       deadline, client, BackoffType::EXPONENTIAL, req, resp,
diff --git a/src/kudu/client/client-internal.h 
b/src/kudu/client/client-internal.h
index deab47d42..61661f73b 100644
--- a/src/kudu/client/client-internal.h
+++ b/src/kudu/client/client-internal.h
@@ -105,7 +105,8 @@ class KuduClient::Data {
                             const master::CreateTableRequestPB& req,
                             master::CreateTableResponsePB* resp,
                             const MonoTime& deadline,
-                            bool has_range_partition_bounds);
+                            bool has_range_partition_bounds,
+                            bool has_range_specific_hash_schema);
 
   static Status IsCreateTableInProgress(KuduClient* client,
                                         master::TableIdentifierPB table,
@@ -125,7 +126,8 @@ class KuduClient::Data {
                            const master::AlterTableRequestPB& req,
                            master::AlterTableResponsePB* resp,
                            const MonoTime& deadline,
-                           bool has_add_drop_partition);
+                           bool has_add_drop_partition,
+                           bool adding_range_with_custom_hash_schema);
 
   static Status IsAlterTableInProgress(KuduClient* client,
                                        master::TableIdentifierPB table,
diff --git a/src/kudu/client/client.cc b/src/kudu/client/client.cc
index 1f3b86c56..46b9a53da 100644
--- a/src/kudu/client/client.cc
+++ b/src/kudu/client/client.cc
@@ -1029,10 +1029,14 @@ Status KuduTableCreator::Create() {
   }
 
   CreateTableResponsePB resp;
-  RETURN_NOT_OK_PREPEND(data_->client_->data_->CreateTable(
-      data_->client_, req, &resp, deadline, !data_->range_partitions_.empty()),
-                        Substitute("Error creating table $0 on the master",
-                                   data_->table_name_));
+  RETURN_NOT_OK_PREPEND(
+      data_->client_->data_->CreateTable(data_->client_,
+                                         req,
+                                         &resp,
+                                         deadline,
+                                         !data_->range_partitions_.empty(),
+                                         has_range_with_custom_hash_schema),
+      Substitute("Error creating table $0 on the master", data_->table_name_));
   // Spin until the table is fully created, if requested.
   if (data_->wait_) {
     TableIdentifierPB table;
@@ -1572,6 +1576,9 @@ KuduTableAlterer* KuduTableAlterer::AddRangePartition(
                  nullopt };
   data_->steps_.emplace_back(std::move(s));
   data_->has_alter_partitioning_steps = true;
+  if 
(!data_->steps_.back().range_partition->data_->is_table_wide_hash_schema_) {
+    data_->adding_range_with_custom_hash_schema = true;
+  }
   return this;
 }
 
@@ -1645,8 +1652,10 @@ Status KuduTableAlterer::Alter() {
     data_->timeout_ :
     data_->client_->default_admin_operation_timeout();
   MonoTime deadline = MonoTime::Now() + timeout;
-  RETURN_NOT_OK(data_->client_->data_->AlterTable(data_->client_, req, &resp, 
deadline,
-                                                  
data_->has_alter_partitioning_steps));
+  RETURN_NOT_OK(data_->client_->data_->AlterTable(
+      data_->client_, req, &resp, deadline,
+      data_->has_alter_partitioning_steps,
+      data_->adding_range_with_custom_hash_schema));
 
   if (data_->has_alter_partitioning_steps) {
     // If the table partitions change, clear the local meta cache so that the
diff --git a/src/kudu/client/flex_partitioning_client-test.cc 
b/src/kudu/client/flex_partitioning_client-test.cc
index 3ce4f0273..97c4a5375 100644
--- a/src/kudu/client/flex_partitioning_client-test.cc
+++ b/src/kudu/client/flex_partitioning_client-test.cc
@@ -1063,6 +1063,43 @@ TEST_F(FlexPartitioningCreateTableTest, Negatives) {
   }
 }
 
+// When working with a cluster that doesn't support range-specific hash schemas
+// for tables, the client should receive proper error while trying to create
+// a table with custom hash schema for at least one of its ranges.
+TEST_F(FlexPartitioningCreateTableTest, UnsupportedRangeSpecificHashSchema) {
+  // Turn off the support for range-specific hash schemas, emulating the
+  // situation when a Kudu cluster is running an older version released prior
+  // to the introduction of the feature.
+  FLAGS_enable_per_range_hash_schemas = false;
+
+  constexpr const char* const kTableName =
+      "UnsupportedRangeSpecificHashSchemaCreateTable";
+
+  unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator());
+  table_creator->table_name(kTableName)
+      .schema(&schema_)
+      .num_replicas(1)
+      .add_hash_partitions({ kKeyColumn }, 3)
+      .set_range_partition_columns({ kKeyColumn });
+
+  // Add a range partition with the table-wide hash partitioning rules.
+  unique_ptr<KuduPartialRow> lower(schema_.NewRow());
+  ASSERT_OK(lower->SetInt32(kKeyColumn, 0));
+  unique_ptr<KuduPartialRow> upper(schema_.NewRow());
+  ASSERT_OK(upper->SetInt32(kKeyColumn, 111));
+  table_creator->add_range_partition(lower.release(), upper.release());
+
+  // Add a range partition with custom hash schema.
+  auto p = CreateRangePartition(111, 222);
+  ASSERT_OK(p->add_hash_partitions({ kKeyColumn }, 5, 0));
+  table_creator->add_custom_range_partition(p.release());
+
+  const auto s = table_creator->Create();
+  ASSERT_TRUE(s.IsNotSupported()) << s.ToString();
+  ASSERT_STR_CONTAINS(s.ToString(), "cluster does not support CreateTable with 
"
+                                    "feature(s) RANGE_SPECIFIC_HASH_SCHEMA");
+}
+
 // Test for scenarios covering range partitioning with custom hash schemas
 // specified when adding a new custom hash schema partition to a table.
 class FlexPartitioningAlterTableTest : public FlexPartitioningTest {};
@@ -1241,5 +1278,106 @@ TEST_F(FlexPartitioningAlterTableTest, 
ReadAndWriteToCustomRangePartition) {
     }
   }
 }
+
+// When working with a cluster that doesn't support range-specific hash schemas
+// for tables, the client should receive proper error while trying to add
+// a range with custom hash schema.
+TEST_F(FlexPartitioningAlterTableTest, UnsupportedRangeSpecificHashSchema) {
+  // Turn off the support for range-specific hash schemas, emulating the
+  // situation when a Kudu cluster is running an older version released prior
+  // to the introduction of the feature.
+  FLAGS_enable_per_range_hash_schemas = false;
+
+  constexpr const char* const kTableName =
+      "UnsupportedRangeSpecificHashSchemaAlterTable";
+  constexpr const char* const kErrMsg = "cluster does not support AlterTable "
+      "with feature(s) RANGE_SPECIFIC_HASH_SCHEMA";
+
+  unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator());
+  table_creator->table_name(kTableName)
+      .schema(&schema_)
+      .num_replicas(1)
+      .add_hash_partitions({ kKeyColumn }, 2)
+      .set_range_partition_columns({ kKeyColumn });
+
+  // Add a range partition with the table-wide hash partitioning rules.
+  unique_ptr<KuduPartialRow> lower(schema_.NewRow());
+  ASSERT_OK(lower->SetInt32(kKeyColumn, 0));
+  unique_ptr<KuduPartialRow> upper(schema_.NewRow());
+  ASSERT_OK(upper->SetInt32(kKeyColumn, 111));
+  table_creator->add_range_partition(lower.release(), upper.release());
+
+  ASSERT_OK(table_creator->Create());
+
+  // Try to add a single range with custom hash schema.
+  {
+    unique_ptr<KuduTableAlterer> alterer(client_->NewTableAlterer(kTableName));
+    auto p = CreateRangePartition(111, 222);
+    ASSERT_OK(p->add_hash_partitions({ kKeyColumn }, 5, 0));
+    alterer->AddRangePartition(p.release());
+
+    const auto s = alterer->Alter();
+    ASSERT_TRUE(s.IsNotSupported()) << s.ToString();
+    ASSERT_STR_CONTAINS(s.ToString(), kErrMsg);
+  }
+
+  // Try to add a mix of ranges: with the table-wide and custom hash schemas.
+  {
+    unique_ptr<KuduTableAlterer> alterer(client_->NewTableAlterer(kTableName));
+
+    unique_ptr<KuduPartialRow> lower(schema_.NewRow());
+    ASSERT_OK(lower->SetInt32(kKeyColumn, 111));
+    unique_ptr<KuduPartialRow> upper(schema_.NewRow());
+    ASSERT_OK(upper->SetInt32(kKeyColumn, 222));
+    alterer->AddRangePartition(lower.release(), upper.release());
+
+    auto p = CreateRangePartition(222, 333);
+    ASSERT_OK(p->add_hash_partitions({ kKeyColumn }, 8, 0));
+    alterer->AddRangePartition(p.release());
+
+    const auto s = alterer->Alter();
+    ASSERT_TRUE(s.IsNotSupported()) << s.ToString();
+    ASSERT_STR_CONTAINS(s.ToString(), kErrMsg);
+  }
+
+  // Temporary enable support for the RANGE_SPECIFIC_HASH_SCHEMA feature to add
+  // a new range with custom hash schema to be dropped later.
+  FLAGS_enable_per_range_hash_schemas = true;
+  {
+    unique_ptr<KuduTableAlterer> alterer(client_->NewTableAlterer(kTableName));
+
+    auto p0 = CreateRangePartition(111, 222);
+    ASSERT_OK(p0->add_hash_partitions({ kKeyColumn }, 5, 0));
+    alterer->AddRangePartition(p0.release());
+
+    auto p1 = CreateRangePartition(222, 333);
+    ASSERT_OK(p1->add_hash_partitions({ kKeyColumn }, 8, 0));
+    alterer->AddRangePartition(p1.release());
+
+    ASSERT_OK(alterer->Alter());
+  }
+  // Disable the support for the RANGE_SPECIFIC_HASH_SCHEMA.
+  FLAGS_enable_per_range_hash_schemas = false;
+
+  // Dropping ranges with the table-wide or custom hash schemas should be fine
+  // even if the cluster doesn't support the RANGE_SPECIFIC_HASH_SCHEMA schema
+  // It's rather a hypothetical situation unless they toggle the flag after
+  // adding ranges with custom hash schemas or running older Kudu binaries with
+  // new data.
+  {
+    unique_ptr<KuduTableAlterer> alterer(client_->NewTableAlterer(kTableName));
+    unique_ptr<KuduPartialRow> lower(schema_.NewRow());
+    ASSERT_OK(lower->SetInt32(kKeyColumn, 111));
+    unique_ptr<KuduPartialRow> upper(schema_.NewRow());
+    ASSERT_OK(upper->SetInt32(kKeyColumn, 222));
+    alterer->DropRangePartition(lower.release(), upper.release());
+    ASSERT_OK(alterer->Alter());
+  }
+
+  // Dropping tables having ranges with custom hash schema should be fine
+  // even if the server side has no RANGE_SPECIFIC_HASH_SCHEMA feature.
+  ASSERT_OK(client_->DeleteTable(kTableName));
+}
+
 } // namespace client
 } // namespace kudu
diff --git a/src/kudu/client/table_alterer-internal.h 
b/src/kudu/client/table_alterer-internal.h
index 19b43493f..c9ce547d2 100644
--- a/src/kudu/client/table_alterer-internal.h
+++ b/src/kudu/client/table_alterer-internal.h
@@ -81,6 +81,9 @@ class KuduTableAlterer::Data {
   // Set to true if there are alter partition steps.
   bool has_alter_partitioning_steps = false;
 
+  // Set to true if a new range with custom hash schema is being added.
+  bool adding_range_with_custom_hash_schema = false;
+
   // Schema of add/drop range partition bound rows.
   const Schema* schema_;
 
diff --git a/src/kudu/master/master.proto b/src/kudu/master/master.proto
index 48b004721..afab96ef2 100644
--- a/src/kudu/master/master.proto
+++ b/src/kudu/master/master.proto
@@ -1116,6 +1116,8 @@ enum MasterFeatures {
   // Though this is technically a tserver feature, it's unreasonable to check 
if every
   // tablet server supports this feature. Instead we use the master as a proxy.
   IGNORE_OPERATIONS = 7;
+  // Whether master supports tables with range-specific hash schemas.
+  RANGE_SPECIFIC_HASH_SCHEMA = 8;
 }
 
 service MasterService {
diff --git a/src/kudu/master/master_service.cc 
b/src/kudu/master/master_service.cc
index 07f42a8b1..d7c3f6ca8 100644
--- a/src/kudu/master/master_service.cc
+++ b/src/kudu/master/master_service.cc
@@ -61,6 +61,7 @@
 #include "kudu/util/scoped_cleanup.h"
 #include "kudu/util/status.h"
 
+DECLARE_bool(enable_per_range_hash_schemas);
 DECLARE_bool(hive_metastore_sasl_enabled);
 DECLARE_bool(raft_prepare_replacement_before_eviction);
 DECLARE_string(hive_metastore_uris);
@@ -883,6 +884,8 @@ bool MasterServiceImpl::SupportsFeature(uint32_t feature) 
const {
       return FLAGS_master_support_change_config;
     case MasterFeatures::IGNORE_OPERATIONS:
       return FLAGS_master_support_ignore_operations;
+    case MasterFeatures::RANGE_SPECIFIC_HASH_SCHEMA:
+      return FLAGS_enable_per_range_hash_schemas;
     default:
       return false;
   }

Reply via email to