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 6998193 KUDU-2671 number of per-range hash dimensions should be fixed
for now
6998193 is described below
commit 6998193e69eeda497f912d1d806470c95b591ad4
Author: Alexey Serbin <[email protected]>
AuthorDate: Thu Nov 18 12:01:43 2021 -0800
KUDU-2671 number of per-range hash dimensions should be fixed for now
As it turned out, updating the client's metacache, the system catalog's
logic, and the partition pruner to accommodate for partition keys with
variable size of the hash part seems to be a substantial effort on
itself. However, we can still deliver the most frequently requested
functionality of changing the number of hash buckets per range partition
if adding the restriction on the size of the hash part of a partition
key, requiring it to be of the same size across all the range partitions
in a table.
So, this patch adds a new restriction for per-range custom schemas:
the number of hash dimensions must be the same for all the ranges in
a table. Since the absence of hash bucketing is equivalent to having
zero hash dimensions for a table's range, that means it's not possible
to make a particular range having no hash bucketing at all if the rest
of the ranges in the table have non-trivial hash schemas.
With the introduced restriction, it's still possible to change other
parameters used to define a hash schema per range in any hash dimension:
* the number of hash buckets (NOTE: the number of hash buckets must be
equal or greater than two)
* the set of columns for the hash bucketing
* the seed for the hash function
As a part of this changelist, a few test scenarios are now disabled:
those are to be re-enabled once the rest of the code in the system
catalog, the client metacache, and the partition pruner is able to
handle different number of hash dimensions. In addition, new test
scenarios have been added to verify that the invariant of the same
number of hash dimensions across all the range partition is properly
enforced at the server side while creating a table.
Also, I updated the comparison operator for PartitionKey: since the
number of hash dimensions isn't varying across per-range hash schemas,
it's no longer necessary to concatenate the hash and the range parts to
provide the legacy ordering of partition keys for some edge cases which
were pertinent to situations with varying number of hash dimensions for
the hash schema of a range. The implementation of the PartitionKey's
comparison operator might change if switching to a single string under
the hood, but at this point I decided to keep them separate. For the
sake of keeping the code future-proof and easier for review, I think
of starting using strings views (or Slice) for the range_key() and the
hash_key() methods in a follow-up changelist, regardless of how the
serialized partition key is represented under the hood in PartitionKey.
Change-Id: Ic884fa556462b85c64d77385a521d9077d33c7c1
Reviewed-on: http://gerrit.cloudera.org:8080/18045
Tested-by: Alexey Serbin <[email protected]>
Reviewed-by: Andrew Wong <[email protected]>
---
src/kudu/client/flex_partitioning_client-test.cc | 332 +++++++++++++++++++--
src/kudu/common/partition.h | 8 +-
src/kudu/common/partition_pruner-test.cc | 8 +-
.../integration-tests/table_locations-itest.cc | 137 ++++++---
src/kudu/master/catalog_manager.cc | 18 +-
5 files changed, 423 insertions(+), 80 deletions(-)
diff --git a/src/kudu/client/flex_partitioning_client-test.cc
b/src/kudu/client/flex_partitioning_client-test.cc
index b4967f0..e8e97e4 100644
--- a/src/kudu/client/flex_partitioning_client-test.cc
+++ b/src/kudu/client/flex_partitioning_client-test.cc
@@ -17,6 +17,8 @@
#include <cstdint>
#include <cstdlib>
+#include <ios>
+#include <iostream>
#include <memory>
#include <string>
#include <utility>
@@ -34,7 +36,6 @@
#include "kudu/gutil/port.h"
#include "kudu/gutil/ref_counted.h"
#include "kudu/gutil/stl_util.h"
-#include "kudu/gutil/strings/substitute.h"
#include "kudu/master/catalog_manager.h"
#include "kudu/master/master.h"
#include "kudu/master/mini_master.h"
@@ -60,27 +61,26 @@ using kudu::tablet::TabletReplica;
using std::string;
using std::unique_ptr;
using std::vector;
-using strings::Substitute;
+
+static constexpr const char* const kKeyColumn = "key";
+static constexpr const char* const kIntValColumn = "int_val";
+static constexpr const char* const kStringValColumn = "string_val";
namespace kudu {
namespace client {
class FlexPartitioningTest : public KuduTest {
public:
- static constexpr const char* const kKeyColumn = "key";
- static constexpr const char* const kIntValColumn = "int_val";
- static constexpr const char* const kStringValColumn = "string_val";
-
FlexPartitioningTest() {
KuduSchemaBuilder b;
- b.AddColumn("key")->
+ b.AddColumn(kKeyColumn)->
Type(KuduColumnSchema::INT32)->
NotNull()->
PrimaryKey();
- b.AddColumn("int_val")->
+ b.AddColumn(kIntValColumn)->
Type(KuduColumnSchema::INT32)->
NotNull();
- b.AddColumn("string_val")->
+ b.AddColumn(kStringValColumn)->
Type(KuduColumnSchema::STRING)->
Nullable();
CHECK_OK(b.Build(&schema_));
@@ -359,7 +359,9 @@ TEST_F(FlexPartitioningCreateTableTest,
EmptyTableWideHashSchema) {
//NO_FATALS(CheckTableRowsNum(kTableName, 333));
}
-TEST_F(FlexPartitioningCreateTableTest, SingleCustomRangeEmptyHashSchema) {
+// TODO(aserbin): re-enable this scenario once varying hash dimensions per
range
+// are supported
+TEST_F(FlexPartitioningCreateTableTest,
DISABLED_SingleCustomRangeEmptyHashSchema) {
// Create a table with the following partitions:
//
// hash bucket
@@ -413,7 +415,7 @@ TEST_F(FlexPartitioningCreateTableTest,
SingleCustomRangeEmptyHashSchema) {
//
// TODO(aserbin): add verification based on PartitionSchema provided by
// KuduTable::partition_schema() once PartitionPruner
-// recognized custom hash bucket schema for ranges
+// recognizes custom hash bucket schema per range
TEST_F(FlexPartitioningCreateTableTest, DefaultAndCustomHashSchemas) {
// Create a table with the following partitions:
//
@@ -424,7 +426,6 @@ TEST_F(FlexPartitioningCreateTableTest,
DefaultAndCustomHashSchemas) {
// 111-222 x:{key} x:{key} x:{key} -
// 222-333 x:{key} x:{key} x:{key} x:{key}
// 333-444 x:{key} x:{key} - -
- // 444-555 - - - -
constexpr const char* const kTableName = "DefaultAndCustomHashSchemas";
unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator());
@@ -467,29 +468,16 @@ TEST_F(FlexPartitioningCreateTableTest,
DefaultAndCustomHashSchemas) {
table_creator->add_custom_range_partition(p.release());
}
- // Add a range partition with no hash bucketing. The table-wide hash schema
- // established by the KuduTableCreator::add_hash_partition() call in the
- // beginning of the scenario defines hash bucketing for ranges added by the
- // KuduTableCreator::add_range_partition() method, but here the newly created
- // range is added via the KuduTableCreator::add_custom_range_partition()
call,
- // so the new range has custom hash schema. Not calling
- // KuduRangePartition::add_hash_partitions() on the newly created range means
- // the range doesn't have any hash bucketing.
- {
- auto p = CreateRangePartition(444, 555);
- table_creator->add_custom_range_partition(p.release());
- }
-
ASSERT_OK(table_creator->Create());
- NO_FATALS(CheckTabletCount(kTableName, 12));
+ NO_FATALS(CheckTabletCount(kTableName, 11));
// Make sure it's possible to insert rows into the table for all the existing
// the partitions: first check the range of table-wide schema, then check
// the ranges with custom hash schemas.
ASSERT_OK(InsertTestRows(kTableName, -111, 0));
NO_FATALS(CheckLiveRowCount(kTableName, 111));
- ASSERT_OK(InsertTestRows(kTableName, 111, 555));
- NO_FATALS(CheckLiveRowCount(kTableName, 555));
+ ASSERT_OK(InsertTestRows(kTableName, 111, 444));
+ NO_FATALS(CheckLiveRowCount(kTableName, 444));
// Meanwhile, inserting into non-covered ranges should result in a proper
// error status return to the client attempting such an operation.
@@ -497,7 +485,7 @@ TEST_F(FlexPartitioningCreateTableTest,
DefaultAndCustomHashSchemas) {
vector<KuduError*> errors;
ElementDeleter drop(&errors);
auto s = InsertTestRows(
- kTableName, 555, 556, KuduSession::AUTO_FLUSH_SYNC, &errors);
+ kTableName, 444, 445, KuduSession::AUTO_FLUSH_SYNC, &errors);
ASSERT_TRUE(s.IsIOError()) << s.ToString();
ASSERT_STR_CONTAINS(s.ToString(), "failed to flush data");
ASSERT_EQ(1, errors.size());
@@ -513,7 +501,7 @@ TEST_F(FlexPartitioningCreateTableTest,
DefaultAndCustomHashSchemas) {
vector<KuduError*> errors;
ElementDeleter drop(&errors);
auto s = InsertTestRows(
- kTableName, 556, 556 + kNumRows, KuduSession::MANUAL_FLUSH, &errors);
+ kTableName, 445, 445 + kNumRows, KuduSession::MANUAL_FLUSH, &errors);
ASSERT_TRUE(s.IsIOError()) << s.ToString();
ASSERT_STR_CONTAINS(s.ToString(), "failed to flush data");
ASSERT_EQ(kNumRows, errors.size());
@@ -526,10 +514,292 @@ TEST_F(FlexPartitioningCreateTableTest,
DefaultAndCustomHashSchemas) {
}
}
+// Parameters for a single hash dimension.
+struct HashDimensionParameters {
+ vector<string> columns; // names of the columns to use for hash bucketing
+ int32_t num_buckets; // number of hash buckets
+};
+std::ostream& operator<<(std::ostream& os,
+ const HashDimensionParameters& params) {
+ os << "{columns:{";
+ for (const auto& c : params.columns) {
+ os << c << ",";
+ }
+ os << "},";
+ os << "num_buckets:" << params.num_buckets << ",";
+ return os;
+}
+
+// Hash bucketing parameters for a range partition.
+typedef vector<HashDimensionParameters> HashPartitionParameters;
+std::ostream& operator<<(std::ostream& os,
+ const HashPartitionParameters& params) {
+ os << "{";
+ for (const auto& e : params) {
+ os << e << ",";
+ }
+ os << "}";
+ return os;
+}
+
+// Range partition bounds.
+struct RangeBounds {
+ int32_t lower; // inclusive lower bound
+ int32_t upper; // exclusive upper bound
+};
+
+// The whole set of parameters per range.
+struct PerRangeParameters {
+ RangeBounds bounds;
+ HashPartitionParameters hash_params;
+};
+std::ostream& operator<<(std::ostream& os,
+ const PerRangeParameters& params) {
+ os << "{bounds:["
+ << params.bounds.lower << ","
+ << params.bounds.upper << "),{";
+ for (const auto& hp : params.hash_params) {
+ os << hp << ",";
+ }
+ os << "}}";
+ return os;
+}
+
+typedef vector<PerRangeParameters> TablePartitionParameters;
+std::ostream& operator<<(std::ostream& os,
+ const TablePartitionParameters& params) {
+ os << "{";
+ for (const auto& per_range_params : params) {
+ os << per_range_params;
+ }
+ os << "}";
+ return os;
+}
+
+struct PerRangeTestParameters {
+ bool allowed;
+ TablePartitionParameters partition_params;
+};
+std::ostream& operator<<(std::ostream& os,
+ const PerRangeTestParameters& params) {
+ os << "allowed:" << std::boolalpha << params.allowed
+ << " partition_params:" << params.partition_params;
+ return os;
+}
+
+class FlexPartitioningCreateTableParamTest :
+ public FlexPartitioningCreateTableTest,
+ public testing::WithParamInterface<PerRangeTestParameters> {
+ public:
+ FlexPartitioningCreateTableParamTest() {
+ KuduSchemaBuilder b;
+ b.AddColumn(kKeyColumn)->Type(KuduColumnSchema::INT32)->NotNull();
+ b.AddColumn(kIntValColumn)->Type(KuduColumnSchema::INT32)->NotNull();
+ b.AddColumn(kStringValColumn)->Type(KuduColumnSchema::STRING);
+ b.SetPrimaryKey({ kKeyColumn, kIntValColumn });
+ CHECK_OK(b.Build(&schema_));
+ }
+
+ void Run() {
+ constexpr const char* const kTableName = "PerRangeHashSchemaVariations";
+ unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator());
+ table_creator->table_name(kTableName)
+ .schema(&schema_)
+ .num_replicas(1)
+ .set_range_partition_columns({ kKeyColumn });
+
+ const PerRangeTestParameters& params = GetParam();
+ for (const auto& per_range_params : params.partition_params) {
+ auto p = CreateRangePartition(per_range_params.bounds.lower,
+ per_range_params.bounds.upper);
+ const auto& hash_params = per_range_params.hash_params;
+ for (const auto& hd : hash_params) {
+ ASSERT_OK(p->add_hash_partitions(hd.columns, hd.num_buckets, 0));
+ }
+ table_creator->add_custom_range_partition(p.release());
+ }
+
+ if (params.allowed) {
+ ASSERT_OK(table_creator->Create());
+ } else {
+ const auto s = table_creator->Create();
+ ASSERT_TRUE(s.IsNotSupported()) << s.ToString();
+ ASSERT_STR_CONTAINS(s.ToString(),
+ "varying number of hash dimensions per range is not yet supported");
+ }
+ }
+};
+
+const vector<PerRangeTestParameters> kPerRangeTestParameters = {
+ // A single range with no hashing at all.
+ {
+ true,
+ {
+ { { INT32_MIN, 0 }, {} },
+ }
+ },
+ // A single range with minimal hash schema.
+ {
+ true,
+ {
+ { { INT32_MIN, 0 }, { { { kKeyColumn }, 2, } } },
+ }
+ },
+ // A single range with hashing on both columns of the primary key.
+ {
+ true,
+ {
+ { { INT32_MIN, 0 }, { { { kKeyColumn, kIntValColumn }, 2, } } },
+ }
+ },
+ // Varying the number of hash buckets.
+ {
+ true,
+ {
+ { { 0, 100 }, { { { kKeyColumn }, 2, } } },
+ { { 100, 200 }, { { { kKeyColumn }, 3, } } },
+ { { 200, 300 }, { { { kKeyColumn }, 2, } } },
+ { { 300, 400 }, { { { kKeyColumn }, 10, } } },
+ }
+ },
+ // Varying the set of columns for hash bucketing.
+ {
+ true,
+ {
+ { { 100, 200 }, { { { kKeyColumn }, 2, } } },
+ { { 200, 300 }, { { { kKeyColumn, kIntValColumn }, 2, } } },
+ { { 300, 400 }, { { { kIntValColumn }, 2, } } },
+ { { 400, 500 }, { { { kIntValColumn, kKeyColumn }, 2, } } },
+ }
+ },
+ // Varying the set of columns for hash bucketing and number of buckets.
+ {
+ true,
+ {
+ { { 200, 300 }, { { { kKeyColumn }, 10, } } },
+ { { 300, 400 }, { { { kKeyColumn, kIntValColumn }, 2, } } },
+ { { 400, 500 }, { { { kKeyColumn, kIntValColumn }, 5, } } },
+ { { 500, 600 }, { { { kIntValColumn }, 8, } } },
+ { { 600, 700 }, { { { kKeyColumn }, 3, } } },
+ { { 700, 800 }, { { { kIntValColumn, kKeyColumn }, 16, } } },
+ { { 800, 900 }, { { { kIntValColumn }, 32, } } },
+ }
+ },
+ // Below are multiple scenarios with varying number of hash dimensions
+ // for hash bucketing.
+ {
+ false,
+ {
+ { { INT32_MIN, 0 }, {} },
+ { { 0, 100 }, { { { kKeyColumn }, 2 } } },
+ }
+ },
+ {
+ false,
+ {
+ { { INT32_MIN, 0 }, { { { kKeyColumn }, 3 } } },
+ { { 0, 100 }, {} },
+ }
+ },
+ {
+ false,
+ {
+ { { INT32_MIN, 0 }, {} },
+ { { 0, 100 }, { { { kKeyColumn }, 2 } } },
+ { { 100, 200 }, {} },
+ }
+ },
+ {
+ false,
+ {
+ { { INT32_MIN, 0 }, { { { kKeyColumn }, 3 } } },
+ { { 0, 100 }, {} },
+ { { 100, 200 }, { { { kIntValColumn }, 2 } } },
+ }
+ },
+ {
+ false,
+ {
+ { { 0, 100 }, { { { kKeyColumn }, 2 }, { { kIntValColumn }, 2 } } },
+ { { 100, 200 }, { { { kKeyColumn, kIntValColumn }, 2 } } },
+ }
+ },
+ {
+ false,
+ {
+ { { 100, 200 }, { { { kKeyColumn }, 2 }, { { kIntValColumn }, 3 } } },
+ { { 200, 300 }, { { { kKeyColumn }, 3 } } },
+ }
+ },
+ {
+ false,
+ {
+ { { 300, 400 }, { { { kKeyColumn }, 2 }, { { kIntValColumn }, 3 } } },
+ { { 400, 500 }, { { { kKeyColumn }, 3 } } },
+ }
+ },
+ {
+ false,
+ {
+ { { 500, 600 }, { { { kKeyColumn }, 10 }, { { kIntValColumn }, 5 } } },
+ { { 600, 700 }, { { { kKeyColumn }, 2 } } },
+ { { 700, INT32_MAX }, {} },
+ }
+ },
+};
+INSTANTIATE_TEST_SUITE_P(Variations, FlexPartitioningCreateTableParamTest,
+ testing::ValuesIn(kPerRangeTestParameters));
+TEST_P(FlexPartitioningCreateTableParamTest, PerRangeHashSchemaVariations) {
+ NO_FATALS(Run());
+}
+
+TEST_F(FlexPartitioningCreateTableTest,
CustomHashSchemasVaryingDimensionsNumber) {
+ constexpr const char* const kTableName =
"CustomHashSchemasVaryingDimensions";
+
+ // Try creating a table with the following partitions:
+ //
+ // hash bucket
+ // key 0 1
+ // -------------------------
+ // <111 x:{key} x:{key}
+ // 111-222 - -
+ 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, INT32_MIN));
+ 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 no hash bucketing. Not calling
+ // KuduRangePartition::add_hash_partitions() on the newly created range
+ // means the range doesn't have any hash bucketing.
+ {
+ auto p = CreateRangePartition(111, 222);
+ 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(),
+ "varying number of hash dimensions per range is not yet supported");
+}
+
// This test scenario creates a table with a range partition having no upper
// bound. The range partition has a custom empty hash schema (i.e. no hash
// bucketing for the range) in the presence of non-empty table-wide hash
schema.
-TEST_F(FlexPartitioningCreateTableTest, NoUpperBoundRangeCustomHashSchema) {
+//
+// TODO(aserbin): re-enable this scenario once varying hash dimensions per
range
+// are supported
+TEST_F(FlexPartitioningCreateTableTest,
DISABLED_NoUpperBoundRangeCustomHashSchema) {
// Create a table with the following partitions:
//
// hash bucket
diff --git a/src/kudu/common/partition.h b/src/kudu/common/partition.h
index f43b0d4..fd03d54 100644
--- a/src/kudu/common/partition.h
+++ b/src/kudu/common/partition.h
@@ -83,7 +83,13 @@ class PartitionKey {
}
bool operator<(const PartitionKey& other) const {
- return ToString() < other.ToString();
+ if (hash_key_ < other.hash_key_) {
+ return true;
+ }
+ if (hash_key_ == other.hash_key_) {
+ return range_key_ < other.range_key_;
+ }
+ return false;
}
bool operator<=(const PartitionKey& other) const {
diff --git a/src/kudu/common/partition_pruner-test.cc
b/src/kudu/common/partition_pruner-test.cc
index 09bcff8..1552eec 100644
--- a/src/kudu/common/partition_pruner-test.cc
+++ b/src/kudu/common/partition_pruner-test.cc
@@ -1081,7 +1081,9 @@ TEST_F(PartitionPrunerTest, TestKudu2173) {
2));
}
-TEST_F(PartitionPrunerTest, TestHashSchemasPerRangePruning) {
+// TODO(aserbin): re-enable this scenario once varying hash dimensions per
range
+// are supported
+TEST_F(PartitionPrunerTest, DISABLED_TestHashSchemasPerRangePruning) {
// CREATE TABLE t
// (A INT8, B INT8, C STRING)
// PRIMARY KEY (A, B, C)
@@ -1480,7 +1482,9 @@ TEST_F(PartitionPrunerTest,
TestInListHashPruningPerRange) {
4, 4));
}
-TEST_F(PartitionPrunerTest, TestSingleRangeElementAndBoundaryCase) {
+// TODO(aserbin): re-enable this scenario once varying hash dimensions per
range
+// are supported
+TEST_F(PartitionPrunerTest, DISABLED_TestSingleRangeElementAndBoundaryCase) {
// CREATE TABLE t
// (A INT8, B INT8)
// PRIMARY KEY (A, B)
diff --git a/src/kudu/integration-tests/table_locations-itest.cc
b/src/kudu/integration-tests/table_locations-itest.cc
index fed0882..a274cde 100644
--- a/src/kudu/integration-tests/table_locations-itest.cc
+++ b/src/kudu/integration-tests/table_locations-itest.cc
@@ -236,10 +236,11 @@ Status TableLocationsTest::CreateTable(
CreateTableResponsePB resp;
RpcController controller;
+ RETURN_NOT_OK(proxy_->CreateTable(req, &resp, &controller));
if (resp.has_error()) {
RETURN_NOT_OK(StatusFromPB(resp.error().status()));
}
- return proxy_->CreateTable(req, &resp, &controller);
+ return Status::OK();
}
void TableLocationsTest::CreateTable(const string& table_name, int num_splits)
{
@@ -467,35 +468,48 @@ TEST_F(TableLocationsTest, TestGetTableLocations) {
}
}
-TEST_F(TableLocationsTest, TestRangeSpecificHashing) {
- const string table_name = "test";
- Schema schema({ ColumnSchema("key", STRING), ColumnSchema("val", STRING) },
2);
+TEST_F(TableLocationsTest, RangeSpecificHashingSameDimensions) {
+ const string table_name = CURRENT_TEST_NAME();
+ Schema schema({
+ ColumnSchema("str_0", STRING),
+ ColumnSchema("int_1", INT32),
+ ColumnSchema("str_2", STRING),
+ }, 3);
KuduPartialRow row(&schema);
FLAGS_enable_per_range_hash_schemas = true; // enable for testing.
- vector<pair<KuduPartialRow, KuduPartialRow>> bounds(3, { row, row });
+ // Use back-to-back and sparse range placement.
+ vector<pair<KuduPartialRow, KuduPartialRow>> bounds(5, { row, row });
ASSERT_OK(bounds[0].first.SetStringNoCopy(0, "a"));
ASSERT_OK(bounds[0].second.SetStringNoCopy(0, "b"));
- ASSERT_OK(bounds[1].first.SetStringNoCopy(0, "c"));
- ASSERT_OK(bounds[1].second.SetStringNoCopy(0, "d"));
- ASSERT_OK(bounds[2].first.SetStringNoCopy(0, "e"));
- ASSERT_OK(bounds[2].second.SetStringNoCopy(0, "f"));
+ ASSERT_OK(bounds[1].first.SetStringNoCopy(0, "b"));
+ ASSERT_OK(bounds[1].second.SetStringNoCopy(0, "c"));
+ ASSERT_OK(bounds[2].first.SetStringNoCopy(0, "c"));
+ ASSERT_OK(bounds[2].second.SetStringNoCopy(0, "d"));
+ ASSERT_OK(bounds[3].first.SetStringNoCopy(0, "e"));
+ ASSERT_OK(bounds[3].second.SetStringNoCopy(0, "f"));
+ ASSERT_OK(bounds[4].first.SetStringNoCopy(0, "g"));
+ ASSERT_OK(bounds[4].second.SetStringNoCopy(0, "h"));
vector<HashSchema> range_hash_schemas;
- HashSchema hash_schema_4_by_2 = { { { "key" }, 4, 0 }, { { "val" }, 2, 0} };
- range_hash_schemas.emplace_back(hash_schema_4_by_2);
- HashSchema hash_schema_6 = { { { "key" }, 6, 2 } };
+ HashSchema hash_schema_5 = { { { "str_0", "int_1" }, 5, 0 } };
+ range_hash_schemas.emplace_back(hash_schema_5);
+ HashSchema hash_schema_4 = { { { "str_0" }, 4, 1 } };
+ range_hash_schemas.emplace_back(hash_schema_4);
+ HashSchema hash_schema_3 = { { { "int_1" }, 3, 2 } };
+ range_hash_schemas.emplace_back(hash_schema_3);
+ HashSchema hash_schema_6 = { { { "str_2", "str_0" }, 6, 3 } };
range_hash_schemas.emplace_back(hash_schema_6);
- // Use 5 bucket hash schema as the table-wide one.
- HashSchema table_hash_schema_5 = { { { "val" }, 5, 4 } };
+ // Use 2 bucket hash schema as the table-wide one.
+ HashSchema table_hash_schema_2 = { { { "str_2" }, 2, 4 } };
// Apply table-wide hash schema applied to this range.
- range_hash_schemas.emplace_back(table_hash_schema_5);
+ range_hash_schemas.emplace_back(table_hash_schema_2);
ASSERT_OK(CreateTable(
- table_name, schema, {}, bounds, range_hash_schemas,
table_hash_schema_5));
- NO_FATALS(CheckMasterTableCreation(table_name, 19));
+ table_name, schema, {}, bounds, range_hash_schemas,
table_hash_schema_2));
+ NO_FATALS(CheckMasterTableCreation(table_name, 20));
// The default setting for GetTableLocationsRequestPB::max_returned_locations
// is 10 , but here it's necessary to fetch all the existing tablets.
@@ -506,40 +520,75 @@ TEST_F(TableLocationsTest, TestRangeSpecificHashing) {
RpcController controller;
ASSERT_OK(proxy_->GetTableLocations(req, &resp, &controller));
SCOPED_TRACE(SecureDebugString(resp));
-
ASSERT_FALSE(resp.has_error());
- ASSERT_EQ(19, resp.tablet_locations().size());
-
- vector<PartitionKey> partition_key_starts = {
- { string("\0\0\0\0" "\0\0\0\0", 8), string("a" "\0\0", 3) },
- { string("\0\0\0\0" "\0\0\0\1", 8), string("a" "\0\0", 3) },
- { string("\0\0\0\1" "\0\0\0\0", 8), string("a" "\0\0", 3) },
- { string("\0\0\0\1" "\0\0\0\1", 8), string("a" "\0\0", 3) },
- { string("\0\0\0\2" "\0\0\0\0", 8), string("a" "\0\0", 3) },
- { string("\0\0\0\2" "\0\0\0\1", 8), string("a" "\0\0", 3) },
- { string("\0\0\0\3" "\0\0\0\0", 8), string("a" "\0\0", 3) },
- { string("\0\0\0\3" "\0\0\0\1", 8), string("a" "\0\0", 3) },
- { string("\0\0\0\0", 4), string("c" "\0\0", 3) },
- { string("\0\0\0\1", 4), string("c" "\0\0", 3) },
- { string("\0\0\0\2", 4), string("c" "\0\0", 3) },
- { string("\0\0\0\3", 4), string("c" "\0\0", 3) },
- { string("\0\0\0\4", 4), string("c" "\0\0", 3) },
- { string("\0\0\0\5", 4), string("c" "\0\0", 3) },
- { string("\0\0\0\0", 4), string("e" "\0\0", 3) },
- { string("\0\0\0\1", 4), string("e" "\0\0", 3) },
- { string("\0\0\0\2", 4), string("e" "\0\0", 3) },
- { string("\0\0\0\3", 4), string("e" "\0\0", 3) },
- { string("\0\0\0\4", 4), string("e" "\0\0", 3) },
+ ASSERT_EQ(20, resp.tablet_locations().size());
+
+ vector<PartitionKey> partition_keys_ref = {
+ { string("\0\0\0\0", 4), string("a" "\0\0\0\0\0\0", 7) },
+ { string("\0\0\0\1", 4), string("a" "\0\0\0\0\0\0", 7) },
+ { string("\0\0\0\2", 4), string("a" "\0\0\0\0\0\0", 7) },
+ { string("\0\0\0\3", 4), string("a" "\0\0\0\0\0\0", 7) },
+ { string("\0\0\0\4", 4), string("a" "\0\0\0\0\0\0", 7) },
+
+ { string("\0\0\0\0", 4), string("b" "\0\0\0\0\0\0", 7) },
+ { string("\0\0\0\1", 4), string("b" "\0\0\0\0\0\0", 7) },
+ { string("\0\0\0\2", 4), string("b" "\0\0\0\0\0\0", 7) },
+ { string("\0\0\0\3", 4), string("b" "\0\0\0\0\0\0", 7) },
+
+ { string("\0\0\0\0", 4), string("c" "\0\0\0\0\0\0", 7) },
+ { string("\0\0\0\1", 4), string("c" "\0\0\0\0\0\0", 7) },
+ { string("\0\0\0\2", 4), string("c" "\0\0\0\0\0\0", 7) },
+
+ { string("\0\0\0\0", 4), string("e" "\0\0\0\0\0\0", 7) },
+ { string("\0\0\0\1", 4), string("e" "\0\0\0\0\0\0", 7) },
+ { string("\0\0\0\2", 4), string("e" "\0\0\0\0\0\0", 7) },
+ { string("\0\0\0\3", 4), string("e" "\0\0\0\0\0\0", 7) },
+ { string("\0\0\0\4", 4), string("e" "\0\0\0\0\0\0", 7) },
+ { string("\0\0\0\5", 4), string("e" "\0\0\0\0\0\0", 7) },
+
+ { string("\0\0\0\0", 4), string("g" "\0\0\0\0\0\0", 7) },
+ { string("\0\0\0\1", 4), string("g" "\0\0\0\0\0\0", 7) },
};
// Sorting partition keys to match the tablets that are returned in sorted
order.
- sort(partition_key_starts.begin(), partition_key_starts.end());
- ASSERT_EQ(partition_key_starts.size(), resp.tablet_locations_size());
- for (int i = 0; i < resp.tablet_locations_size(); i++) {
- EXPECT_EQ(partition_key_starts[i].ToString(),
+ sort(partition_keys_ref.begin(), partition_keys_ref.end());
+ ASSERT_EQ(partition_keys_ref.size(), resp.tablet_locations_size());
+ for (auto i = 0; i < resp.tablet_locations_size(); ++i) {
+ EXPECT_EQ(partition_keys_ref[i].ToString(),
resp.tablet_locations(i).partition().partition_key_start());
}
}
+// TODO(aserbin): update this scenario once varying hash dimensions per range
+// are supported
+TEST_F(TableLocationsTest, RangeSpecificHashingVaryingDimensions) {
+ const string table_name = "test";
+ Schema schema({ ColumnSchema("key", STRING), ColumnSchema("val", STRING) },
2);
+ KuduPartialRow row(&schema);
+
+ FLAGS_enable_per_range_hash_schemas = true; // enable for testing.
+
+ vector<pair<KuduPartialRow, KuduPartialRow>> bounds(2, { row, row });
+ ASSERT_OK(bounds[0].first.SetStringNoCopy(0, "a"));
+ ASSERT_OK(bounds[0].second.SetStringNoCopy(0, "b"));
+ ASSERT_OK(bounds[1].first.SetStringNoCopy(0, "c"));
+ ASSERT_OK(bounds[1].second.SetStringNoCopy(0, "d"));
+
+ vector<HashSchema> range_hash_schemas;
+ HashSchema hash_schema_3_by_2 = { { { "key" }, 3, 0 }, { { "val" }, 2, 1 } };
+ range_hash_schemas.emplace_back(hash_schema_3_by_2);
+
+ // Use 4 bucket hash schema as the table-wide one.
+ HashSchema table_hash_schema_4 = { { { "val" }, 4, 2 } };
+ // Apply table-wide hash schema applied to this range.
+ range_hash_schemas.emplace_back(table_hash_schema_4);
+
+ const auto s = CreateTable(
+ table_name, schema, {}, bounds, range_hash_schemas, table_hash_schema_4);
+ ASSERT_TRUE(s.IsNotSupported()) << s.ToString();
+ ASSERT_STR_CONTAINS(s.ToString(),
+ "varying number of hash dimensions per range is not yet supported");
+}
+
TEST_F(TableLocationsWithTSLocationTest, TestGetTSLocation) {
const string table_name = "test";
Schema schema({ ColumnSchema("key", STRING) }, 1);
diff --git a/src/kudu/master/catalog_manager.cc
b/src/kudu/master/catalog_manager.cc
index 36b6775..cb9ab1c 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -1888,8 +1888,22 @@ Status CatalogManager::CreateTable(const
CreateTableRequestPB* orig_req,
// Create partitions based on specified partition schema and split rows.
vector<Partition> partitions;
- RETURN_NOT_OK(partition_schema.CreatePartitions(split_rows, range_bounds,
- range_hash_schemas, schema,
&partitions));
+ RETURN_NOT_OK(partition_schema.CreatePartitions(
+ split_rows, range_bounds, range_hash_schemas, schema, &partitions));
+
+ // Check the restriction on the same number of hash dimensions across all the
+ // ranges.
+ //
+ // TODO(aserbin): remove the restriction once the rest of the code is ready
+ // to handle range partitions with arbitrary hash schemas
+ CHECK(!partitions.empty());
+ const auto hash_dimensions_num = partitions.begin()->hash_buckets().size();
+ for (const auto& p : partitions) {
+ if (p.hash_buckets().size() != hash_dimensions_num) {
+ return Status::NotSupported(
+ "varying number of hash dimensions per range is not yet supported");
+ }
+ }
// If they didn't specify a num_replicas, set it based on the default.
if (!req.has_num_replicas()) {
