From 7eddfe670fd331363afd71466600efa4e05d3f67 Mon Sep 17 00:00:00 2001 From: lidezhu <47731263+lidezhu@users.noreply.github.com> Date: Tue, 8 Aug 2023 13:54:24 +0800 Subject: [PATCH 1/3] This is an automated cherry-pick of #7901 Signed-off-by: ti-chi-bot --- dbms/src/Storages/DeltaMerge/RowKeyRange.h | 40 +++++++++ .../DeltaMerge/tests/gtest_key_range.cpp | 84 +++++++++++++++++++ 2 files changed, 124 insertions(+) diff --git a/dbms/src/Storages/DeltaMerge/RowKeyRange.h b/dbms/src/Storages/DeltaMerge/RowKeyRange.h index 5b6d2db0210..bd1223e73d2 100644 --- a/dbms/src/Storages/DeltaMerge/RowKeyRange.h +++ b/dbms/src/Storages/DeltaMerge/RowKeyRange.h @@ -83,6 +83,46 @@ struct RowKeyValue { size_t cursor = 0; int_value = DB::DecodeInt64(cursor, *value); + if (unlikely(value->size() != sizeof(Int64))) + { + // For int type handle, the standard key enconding format should be t{table_id}_r{handle_value}. + // But TiKV may generate region range keys which are not strictly following the standard format. + // More concretely, the key may be t{table_id}_r{handle_value} + some other bytes. + // We need to adapt the key to the standard format. + // For example, the key may be t100_r1000 + 0x00, we need to adapt it to t100_r1001. + // This is ok, because + // 1) if the key is the start range, then [t100_r1000 + 0x00, xxx) has the same semantics with [t100_r1001, xxx) + // 2) if the key is the end range, then [xxx, t100_r1000 + 0x00) also has the same semantics with [xxx, t100_r1001) + // + // Note if the `int_value` is Int64::max_value, + // it is a value generated by tiflash itself to means +inf()(which is RowKeyValue::INT_HANDLE_MAX_KEY). + // So we can just ignore it. + if (value->size() != sizeof(UInt64) + 1 || value->back() != 0x00) + { + LOG_WARNING( + Logger::get(), + "Meet rowkey {} with unexpected encoding format", + Redact::keyToDebugString(value->data(), value->size())); + } + else + { + if (int_value < std::numeric_limits::max()) + { + LOG_WARNING( + Logger::get(), + "Meet rowkey {} which has an extra zero suffix", + Redact::keyToDebugString(value->data(), value->size())); + int_value = int_value + 1; + WriteBufferFromOwnString ss; + DB::EncodeInt64(int_value, ss); + value = std::make_shared(ss.releaseStr()); + } + else + { + // ignore RowKeyValue::INT_HANDLE_MAX_KEY + } + } + } } } diff --git a/dbms/src/Storages/DeltaMerge/tests/gtest_key_range.cpp b/dbms/src/Storages/DeltaMerge/tests/gtest_key_range.cpp index 802c047a1d7..b933c36733a 100644 --- a/dbms/src/Storages/DeltaMerge/tests/gtest_key_range.cpp +++ b/dbms/src/Storages/DeltaMerge/tests/gtest_key_range.cpp @@ -98,6 +98,90 @@ TEST(RowKeyRange_test, RedactRangeFromCommonHandle) Redact::setRedactLog(false); // restore flags } +<<<<<<< HEAD +======= +TEST(RowKey, ToNextKeyIntHandle) +{ + const auto key = RowKeyValue::fromHandle(20); + const auto next = key.toNext(); + EXPECT_EQ("21", next.toDebugString()); + + { + const auto expected_next_int = RowKeyValue::fromHandle(21); + EXPECT_EQ(0, compare(next.toRowKeyValueRef(), expected_next_int.toRowKeyValueRef())); + } + { + const auto range_keys = std::make_shared( + RecordKVFormat::genKey(1, 0), + RecordKVFormat::genKey(1, 21)); + const auto range = RowKeyRange::fromRegionRange( + range_keys, + /* table_id */ 1, + /* is_common_handle */ false, + /* row_key_column_size */ 1); + EXPECT_EQ(0, compare(next.toRowKeyValueRef(), range.getEnd())); + } + // Note: {20,00} will be regarded as Key=21 in RowKeyRange::fromRegionRange. + { + auto key_end = RecordKVFormat::genRawKey(1, 20); + key_end.push_back(0); + auto tikv_key_end = RecordKVFormat::encodeAsTiKVKey(key_end); + const auto range_keys = std::make_shared( + RecordKVFormat::genKey(1, 0), + std::move(tikv_key_end)); + const auto range = RowKeyRange::fromRegionRange( + range_keys, + /* table_id */ 1, + /* is_common_handle */ false, + /* row_key_column_size */ 1); + EXPECT_EQ(0, compare(next.toRowKeyValueRef(), range.getEnd())); + } +} + +TEST(RowKey, ToNextKeyCommonHandle) +{ + using namespace std::literals::string_literals; + + const auto key = RowKeyValue(/* is_common_handle */ true, std::make_shared("\xcc\xab"s), 0); + const auto next = key.toNext(); + EXPECT_EQ("CCAB00", next.toDebugString()); + + const auto my_next = RowKeyValue(/* is_common_handle */ true, std::make_shared("\xcc\xab\x00"s), 0); + EXPECT_EQ(0, compare(my_next.toRowKeyValueRef(), next.toRowKeyValueRef())); +} + +TEST(RowKey, NextIntHandleCompare) +{ + auto int_max = RowKeyValue::INT_HANDLE_MAX_KEY; + auto int_max_i64 = RowKeyValue::fromHandle(Handle(std::numeric_limits::max())); + + EXPECT_EQ(1, compare(int_max.toRowKeyValueRef(), int_max_i64.toRowKeyValueRef())); + + auto int_max_i64_pnext = int_max_i64.toPrefixNext(); + EXPECT_EQ(int_max, int_max_i64_pnext); + EXPECT_EQ(0, compare(int_max.toRowKeyValueRef(), int_max_i64_pnext.toRowKeyValueRef())); + EXPECT_EQ(0, compare(int_max_i64_pnext.toRowKeyValueRef(), int_max.toRowKeyValueRef())); + + auto int_max_i64_next = int_max_i64.toNext(); + EXPECT_EQ(int_max, int_max_i64_next); + EXPECT_EQ(0, compare(int_max.toRowKeyValueRef(), int_max_i64_next.toRowKeyValueRef())); + EXPECT_EQ(0, compare(int_max_i64_next.toRowKeyValueRef(), int_max.toRowKeyValueRef())); +} + +TEST(RowKey, NextIntHandleMinMax) +{ + auto v0 = RowKeyValue::fromHandle(Handle(1178400)); + auto v0_next = v0.toNext(); + auto v1 = RowKeyValue::fromHandle(Handle(1178401)); + + EXPECT_EQ(v0, min(v0, v1)); + EXPECT_EQ(v0, min(v0, v0_next)); + + EXPECT_EQ(v1, max(v0, v1)); + EXPECT_EQ(v1, max(v0, v0_next)); +} + +>>>>>>> 7dccf2c76b (fix row key not in standard format (#7901)) } // namespace tests } // namespace DM } // namespace DB From 72b4d3acf6de2a227abfba78cc1e628c1cb65a09 Mon Sep 17 00:00:00 2001 From: lidezhu Date: Tue, 8 Aug 2023 14:30:55 +0800 Subject: [PATCH 2/3] fix conflict --- .../DeltaMerge/tests/gtest_key_range.cpp | 94 +++---------------- 1 file changed, 13 insertions(+), 81 deletions(-) diff --git a/dbms/src/Storages/DeltaMerge/tests/gtest_key_range.cpp b/dbms/src/Storages/DeltaMerge/tests/gtest_key_range.cpp index b933c36733a..8e3410fcd52 100644 --- a/dbms/src/Storages/DeltaMerge/tests/gtest_key_range.cpp +++ b/dbms/src/Storages/DeltaMerge/tests/gtest_key_range.cpp @@ -98,90 +98,22 @@ TEST(RowKeyRange_test, RedactRangeFromCommonHandle) Redact::setRedactLog(false); // restore flags } -<<<<<<< HEAD -======= -TEST(RowKey, ToNextKeyIntHandle) +TEST(RowKey, DecodeKeyWithExtraZeroSuffix) { - const auto key = RowKeyValue::fromHandle(20); - const auto next = key.toNext(); - EXPECT_EQ("21", next.toDebugString()); - - { - const auto expected_next_int = RowKeyValue::fromHandle(21); - EXPECT_EQ(0, compare(next.toRowKeyValueRef(), expected_next_int.toRowKeyValueRef())); - } - { - const auto range_keys = std::make_shared( - RecordKVFormat::genKey(1, 0), - RecordKVFormat::genKey(1, 21)); - const auto range = RowKeyRange::fromRegionRange( - range_keys, - /* table_id */ 1, - /* is_common_handle */ false, - /* row_key_column_size */ 1); - EXPECT_EQ(0, compare(next.toRowKeyValueRef(), range.getEnd())); - } // Note: {20,00} will be regarded as Key=21 in RowKeyRange::fromRegionRange. - { - auto key_end = RecordKVFormat::genRawKey(1, 20); - key_end.push_back(0); - auto tikv_key_end = RecordKVFormat::encodeAsTiKVKey(key_end); - const auto range_keys = std::make_shared( - RecordKVFormat::genKey(1, 0), - std::move(tikv_key_end)); - const auto range = RowKeyRange::fromRegionRange( - range_keys, - /* table_id */ 1, - /* is_common_handle */ false, - /* row_key_column_size */ 1); - EXPECT_EQ(0, compare(next.toRowKeyValueRef(), range.getEnd())); - } -} - -TEST(RowKey, ToNextKeyCommonHandle) -{ - using namespace std::literals::string_literals; - - const auto key = RowKeyValue(/* is_common_handle */ true, std::make_shared("\xcc\xab"s), 0); - const auto next = key.toNext(); - EXPECT_EQ("CCAB00", next.toDebugString()); - - const auto my_next = RowKeyValue(/* is_common_handle */ true, std::make_shared("\xcc\xab\x00"s), 0); - EXPECT_EQ(0, compare(my_next.toRowKeyValueRef(), next.toRowKeyValueRef())); + auto key_end = RecordKVFormat::genRawKey(1, 20); + key_end.push_back(0); + auto tikv_key_end = RecordKVFormat::encodeAsTiKVKey(key_end); + const auto range_keys = std::make_shared( + RecordKVFormat::genKey(1, 0), + std::move(tikv_key_end)); + const auto range = RowKeyRange::fromRegionRange( + range_keys, + /* table_id */ 1, + /* is_common_handle */ false, + /* row_key_column_size */ 1); + EXPECT_EQ(0, compare(RowKeyValue::fromHandle(21).toRowKeyValueRef(), range.getEnd())); } - -TEST(RowKey, NextIntHandleCompare) -{ - auto int_max = RowKeyValue::INT_HANDLE_MAX_KEY; - auto int_max_i64 = RowKeyValue::fromHandle(Handle(std::numeric_limits::max())); - - EXPECT_EQ(1, compare(int_max.toRowKeyValueRef(), int_max_i64.toRowKeyValueRef())); - - auto int_max_i64_pnext = int_max_i64.toPrefixNext(); - EXPECT_EQ(int_max, int_max_i64_pnext); - EXPECT_EQ(0, compare(int_max.toRowKeyValueRef(), int_max_i64_pnext.toRowKeyValueRef())); - EXPECT_EQ(0, compare(int_max_i64_pnext.toRowKeyValueRef(), int_max.toRowKeyValueRef())); - - auto int_max_i64_next = int_max_i64.toNext(); - EXPECT_EQ(int_max, int_max_i64_next); - EXPECT_EQ(0, compare(int_max.toRowKeyValueRef(), int_max_i64_next.toRowKeyValueRef())); - EXPECT_EQ(0, compare(int_max_i64_next.toRowKeyValueRef(), int_max.toRowKeyValueRef())); -} - -TEST(RowKey, NextIntHandleMinMax) -{ - auto v0 = RowKeyValue::fromHandle(Handle(1178400)); - auto v0_next = v0.toNext(); - auto v1 = RowKeyValue::fromHandle(Handle(1178401)); - - EXPECT_EQ(v0, min(v0, v1)); - EXPECT_EQ(v0, min(v0, v0_next)); - - EXPECT_EQ(v1, max(v0, v1)); - EXPECT_EQ(v1, max(v0, v0_next)); -} - ->>>>>>> 7dccf2c76b (fix row key not in standard format (#7901)) } // namespace tests } // namespace DM } // namespace DB From a32540f7c78ddb368ceb21b33550b774819ff8fb Mon Sep 17 00:00:00 2001 From: lidezhu Date: Tue, 8 Aug 2023 15:02:48 +0800 Subject: [PATCH 3/3] fix build --- dbms/src/Storages/DeltaMerge/RowKeyRange.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/dbms/src/Storages/DeltaMerge/RowKeyRange.h b/dbms/src/Storages/DeltaMerge/RowKeyRange.h index bd1223e73d2..d7db029b8ef 100644 --- a/dbms/src/Storages/DeltaMerge/RowKeyRange.h +++ b/dbms/src/Storages/DeltaMerge/RowKeyRange.h @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -25,6 +26,7 @@ #include #include #include +#include namespace DB::DM { @@ -99,8 +101,8 @@ struct RowKeyValue // So we can just ignore it. if (value->size() != sizeof(UInt64) + 1 || value->back() != 0x00) { - LOG_WARNING( - Logger::get(), + LOG_FMT_WARNING( + &Poco::Logger::get("RowKeyValue"), "Meet rowkey {} with unexpected encoding format", Redact::keyToDebugString(value->data(), value->size())); } @@ -108,8 +110,8 @@ struct RowKeyValue { if (int_value < std::numeric_limits::max()) { - LOG_WARNING( - Logger::get(), + LOG_FMT_WARNING( + &Poco::Logger::get("RowKeyValue"), "Meet rowkey {} which has an extra zero suffix", Redact::keyToDebugString(value->data(), value->size())); int_value = int_value + 1;