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 9ae68af0d KUDU-3515: fix incompatibility introduced with KUDU-2671
9ae68af0d is described below
commit 9ae68af0dcd7a58c9fe754f69fb99861ba26e2ff
Author: Alexey Serbin <[email protected]>
AuthorDate: Fri Sep 29 13:02:01 2023 -0700
KUDU-3515: fix incompatibility introduced with KUDU-2671
This patch addresses the issue reported in KUDU-3515. The issue has
been originally reported at the #kudu-general Slack channel [1].
The root cause of the issue is an incompatibility in the serialized
representation of tablets' partition keys introduced by changelist [2].
The essence of the fix is to convert the serialized partition keys
representing unbounded ends of the tables' ranges from the legacy
to the new format on-the-fly while loading that information from the
system catalog upon bootstrapping a leader master.
This patch contains unit test scenarios for the function that performs
the conversion from the legacy to the new format.
Also, I verified the fix works as expected for an existing Kudu 1.16.0
cluster with a bunch of tables after upgrading the binaries to Kudu
1.17.0 bits.
This is a follow-up to 8df970f7a6520bb0dc0f9cc89ad7f62ab349e84d.
[1] https://getkudu.slack.com/archives/C0CPXJ3CH/p1695107377230829
[2] https://github.com/apache/kudu/commit/8df970f7a652
Change-Id: I45df424770a09cf7c94f5e1d390757f29f9fb3f4
Reviewed-on: http://gerrit.cloudera.org:8080/20525
Tested-by: Alexey Serbin <[email protected]>
Reviewed-by: Mahesh Reddy <[email protected]>
Reviewed-by: Abhishek Chennaka <[email protected]>
---
src/kudu/common/partition.cc | 33 +++++
src/kudu/common/partition.h | 23 +++
src/kudu/master/CMakeLists.txt | 1 +
src/kudu/master/catalog_manager.cc | 63 +-------
src/kudu/master/sys_catalog-test.cc | 279 ++++++++++++++++++++++++++++++++++--
src/kudu/master/tablet_loader.cc | 148 +++++++++++++++++++
src/kudu/master/tablet_loader.h | 67 +++++++++
7 files changed, 543 insertions(+), 71 deletions(-)
diff --git a/src/kudu/common/partition.cc b/src/kudu/common/partition.cc
index 8089e653d..b8ccd386c 100644
--- a/src/kudu/common/partition.cc
+++ b/src/kudu/common/partition.cc
@@ -124,6 +124,39 @@ PartitionKey Partition::StringToPartitionKey(const
std::string& key_str,
key_str.substr(hash_part_size));
}
+bool PartitionKey::IsLegacy(const PartitionKey& pk,
+ const vector<int32_t>& hash_buckets) {
+ return pk.range_key().empty() && !hash_buckets.empty() &&
+ pk.hash_key().size() != kEncodedBucketSize * hash_buckets.size();
+
+}
+
+PartitionKey PartitionKey::LowerBoundFromLegacy(const PartitionKey& pk,
+ const vector<int32_t>&
hash_buckets) {
+ DCHECK(PartitionKey::IsLegacy(pk, hash_buckets));
+ const auto& hash_encoder = GetKeyEncoder<string>(GetTypeInfo(UINT32));
+ PartitionKey res;
+ for (const auto bucket : hash_buckets) {
+ hash_encoder.Encode(&bucket, res.mutable_hash_key());
+ }
+ return res;
+}
+
+PartitionKey PartitionKey::UpperBoundFromLegacy(const PartitionKey& pk,
+ const vector<int32_t>&
hash_buckets) {
+ DCHECK(PartitionKey::IsLegacy(pk, hash_buckets));
+ const auto& hash_encoder = GetKeyEncoder<string>(GetTypeInfo(UINT32));
+ PartitionKey res;
+ for (auto idx = 0; idx < hash_buckets.size(); ++idx) {
+ int32_t b = hash_buckets[idx];
+ if (idx + 1 == hash_buckets.size()) {
+ ++b;
+ }
+ hash_encoder.Encode(&b, res.mutable_hash_key());
+ }
+ return res;
+}
+
string PartitionKey::DebugString() const {
std::ostringstream ss;
ss << "h:";
diff --git a/src/kudu/common/partition.h b/src/kudu/common/partition.h
index 20c88222e..cd0353f70 100644
--- a/src/kudu/common/partition.h
+++ b/src/kudu/common/partition.h
@@ -62,10 +62,33 @@ template <typename Buffer> class KeyEncoder;
// serialized representation.
class PartitionKey {
public:
+ // Return 'true' if the partition key 'pk' for the range partition with
+ // the bucket indices for its hash schema specified in 'hash_buckets' is
+ // in legacy (pre-KUDU-2671) representation, 'false' otherwise.
+ // The difference between the two representations exists only for unbounded
+ // ranges that have non-trivial hash schemas (i.e. when a range is backed
+ // at least by two tablets corresponding to different hash buckets).
+ // For details, see https://github.com/apache/kudu/commit/8df970f7a652
+ static bool IsLegacy(const PartitionKey& pk,
+ const std::vector<int32_t>& hash_buckets);
+
+ // Convert lower bound partition key 'pk' from pre-KUDU-2671 (i.e. legacy)
+ // representation into post-KUDU-2671 representation.
+ static PartitionKey LowerBoundFromLegacy(const PartitionKey& pk,
+ const std::vector<int32_t>&
hash_buckets);
+
+ // Convert upper bound partition key 'pk' from pre-KUDU-2671 (i.e. legacy)
+ // representation into post-KUDU-2671 representation.
+ static PartitionKey UpperBoundFromLegacy(const PartitionKey& pk,
+ const std::vector<int32_t>&
hash_buckets);
+
// Build a PartitionKey that represents infinity. For the beginning of a
// partition represents '-inf'; for the end of a partition represents '+inf'.
PartitionKey() = default;
+ // Build a PartitionKey from another instance of this class.
+ PartitionKey(const PartitionKey& other) = default;
+
// Build PartitionKey object given hash and range parts.
PartitionKey(std::string hash_key, std::string range_key)
: hash_key_(std::move(hash_key)),
diff --git a/src/kudu/master/CMakeLists.txt b/src/kudu/master/CMakeLists.txt
index 21fe08058..6c7c1ed8f 100644
--- a/src/kudu/master/CMakeLists.txt
+++ b/src/kudu/master/CMakeLists.txt
@@ -67,6 +67,7 @@ set(MASTER_SRCS
table_locations_cache.cc
table_locations_cache_metrics.cc
table_metrics.cc
+ tablet_loader.cc
ts_descriptor.cc
ts_manager.cc
txn_manager.cc
diff --git a/src/kudu/master/catalog_manager.cc
b/src/kudu/master/catalog_manager.cc
index ef0b3444f..292f39005 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -110,6 +110,7 @@
#include "kudu/master/table_locations_cache.h"
#include "kudu/master/table_locations_cache_metrics.h"
#include "kudu/master/table_metrics.h"
+#include "kudu/master/tablet_loader.h"
#include "kudu/master/ts_descriptor.h"
#include "kudu/master/ts_manager.h"
#include "kudu/rpc/messenger.h" // IWYU pragma: keep
@@ -599,13 +600,13 @@ GROUP_FLAG_VALIDATOR(replication_factor_flags,
ValidateReplicationFactorFlags);
} // anonymous namespace
+namespace kudu {
+namespace master {
+
////////////////////////////////////////////////////////////
// Table Loader
////////////////////////////////////////////////////////////
-namespace kudu {
-namespace master {
-
class TableLoader : public TableVisitor {
public:
explicit TableLoader(CatalogManager *catalog_manager)
@@ -676,62 +677,6 @@ class TableLoader : public TableVisitor {
DISALLOW_COPY_AND_ASSIGN(TableLoader);
};
-////////////////////////////////////////////////////////////
-// Tablet Loader
-////////////////////////////////////////////////////////////
-
-class TabletLoader : public TabletVisitor {
- public:
- explicit TabletLoader(CatalogManager *catalog_manager)
- : catalog_manager_(catalog_manager) {
- }
-
- Status VisitTablet(const string& table_id,
- const string& tablet_id,
- const SysTabletsEntryPB& metadata) override {
- // Lookup the table.
- scoped_refptr<TableInfo> table(FindPtrOrNull(
- catalog_manager_->table_ids_map_, table_id));
- if (table == nullptr) {
- // Tables and tablets are always created/deleted in one operation, so
- // this shouldn't be possible.
- string msg = Substitute("Missing table $0 required by tablet $1
(metadata: $2)",
- table_id, tablet_id,
SecureDebugString(metadata));
- LOG(ERROR) << msg;
- return Status::Corruption(msg);
- }
-
- // Set up the tablet info.
- scoped_refptr<TabletInfo> tablet = new TabletInfo(table, tablet_id);
- TabletMetadataLock l(tablet.get(), LockMode::WRITE);
- l.mutable_data()->pb.CopyFrom(metadata);
-
- // Add the tablet to the tablet manager.
- catalog_manager_->tablet_map_[tablet->id()] = tablet;
-
- // Add the tablet to the table.
- bool is_deleted = l.mutable_data()->is_deleted();
- l.Commit();
- if (!is_deleted) {
- // Need to use a new tablet lock here because AddRemoveTablets() reads
- // from clean state, which is uninitialized for these brand new tablets.
- TabletMetadataLock l(tablet.get(), LockMode::READ);
- table->AddRemoveTablets({ tablet }, {});
- LOG(INFO) << Substitute("Loaded metadata for tablet $0 (table $1)",
- tablet_id, table->ToString());
- }
-
- VLOG(2) << Substitute("Metadata for tablet $0: $1",
- tablet_id, SecureShortDebugString(metadata));
- return Status::OK();
- }
-
- private:
- CatalogManager *catalog_manager_;
-
- DISALLOW_COPY_AND_ASSIGN(TabletLoader);
-};
-
////////////////////////////////////////////////////////////
// TSK (Token Signing Key) Entry Loader
////////////////////////////////////////////////////////////
diff --git a/src/kudu/master/sys_catalog-test.cc
b/src/kudu/master/sys_catalog-test.cc
index 3841f1bcb..8a0a940f2 100644
--- a/src/kudu/master/sys_catalog-test.cc
+++ b/src/kudu/master/sys_catalog-test.cc
@@ -27,6 +27,7 @@
#include <gtest/gtest.h>
#include "kudu/common/common.pb.h"
+#include "kudu/common/partition.h"
#include "kudu/common/schema.h"
#include "kudu/common/wire_protocol.h"
#include "kudu/gutil/ref_counted.h"
@@ -35,6 +36,7 @@
#include "kudu/master/master.pb.h"
#include "kudu/master/master.proxy.h"
#include "kudu/master/mini_master.h"
+#include "kudu/master/tablet_loader.h"
#include "kudu/rpc/messenger.h"
#include "kudu/security/cert.h"
#include "kudu/security/crypto.h"
@@ -68,6 +70,18 @@ class Message;
namespace kudu {
namespace master {
+static bool PbEquals(const google::protobuf::Message& a, const
google::protobuf::Message& b) {
+ return pb_util::SecureDebugString(a) == pb_util::SecureDebugString(b);
+}
+
+template<class C>
+static bool MetadatasEqual(const scoped_refptr<C>& ti_a,
+ const scoped_refptr<C>& ti_b) {
+ MetadataLock<C> l_a(ti_a.get(), LockMode::READ);
+ MetadataLock<C> l_b(ti_b.get(), LockMode::READ);
+ return PbEquals(l_a.data().pb, l_b.data().pb);
+}
+
class SysCatalogTest : public KuduTest {
protected:
void SetUp() override {
@@ -98,18 +112,6 @@ class SysCatalogTest : public KuduTest {
unique_ptr<MasterServiceProxy> proxy_;
};
-static bool PbEquals(const google::protobuf::Message& a, const
google::protobuf::Message& b) {
- return pb_util::SecureDebugString(a) == pb_util::SecureDebugString(b);
-}
-
-template<class C>
-static bool MetadatasEqual(const scoped_refptr<C>& ti_a,
- const scoped_refptr<C>& ti_b) {
- MetadataLock<C> l_a(ti_a.get(), LockMode::READ);
- MetadataLock<C> l_b(ti_b.get(), LockMode::READ);
- return PbEquals(l_a.data().pb, l_b.data().pb);
-}
-
// Test the sys-catalog tables basic operations (add, update, delete,
// visit)
TEST_F(SysCatalogTest, TestSysCatalogTablesOperations) {
@@ -475,5 +477,258 @@ TEST_F(SysCatalogTest, LoadClusterID) {
ASSERT_EQ(init_id, cluster_id_entry.cluster_id());
}
+// A unit test for the TabletLoader::ConvertFromLegacy(PartitionPB*) method.
+TEST_F(SysCatalogTest, TabletRangesConversionFromLegacyFormat) {
+ // Zero hash dimensions.
+ {
+ PartitionPB pb;
+ PartitionPB pb_orig(pb);
+ ASSERT_FALSE(TabletLoader::ConvertFromLegacy(&pb));
+ ASSERT_TRUE(PbEquals(pb_orig, pb));
+ }
+ {
+ PartitionPB pb;
+ *pb.mutable_partition_key_start() = "";
+ PartitionPB pb_orig(pb);
+ ASSERT_FALSE(TabletLoader::ConvertFromLegacy(&pb));
+ ASSERT_TRUE(PbEquals(pb_orig, pb));
+ }
+ {
+ PartitionPB pb;
+ *pb.mutable_partition_key_end() = "";
+ PartitionPB pb_orig(pb);
+ ASSERT_FALSE(TabletLoader::ConvertFromLegacy(&pb));
+ ASSERT_TRUE(PbEquals(pb_orig, pb));
+ }
+ {
+ PartitionPB pb;
+ *pb.mutable_partition_key_start() = "";
+ *pb.mutable_partition_key_end() = "";
+ PartitionPB pb_orig(pb);
+ ASSERT_FALSE(TabletLoader::ConvertFromLegacy(&pb));
+ ASSERT_TRUE(PbEquals(pb_orig, pb));
+ }
+ {
+ PartitionPB pb;
+ *pb.mutable_partition_key_start() = "a";
+ *pb.mutable_partition_key_end() = "b";
+ PartitionPB pb_orig(pb);
+ ASSERT_FALSE(TabletLoader::ConvertFromLegacy(&pb));
+ ASSERT_TRUE(PbEquals(pb_orig, pb));
+ }
+ {
+ PartitionPB pb;
+ *pb.mutable_partition_key_start() = "";
+ *pb.mutable_partition_key_end() = string("\0\0\0\1\0\0\0\2", 8) + "xyz";
+ PartitionPB pb_orig(pb);
+ ASSERT_FALSE(TabletLoader::ConvertFromLegacy(&pb));
+ ASSERT_TRUE(PbEquals(pb_orig, pb));
+ }
+ {
+ PartitionPB pb;
+ *pb.mutable_partition_key_start() = string("\0\0\0\1", 4);
+ *pb.mutable_partition_key_end() = "";
+ PartitionPB pb_orig(pb);
+ ASSERT_FALSE(TabletLoader::ConvertFromLegacy(&pb));
+ ASSERT_TRUE(PbEquals(pb_orig, pb));
+ }
+ {
+ PartitionPB pb;
+ *pb.mutable_partition_key_start() = string("\0\0\0\1", 4);
+ *pb.mutable_partition_key_end() = string("\0\0\0\1\0\0\0\2", 8) + "xyz";
+ PartitionPB pb_orig(pb);
+ ASSERT_FALSE(TabletLoader::ConvertFromLegacy(&pb));
+ ASSERT_TRUE(PbEquals(pb_orig, pb));
+ }
+
+ // One hash dimension, bounded range, short range keys.
+ // [ (1, "ab"), (1, "ac") ) --> same
+ {
+ PartitionPB pb;
+ pb.add_hash_buckets(1);
+ *pb.mutable_partition_key_start() = string("\0\0\0\1", 4) + "ab";
+ *pb.mutable_partition_key_end() = string("\0\0\0\1", 4) + "ac";
+ PartitionPB pb_orig(pb);
+ ASSERT_FALSE(TabletLoader::ConvertFromLegacy(&pb));
+ ASSERT_TRUE(PbEquals(pb_orig, pb));
+ }
+
+ // One hash dimension, bounded range, long range keys.
+ // [ (2, "0123456789"), (2, "abcdefghijklmn") ) --> same
+ {
+ PartitionPB pb;
+ pb.add_hash_buckets(1);
+ *pb.mutable_partition_key_start() = string("\0\0\0\2", 4) + "0123456789";
+ *pb.mutable_partition_key_end() = string("\0\0\0\2", 4) + "abcdefghijklmn";
+ PartitionPB pb_orig(pb);
+ ASSERT_FALSE(TabletLoader::ConvertFromLegacy(&pb));
+ ASSERT_TRUE(PbEquals(pb_orig, pb));
+ }
+
+ // Two hash dimensions, bounded range, long range keys.
+ // [ (0, 5, "0000000000000005"), (0, 5, "0000000000000006") ) --> same
+ {
+ PartitionPB pb;
+ pb.add_hash_buckets(1);
+ *pb.mutable_partition_key_start() =
+ string("\0\0\0\0\0\0\0\5", 8) + "0000000000000005";
+ *pb.mutable_partition_key_end() =
+ string("\0\0\0\0\0\0\0\5", 8) + "0000000000000006";
+ PartitionPB pb_orig(pb);
+ ASSERT_FALSE(TabletLoader::ConvertFromLegacy(&pb));
+ ASSERT_TRUE(PbEquals(pb_orig, pb));
+ }
+
+ // One hash dimension, new format:
+ // [ (1, _), (1, "a") ) --> same
+ {
+ PartitionPB pb;
+ pb.add_hash_buckets(1);
+ *pb.mutable_partition_key_start() = string("\0\0\0\1", 4);
+ *pb.mutable_partition_key_end() = string("\0\0\0\1", 4) + "a";
+ PartitionPB pb_orig(pb);
+ ASSERT_FALSE(TabletLoader::ConvertFromLegacy(&pb));
+ ASSERT_TRUE(PbEquals(pb_orig, pb));
+ }
+
+ // Two hash dimensions, new format:
+ // [ (2, 0, _), (2, 0, "t") ) --> same
+ {
+ PartitionPB pb;
+ pb.add_hash_buckets(2);
+ pb.add_hash_buckets(0);
+ *pb.mutable_partition_key_start() = string("\0\0\0\2\0\0\0\0", 8);
+ *pb.mutable_partition_key_end() = string("\0\0\0\2\0\0\0\0", 8) + "t";
+ PartitionPB pb_orig(pb);
+ ASSERT_FALSE(TabletLoader::ConvertFromLegacy(&pb));
+ ASSERT_TRUE(PbEquals(pb_orig, pb));
+ }
+
+ // One hash dimension, legacy format:
+ // [ (_, _), (2, "b") ) --> [ (2, _), (2, "b") )
+ {
+ PartitionPB pb;
+ pb.add_hash_buckets(2);
+ *pb.mutable_partition_key_start() = "";
+ *pb.mutable_partition_key_end() = string("\0\0\0\2", 4) + "b";
+ PartitionPB pb_orig(pb);
+ ASSERT_TRUE(TabletLoader::ConvertFromLegacy(&pb));
+ ASSERT_FALSE(PbEquals(pb_orig, pb));
+
+ // Verify the results of the conversion.
+ Partition p;
+ Partition::FromPB(pb, &p);
+ ASSERT_EQ(string("\0\0\0\2", 4), p.begin().hash_key());
+ ASSERT_EQ("", p.begin().range_key());
+ ASSERT_EQ(string("\0\0\0\2", 4), p.end().hash_key());
+ ASSERT_EQ("b", p.end().range_key());
+ }
+
+ // Two hash dimensions, legacy format:
+ // [ (3, _, _), (3, 1, "u") ) --> [ (3, 1, _), (3, 1, "u") )
+ {
+ PartitionPB pb;
+ pb.add_hash_buckets(3);
+ pb.add_hash_buckets(1);
+ *pb.mutable_partition_key_start() = string("\0\0\0\3", 4);
+ *pb.mutable_partition_key_end() = string("\0\0\0\3\0\0\0\1", 8) + "u";
+ PartitionPB pb_orig(pb);
+ ASSERT_TRUE(TabletLoader::ConvertFromLegacy(&pb));
+ ASSERT_FALSE(PbEquals(pb_orig, pb));
+
+ // Verify the results of the conversion.
+ Partition p;
+ Partition::FromPB(pb, &p);
+ ASSERT_EQ(string("\0\0\0\3\0\0\0\1", 8), p.begin().hash_key());
+ ASSERT_EQ("", p.begin().range_key());
+ ASSERT_EQ(string("\0\0\0\3\0\0\0\1", 8), p.end().hash_key());
+ ASSERT_EQ("u", p.end().range_key());
+ }
+
+ // One hash dimension, new format:
+ // [ (3, "d"), (4, _) ) --> same
+ {
+ PartitionPB pb;
+ pb.add_hash_buckets(3);
+ *pb.mutable_partition_key_start() = string("\0\0\0\3", 4) + "c";
+ *pb.mutable_partition_key_end() = string("\0\0\0\4", 4);
+ PartitionPB pb_orig(pb);
+ ASSERT_FALSE(TabletLoader::ConvertFromLegacy(&pb));
+ ASSERT_TRUE(PbEquals(pb_orig, pb));
+ }
+
+ // One hash dimension, legacy format:
+ // [ (4, "d"), (_, _) ) --> [ (4, "d"), (5, _) )
+ {
+ PartitionPB pb;
+ pb.add_hash_buckets(4);
+ *pb.mutable_partition_key_start() = string("\0\0\0\4", 4) + "d";
+ *pb.mutable_partition_key_end() = "";
+ PartitionPB pb_orig(pb);
+ ASSERT_TRUE(TabletLoader::ConvertFromLegacy(&pb));
+ ASSERT_FALSE(PbEquals(pb_orig, pb));
+
+ Partition p;
+ Partition::FromPB(pb, &p);
+ ASSERT_EQ(string("\0\0\0\4", 4), p.begin().hash_key());
+ ASSERT_EQ("d", p.begin().range_key());
+ ASSERT_EQ(string("\0\0\0\5", 4), p.end().hash_key());
+ ASSERT_EQ("", p.end().range_key());
+ }
+
+ // Two hash dimensions, legacy format:
+ // [ (5, 5, "e"), (_, _, _) ) --> [ (5, 5, "e"), (5, 6, _) )
+ {
+ PartitionPB pb;
+ pb.add_hash_buckets(5);
+ pb.add_hash_buckets(5);
+ *pb.mutable_partition_key_start() = string("\0\0\0\5\0\0\0\5", 8) + "e";
+ *pb.mutable_partition_key_end() = "";
+ PartitionPB pb_orig(pb);
+ ASSERT_TRUE(TabletLoader::ConvertFromLegacy(&pb));
+ ASSERT_FALSE(PbEquals(pb_orig, pb));
+
+ Partition p;
+ Partition::FromPB(pb, &p);
+ ASSERT_EQ(string("\0\0\0\5\0\0\0\5", 8), p.begin().hash_key());
+ ASSERT_EQ("e", p.begin().range_key());
+ ASSERT_EQ(string("\0\0\0\5\0\0\0\6", 8), p.end().hash_key());
+ ASSERT_EQ("", p.end().range_key());
+ }
+
+ // Two hash dimensions, new format:
+ // [ (0, 1, "a0b1"), (0, 2, _) ) --> same
+ {
+ PartitionPB pb;
+ pb.add_hash_buckets(0);
+ pb.add_hash_buckets(1);
+ *pb.mutable_partition_key_start() = string("\0\0\0\0\0\0\0\1", 8) + "a0b1";
+ *pb.mutable_partition_key_end() = string("\0\0\0\0\0\0\0\2", 8);
+ PartitionPB pb_orig(pb);
+ ASSERT_FALSE(TabletLoader::ConvertFromLegacy(&pb));
+ ASSERT_TRUE(PbEquals(pb_orig, pb));
+ }
+
+ // Two hash dimensions, legacy format:
+ // [ (1, 2, "x"), (2, _, _) ) --> [ (1, 2, "x"), (1, 3, _) )
+ {
+ PartitionPB pb;
+ pb.add_hash_buckets(1);
+ pb.add_hash_buckets(2);
+ *pb.mutable_partition_key_start() = string("\0\0\0\1\0\0\0\2", 8) + "x";
+ *pb.mutable_partition_key_end() = string("\0\0\0\2", 4);
+ PartitionPB pb_orig(pb);
+ ASSERT_TRUE(TabletLoader::ConvertFromLegacy(&pb));
+ ASSERT_FALSE(PbEquals(pb_orig, pb));
+
+ Partition p;
+ Partition::FromPB(pb, &p);
+ ASSERT_EQ(string("\0\0\0\1\0\0\0\2", 8), p.begin().hash_key());
+ ASSERT_EQ("x", p.begin().range_key());
+ ASSERT_EQ(string("\0\0\0\1\0\0\0\3", 8), p.end().hash_key());
+ ASSERT_EQ("", p.end().range_key());
+ }
+}
+
} // namespace master
} // namespace kudu
diff --git a/src/kudu/master/tablet_loader.cc b/src/kudu/master/tablet_loader.cc
new file mode 100644
index 000000000..a70a8ac81
--- /dev/null
+++ b/src/kudu/master/tablet_loader.cc
@@ -0,0 +1,148 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "kudu/master/tablet_loader.h"
+
+#include <cstddef>
+#include <cstdint>
+#include <ostream>
+#include <string>
+#include <unordered_map>
+#include <vector>
+
+#include <glog/logging.h>
+
+#include "kudu/common/common.pb.h"
+#include "kudu/common/partition.h"
+#include "kudu/gutil/map-util.h"
+#include "kudu/gutil/ref_counted.h"
+#include "kudu/gutil/strings/substitute.h"
+#include "kudu/master/catalog_manager.h"
+#include "kudu/master/master.pb.h"
+#include "kudu/util/cow_object.h"
+#include "kudu/util/pb_util.h"
+
+using kudu::pb_util::SecureDebugString;
+using kudu::pb_util::SecureShortDebugString;
+using std::vector;
+using std::string;
+using strings::Substitute;
+
+namespace kudu {
+namespace master {
+
+TabletLoader::TabletLoader(CatalogManager* catalog_manager)
+ : catalog_manager_(catalog_manager) {
+}
+
+Status TabletLoader::VisitTablet(const string& table_id,
+ const string& tablet_id,
+ const SysTabletsEntryPB& metadata) {
+ // Lookup the table.
+ scoped_refptr<TableInfo> table(FindPtrOrNull(
+ catalog_manager_->table_ids_map_, table_id));
+ if (table == nullptr) {
+ // Tables and tablets are always created/deleted in one operation, so
+ // this shouldn't be possible.
+ string msg = Substitute("missing table $0 required by tablet $1 (metadata:
$2)",
+ table_id, tablet_id, SecureDebugString(metadata));
+ LOG(ERROR) << msg;
+ return Status::Corruption(msg);
+ }
+
+ // Set up the tablet info.
+ scoped_refptr<TabletInfo> tablet = new TabletInfo(table, tablet_id);
+ TabletMetadataLock l(tablet.get(), LockMode::WRITE);
+ l.mutable_data()->pb.CopyFrom(metadata);
+
+ // If the system catalog contains informartion on tablets created with
+ // Kudu of versions 1.16.0 and earlier, transform the legacy partition key
+ // representation into post-KUDU-2671 form. In essence, after KUDU-2671
+ // has been implemented, the hash-related part of the partition key for
+ // an unbounded range is built using slightly different rules by the
+ // PartitionSchema::UpdatePartitionBoundaries() method. For details, see
+ // https://github.com/apache/kudu/commit/8df970f7a652
+ if (ConvertFromLegacy(l.mutable_data()->pb.mutable_partition())) {
+ LOG(INFO) << Substitute(
+ "converted legacy boundary representation for tablet $0 (table $1)",
+ tablet_id, table->ToString());
+ }
+
+ // Add the tablet to the tablet manager.
+ catalog_manager_->tablet_map_[tablet->id()] = tablet;
+
+ // Add the tablet to the table.
+ bool is_deleted = l.mutable_data()->is_deleted();
+ l.Commit();
+ if (!is_deleted) {
+ // Need to use a new tablet lock here because AddRemoveTablets() reads
+ // from clean state, which is uninitialized for these brand new tablets.
+ TabletMetadataLock l(tablet.get(), LockMode::READ);
+ table->AddRemoveTablets({ tablet }, {});
+ LOG(INFO) << Substitute("loaded metadata for tablet $0 (table $1)",
+ tablet_id, table->ToString());
+ }
+
+ VLOG(2) << Substitute("metadata for tablet $0: $1",
+ tablet_id, SecureShortDebugString(metadata));
+ return Status::OK();
+}
+
+bool TabletLoader::ConvertFromLegacy(PartitionPB* p) {
+ if (p->hash_buckets_size() == 0) {
+ return false;
+ }
+
+ // Build std::vector out of PartitionPB.hash_buckets field.
+ vector<int32_t> hash_buckets(p->hash_buckets_size());
+ for (size_t i = 0; i < p->hash_buckets_size(); ++i) {
+ hash_buckets[i] = p->hash_buckets(i);
+ }
+
+ bool converted_lower = false;
+ auto* key_start_str = p->mutable_partition_key_start();
+ auto lower_bound = Partition::StringToPartitionKey(
+ *key_start_str, hash_buckets.size());
+ if (PartitionKey::IsLegacy(lower_bound, hash_buckets)) {
+ const PartitionKey orig(lower_bound);
+ lower_bound = PartitionKey::LowerBoundFromLegacy(
+ lower_bound, hash_buckets);
+ *key_start_str = lower_bound.ToString();
+ LOG(INFO) << Substitute("converted lower range boundary: '$0' --> '$1'",
+ orig.DebugString(), lower_bound.DebugString());
+ converted_lower = true;
+ }
+
+ bool converted_upper = false;
+ auto* key_end_str = p->mutable_partition_key_end();
+ auto upper_bound = Partition::StringToPartitionKey(
+ *key_end_str, hash_buckets.size());
+ if (PartitionKey::IsLegacy(upper_bound, hash_buckets)) {
+ const PartitionKey orig(upper_bound);
+ upper_bound = PartitionKey::UpperBoundFromLegacy(
+ upper_bound, hash_buckets);
+ LOG(INFO) << Substitute("converted lower upper boundary: '$0' --> '$1'",
+ orig.DebugString(), upper_bound.DebugString());
+ *key_end_str = upper_bound.ToString();
+ converted_upper = true;
+ }
+
+ return converted_lower || converted_upper;
+}
+
+} // namespace master
+} // namespace kudu
diff --git a/src/kudu/master/tablet_loader.h b/src/kudu/master/tablet_loader.h
new file mode 100644
index 000000000..337b60c0f
--- /dev/null
+++ b/src/kudu/master/tablet_loader.h
@@ -0,0 +1,67 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+#pragma once
+
+#include <string>
+
+#include <gtest/gtest_prod.h>
+
+#include "kudu/gutil/macros.h"
+#include "kudu/master/sys_catalog.h"
+#include "kudu/util/status.h"
+
+namespace kudu {
+
+class PartitionPB;
+
+namespace master {
+
+class CatalogManager;
+class SysTabletsEntryPB;
+
+////////////////////////////////////////////////////////////
+// Tablet Loader
+////////////////////////////////////////////////////////////
+
+class TabletLoader : public TabletVisitor {
+ public:
+ explicit TabletLoader(CatalogManager* catalog_manager);
+
+ Status VisitTablet(const std::string& table_id,
+ const std::string& tablet_id,
+ const SysTabletsEntryPB& metadata) override;
+ private:
+ FRIEND_TEST(SysCatalogTest, TabletRangesConversionFromLegacyFormat);
+
+ // Check if the stringified partition keys in the
+ // PartitionPB.{partition_key_start, partition_key_end} fields are in
+ // pre-KUDU-2671 (i.e. legacy) representation, and if so, then convert
+ // them into post-KUDU-2671 representation.
+ //
+ // For details, see https://github.com/apache/kudu/commit/8df970f7a652
+ //
+ // This method returns 'true' if lower or upper bound in the in-out parameter
+ // 'p' has been converted, 'false' otherwise.
+ static bool ConvertFromLegacy(PartitionPB* p);
+
+ CatalogManager* catalog_manager_;
+
+ DISALLOW_COPY_AND_ASSIGN(TabletLoader);
+};
+
+} // namespace master
+} // namespace kudu
