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
commit 09a6c8833e085dd46c18acec611191e6374c73b5 Author: Alexey Serbin <[email protected]> AuthorDate: Mon May 9 14:21:37 2022 -0700 KUDU-2671 relax requirement for partition range specification As of now (see [1]), the number of hash dimensions in a per-range custom hash schema must be the same for all the ranges in a table. With that, it's possible to relax the requirements on specifying partition key ranges for a GetTableLocations RPC. That allows legacy Kudu clients to work with tables having custom hash schemas per range. If necessary, it's possible to make the requirements stricter by setting --require_new_spec_for_custom_hash_schema_range_bound=true. This is a follow-up to [1]. [1] https://github.com/apache/kudu/commit/6998193e69eeda497f912d1d806470c95b591ad4 Change-Id: Idfdae92f6baee5e8f197cba41c8e51120f9b4d58 Reviewed-on: http://gerrit.cloudera.org:8080/18509 Reviewed-by: Attila Bukor <[email protected]> Tested-by: Alexey Serbin <[email protected]> --- src/kudu/master/catalog_manager-test.cc | 21 ++++++++++++++++++++- src/kudu/master/catalog_manager.cc | 30 +++++++++++++++++++----------- 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/src/kudu/master/catalog_manager-test.cc b/src/kudu/master/catalog_manager-test.cc index 38db44473..96898656e 100644 --- a/src/kudu/master/catalog_manager-test.cc +++ b/src/kudu/master/catalog_manager-test.cc @@ -23,6 +23,7 @@ #include <vector> #include <glog/logging.h> +#include <gflags/gflags_declare.h> #include <gtest/gtest.h> #include "kudu/common/common.pb.h" @@ -37,6 +38,8 @@ #include "kudu/util/test_macros.h" #include "kudu/util/test_util.h" +DECLARE_bool(require_new_spec_for_custom_hash_schema_range_bound); + using std::iota; using std::string; using std::vector; @@ -134,8 +137,24 @@ TEST(TableInfoTest, GetTableLocationsLegacyCustomHashSchemas) { table->AddRemoveTablets({ tablet }, {}); // Query by specifying the start of the partition via the partition_key_start - // field: it should fail since the table has a range with custom hash schema. + // field: it should pass even if the table has a range with custom hash schema + // since as of now all the range partitions must have the number of hash + // dimenstions fixed across all the ranges in a table. + { + GetTableLocationsRequestPB req; + req.set_max_returned_locations(1); + req.mutable_table()->mutable_table_name()->assign(table_id); + req.mutable_partition_key_start()->assign("a"); + vector<scoped_refptr<TabletInfo>> tablets_in_range; + ASSERT_OK(table->GetTabletsInRange(&req, &tablets_in_range)); + ASSERT_EQ(1, tablets_in_range.size()); + } + + // Query by specifying the start of the partition via the partition_key_start + // field: it should fail since the table has a range with custom hash schema + // and --require_new_spec_for_custom_hash_schema_range_bound=true. { + FLAGS_require_new_spec_for_custom_hash_schema_range_bound = true; GetTableLocationsRequestPB req; req.set_max_returned_locations(1); req.mutable_table()->mutable_table_name()->assign(table_id); diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc index f16ae3d3a..90511d145 100644 --- a/src/kudu/master/catalog_manager.cc +++ b/src/kudu/master/catalog_manager.cc @@ -397,6 +397,13 @@ DEFINE_bool(enable_chunked_tablet_writes, true, TAG_FLAG(enable_chunked_tablet_writes, experimental); TAG_FLAG(enable_chunked_tablet_writes, runtime); +DEFINE_bool(require_new_spec_for_custom_hash_schema_range_bound, false, + "Whether to require the client to use newer signature to specify " + "range bounds when working with a table having custom hash schema " + "per range"); +TAG_FLAG(require_new_spec_for_custom_hash_schema_range_bound, experimental); +TAG_FLAG(require_new_spec_for_custom_hash_schema_range_bound, runtime); + DECLARE_bool(raft_prepare_replacement_before_eviction); DECLARE_int64(tsk_rotation_seconds); DECLARE_string(ranger_config_path); @@ -807,7 +814,7 @@ void CatalogManagerBgTasks::Run() { catalog_manager_->ExtractDeletedTablesAndTablets(&deleted_tables, &deleted_tablets); Status s = Status::OK(); // Clean up metadata for deleted tablets first and then clean up metadata for deleted - // tables. This is the reverse of the order in which we load them. So for any remaining + // tables. This is the reverse of the order in which we load them. So for any remaining // tablet, the metadata of the table to which it belongs must exist. const time_t now = time(nullptr); if (!deleted_tablets.empty()) { @@ -6557,6 +6564,11 @@ Status TableInfo::GetTabletsInRange( const GetTableLocationsRequestPB* req, vector<scoped_refptr<TabletInfo>>* ret) const { + static constexpr const char* const kErrRangeNewSpec = + "$0: for a table with custom per-range hash schemas the range must " + "be specified using partition_key_range field, not " + "partition_key_{start,end} fields"; + size_t hash_dimensions_num = 0; bool has_custom_hash_schemas = false; { @@ -6579,11 +6591,9 @@ Status TableInfo::GetTabletsInRange( has_key_start = true; } } else if (req->has_partition_key_start()) { - if (has_custom_hash_schemas) { - return Status::InvalidArgument(Substitute( - "$0: for a table with custom per-range hash schemas the range must " - "be specified using partition_key_range field, not " - "partition_key_{start,end} fields", ToString())); + if (has_custom_hash_schemas && + FLAGS_require_new_spec_for_custom_hash_schema_range_bound) { + return Status::InvalidArgument(Substitute(kErrRangeNewSpec, ToString())); } partition_key_start = Partition::StringToPartitionKey( req->partition_key_start(), hash_dimensions_num); @@ -6599,11 +6609,9 @@ Status TableInfo::GetTabletsInRange( has_key_end = true; } } else if (req->has_partition_key_end()) { - if (has_custom_hash_schemas) { - return Status::InvalidArgument(Substitute( - "$0: for a table with custom per-range hash schemas the range must " - "be specified using partition_key_range field, not " - "partition_key_{start,end} fields", ToString())); + if (has_custom_hash_schemas && + FLAGS_require_new_spec_for_custom_hash_schema_range_bound) { + return Status::InvalidArgument(Substitute(kErrRangeNewSpec, ToString())); } partition_key_end = Partition::StringToPartitionKey( req->partition_key_end(), hash_dimensions_num);
