From 0784d4c02a305ecc8b71d8daf92c5d6dbefc4f9f Mon Sep 17 00:00:00 2001 From: Calvin Neo Date: Tue, 30 May 2023 12:56:42 +0800 Subject: [PATCH] Add tests for learner read (#7549) ref pingcap/tiflash#7256 --- dbms/src/Debug/MockRaftStoreProxy.cpp | 9 +++ dbms/src/Debug/MockRaftStoreProxy.h | 16 +++- dbms/src/Debug/MockSSTReader.h | 15 ---- dbms/src/Debug/dbgFuncMockRaftSnapshot.cpp | 15 ++++ dbms/src/Storages/Transaction/KVStore.h | 3 +- dbms/src/Storages/Transaction/LearnerRead.cpp | 17 +++-- dbms/src/Storages/Transaction/Region.cpp | 6 +- dbms/src/Storages/Transaction/Region.h | 8 +- .../Storages/Transaction/RegionCFDataBase.h | 3 +- .../Storages/Transaction/RegionRangeKeys.h | 8 ++ dbms/src/Storages/Transaction/RegionState.cpp | 5 ++ dbms/src/Storages/Transaction/RegionTable.cpp | 4 +- .../Transaction/RegionsRangeIndex.cpp | 13 +++- .../Storages/Transaction/RegionsRangeIndex.h | 9 ++- dbms/src/Storages/Transaction/TMTContext.cpp | 4 + dbms/src/Storages/Transaction/TMTContext.h | 5 ++ .../Transaction/tests/gtest_kvstore.cpp | 37 ++++++---- .../Transaction/tests/gtest_new_kvstore.cpp | 74 ++++++++++++++++++- .../Transaction/tests/kvstore_helper.h | 40 +++++++++- 19 files changed, 228 insertions(+), 63 deletions(-) diff --git a/dbms/src/Debug/MockRaftStoreProxy.cpp b/dbms/src/Debug/MockRaftStoreProxy.cpp index c0df7514cee..40d0856739f 100644 --- a/dbms/src/Debug/MockRaftStoreProxy.cpp +++ b/dbms/src/Debug/MockRaftStoreProxy.cpp @@ -324,6 +324,15 @@ void MockRaftStoreProxy::init(size_t region_num) } } +std::unique_ptr MockRaftStoreProxy::generateProxyHelper() +{ + auto proxy_helper = std::make_unique(MockRaftStoreProxy::SetRaftStoreProxyFFIHelper( + RaftStoreProxyPtr{this})); + // Bind ffi to MockSSTReader. + proxy_helper->sst_reader_interfaces = make_mock_sst_reader_interface(); + return proxy_helper; +} + size_t MockRaftStoreProxy::size() const { auto _ = genLockGuard(); diff --git a/dbms/src/Debug/MockRaftStoreProxy.h b/dbms/src/Debug/MockRaftStoreProxy.h index 6cbb070ceb2..8e0ab487f81 100644 --- a/dbms/src/Debug/MockRaftStoreProxy.h +++ b/dbms/src/Debug/MockRaftStoreProxy.h @@ -152,12 +152,12 @@ struct MockRaftStoreProxy : MutexLockWrap } MockProxyRegionPtr getRegion(uint64_t id); - MockProxyRegionPtr doGetRegion(uint64_t id); MockReadIndexTask * makeReadIndexTask(kvrpcpb::ReadIndexRequest req); void init(size_t region_num); + std::unique_ptr generateProxyHelper(); size_t size() const; @@ -332,4 +332,18 @@ struct GCMonitor : MutexLockWrap static GCMonitor global_gc_monitor; }; +template +std::vector> regionRangeToEncodeKeys(Types &&... args) +{ + // RegionRangeKeys::RegionRange is not copy-constructible, however, initialize_list need copy construction. + // So we have to so this way, rather than create a composeXXX that accepts a vector of RegionRangeKeys::RegionRange. + std::vector> ranges_str; + ([&] { + auto & x = args; + ranges_str.emplace_back(std::make_pair(x.first.toString(), x.second.toString())); + }(), + ...); + return ranges_str; +} + } // namespace DB diff --git a/dbms/src/Debug/MockSSTReader.h b/dbms/src/Debug/MockSSTReader.h index 319d3587e2c..d9f962823c5 100644 --- a/dbms/src/Debug/MockSSTReader.h +++ b/dbms/src/Debug/MockSSTReader.h @@ -114,19 +114,4 @@ struct MockSSTReader }; SSTReaderInterfaces make_mock_sst_reader_interface(); - -class RegionMockTest final -{ -public: - RegionMockTest(KVStore * kvstore_, RegionPtr region_); - ~RegionMockTest(); - - DISALLOW_COPY_AND_MOVE(RegionMockTest); - -private: - TiFlashRaftProxyHelper mock_proxy_helper{}; - const TiFlashRaftProxyHelper * ori_proxy_helper{}; - KVStore * kvstore; - RegionPtr region; -}; } // namespace DB diff --git a/dbms/src/Debug/dbgFuncMockRaftSnapshot.cpp b/dbms/src/Debug/dbgFuncMockRaftSnapshot.cpp index c613238316a..fbf52f920af 100644 --- a/dbms/src/Debug/dbgFuncMockRaftSnapshot.cpp +++ b/dbms/src/Debug/dbgFuncMockRaftSnapshot.cpp @@ -262,6 +262,21 @@ void MockRaftCommand::dbgFuncRegionSnapshot(Context & context, const ASTs & args std::map MockSSTReader::MockSSTData; +class RegionMockTest final +{ +public: + RegionMockTest(KVStore * kvstore_, RegionPtr region_); + ~RegionMockTest(); + + DISALLOW_COPY_AND_MOVE(RegionMockTest); + +private: + TiFlashRaftProxyHelper mock_proxy_helper{}; + const TiFlashRaftProxyHelper * ori_proxy_helper{}; + KVStore * kvstore; + RegionPtr region; +}; + RegionMockTest::RegionMockTest(KVStore * kvstore_, RegionPtr region_) : kvstore(kvstore_) , region(region_) diff --git a/dbms/src/Storages/Transaction/KVStore.h b/dbms/src/Storages/Transaction/KVStore.h index 0ece457eace..1f1f631e3fd 100644 --- a/dbms/src/Storages/Transaction/KVStore.h +++ b/dbms/src/Storages/Transaction/KVStore.h @@ -16,6 +16,7 @@ #include #include +#include #include namespace TiDB @@ -85,7 +86,7 @@ class KVStore final : private boost::noncopyable RegionPtr getRegion(RegionID region_id) const; - using RegionRange = std::pair; + using RegionRange = RegionRangeKeys::RegionRange; RegionMap getRegionsByRangeOverlap(const RegionRange & range) const; diff --git a/dbms/src/Storages/Transaction/LearnerRead.cpp b/dbms/src/Storages/Transaction/LearnerRead.cpp index 4905d9eaffc..3d912095e1c 100644 --- a/dbms/src/Storages/Transaction/LearnerRead.cpp +++ b/dbms/src/Storages/Transaction/LearnerRead.cpp @@ -96,7 +96,8 @@ class MvccQueryInfoWrap using Base = MvccQueryInfo; Base & inner; std::optional regions_info; - Base::RegionsQueryInfo * regions_info_ptr; + // Points to either `regions_info` or `mvcc_query_info.regions_query_info`. + Base::RegionsQueryInfo * regions_query_info_ptr; public: MvccQueryInfoWrap(Base & mvcc_query_info, TMTContext & tmt, const TiDB::TableID logical_table_id) @@ -104,28 +105,28 @@ class MvccQueryInfoWrap { if (likely(!inner.regions_query_info.empty())) { - regions_info_ptr = &inner.regions_query_info; + regions_query_info_ptr = &inner.regions_query_info; } else { regions_info = Base::RegionsQueryInfo(); - regions_info_ptr = &*regions_info; - // Only for test, because regions_query_info should never be empty if query is from TiDB or TiSpark. + regions_query_info_ptr = &*regions_info; + // Only for (integration) test, because regions_query_info should never be empty if query is from TiDB or TiSpark. // todo support partition table auto regions = tmt.getRegionTable().getRegionsByTable(NullspaceID, logical_table_id); - regions_info_ptr->reserve(regions.size()); + regions_query_info_ptr->reserve(regions.size()); for (const auto & [id, region] : regions) { if (region == nullptr) continue; - regions_info_ptr->emplace_back( + regions_query_info_ptr->emplace_back( RegionQueryInfo{id, region->version(), region->confVer(), logical_table_id, region->getRange()->rawKeys(), {}}); } } } Base * operator->() { return &inner; } - const Base::RegionsQueryInfo & getRegionsInfo() const { return *regions_info_ptr; } + const Base::RegionsQueryInfo & getRegionsInfo() const { return *regions_query_info_ptr; } void addReadIndexRes(RegionID region_id, UInt64 read_index) { inner.read_index_res[region_id] = read_index; @@ -228,6 +229,7 @@ LearnerReadSnapshot doLearnerRead( } } } + GET_METRIC(tiflash_stale_read_count).Increment(stats.num_stale_read); GET_METRIC(tiflash_raft_read_index_count).Increment(batch_read_index_req.size()); @@ -457,6 +459,7 @@ void validateQueryInfo( if (auto iter = regions_snapshot.find(region_query_info.region_id); // iter == regions_snapshot.end() || iter->second != region) { + // If snapshot is applied during learner read, we should abort with an exception later. status = RegionException::RegionReadStatus::NOT_FOUND; } else if (region->version() != region_query_info.version) diff --git a/dbms/src/Storages/Transaction/Region.cpp b/dbms/src/Storages/Transaction/Region.cpp index 0c7bea70b12..e4654c0db88 100644 --- a/dbms/src/Storages/Transaction/Region.cpp +++ b/dbms/src/Storages/Transaction/Region.cpp @@ -533,9 +533,10 @@ std::tuple Region::waitIndex(UInt64 index, const UInt64 { Stopwatch wait_index_watch; LOG_DEBUG(log, - "{} need to wait learner index {}", + "{} need to wait learner index {} timeout {}", toString(), - index); + index, + timeout_ms); auto wait_idx_res = meta.waitIndex(index, timeout_ms, std::move(check_running)); auto elapsed_secs = wait_index_watch.elapsedSeconds(); switch (wait_idx_res) @@ -579,7 +580,6 @@ void Region::assignRegion(Region && new_region) std::unique_lock lock(mutex); data.assignRegionData(std::move(new_region.data)); - meta.assignRegionMeta(std::move(new_region.meta)); meta.notifyAll(); } diff --git a/dbms/src/Storages/Transaction/Region.h b/dbms/src/Storages/Transaction/Region.h index ff31dfd83bb..b87a586a990 100644 --- a/dbms/src/Storages/Transaction/Region.h +++ b/dbms/src/Storages/Transaction/Region.h @@ -161,6 +161,7 @@ class Region : public std::enable_shared_from_this return region1.meta == region2.meta && region1.data == region2.data; } + // Check if we can read by this index. bool checkIndex(UInt64 index) const; // Return for wait-index. @@ -178,14 +179,9 @@ class Region : public std::enable_shared_from_this RegionMetaSnapshot dumpRegionMetaSnapshot() const; + // Assign data and meta by moving from `new_region`. void assignRegion(Region && new_region); - using HandleMap = std::unordered_map>; - - /// Only can be used for applying snapshot. only can be called by single thread. - /// Try to fill record with delmark if it exists in ch but has been remove by GC in leader. - void compareAndCompleteSnapshot(HandleMap & handle_map, const Timestamp safe_point); - void tryCompactionFilter(const Timestamp safe_point); RegionRaftCommandDelegate & makeRaftCommandDelegate(const KVStoreTaskLock &); diff --git a/dbms/src/Storages/Transaction/RegionCFDataBase.h b/dbms/src/Storages/Transaction/RegionCFDataBase.h index 985ea640151..dd17a86a416 100644 --- a/dbms/src/Storages/Transaction/RegionCFDataBase.h +++ b/dbms/src/Storages/Transaction/RegionCFDataBase.h @@ -14,6 +14,7 @@ #pragma once +#include #include #include @@ -22,7 +23,7 @@ namespace DB { struct TiKVRangeKey; -using RegionRange = std::pair; +using RegionRange = RegionRangeKeys::RegionRange; using RegionDataRes = size_t; enum class DupCheck diff --git a/dbms/src/Storages/Transaction/RegionRangeKeys.h b/dbms/src/Storages/Transaction/RegionRangeKeys.h index 9dddbac8206..afde5d4c8e3 100644 --- a/dbms/src/Storages/Transaction/RegionRangeKeys.h +++ b/dbms/src/Storages/Transaction/RegionRangeKeys.h @@ -41,6 +41,10 @@ struct TiKVRangeKey : boost::noncopyable TiKVRangeKey copy() const; TiKVRangeKey & operator=(TiKVRangeKey &&); std::string toDebugString() const; + std::string toString() const + { + return key.toString(); + } State state; TiKVKey key; @@ -53,9 +57,13 @@ class RegionRangeKeys : boost::noncopyable using RegionRange = std::pair; const RegionRange & comparableKeys() const; + static RegionRange cloneRange(const RegionRange & from); static RegionRange makeComparableKeys(TiKVKey && start_key, TiKVKey && end_key); const std::pair & rawKeys() const; explicit RegionRangeKeys(TiKVKey && start_key, TiKVKey && end_key); + explicit RegionRangeKeys(RegionRange && range) + : RegionRangeKeys(std::move(range.first.key), std::move(range.second.key)) + {} TableID getMappedTableID() const; KeyspaceID getKeyspaceID() const; std::string toDebugString() const; diff --git a/dbms/src/Storages/Transaction/RegionState.cpp b/dbms/src/Storages/Transaction/RegionState.cpp index 9d7ada2aa21..de71ccca540 100644 --- a/dbms/src/Storages/Transaction/RegionState.cpp +++ b/dbms/src/Storages/Transaction/RegionState.cpp @@ -190,6 +190,11 @@ RegionRangeKeys::RegionRange RegionRangeKeys::makeComparableKeys(TiKVKey && star TiKVRangeKey::makeTiKVRangeKey(std::move(end_key))); } +RegionRangeKeys::RegionRange RegionRangeKeys::cloneRange(const RegionRange & from) +{ + return std::make_pair(from.first.copy(), from.second.copy()); +} + std::string TiKVRangeKey::toDebugString() const { if (this->state == MAX) diff --git a/dbms/src/Storages/Transaction/RegionTable.cpp b/dbms/src/Storages/Transaction/RegionTable.cpp index c85d95ebe81..ce8b36adb6a 100644 --- a/dbms/src/Storages/Transaction/RegionTable.cpp +++ b/dbms/src/Storages/Transaction/RegionTable.cpp @@ -46,7 +46,7 @@ RegionTable::Table & RegionTable::getOrCreateTable(const KeyspaceID keyspace_id, { // Load persisted info. it = tables.emplace(ks_tb_id, table_id).first; - LOG_INFO(log, "get new table {}", table_id); + LOG_INFO(log, "get new table {} of keyspace {}", table_id, keyspace_id); } return it->second; } @@ -426,7 +426,9 @@ void RegionTable::handleInternalRegionsByTable(const KeyspaceID keyspace_id, con std::lock_guard lock(mutex); if (auto it = tables.find(KeyspaceTableID{keyspace_id, table_id}); it != tables.end()) + { callback(it->second.regions); + } } std::vector> RegionTable::getRegionsByTable(const KeyspaceID keyspace_id, const TableID table_id) const diff --git a/dbms/src/Storages/Transaction/RegionsRangeIndex.cpp b/dbms/src/Storages/Transaction/RegionsRangeIndex.cpp index a2eeceed464..8bcb647dff3 100644 --- a/dbms/src/Storages/Transaction/RegionsRangeIndex.cpp +++ b/dbms/src/Storages/Transaction/RegionsRangeIndex.cpp @@ -24,10 +24,10 @@ bool TiKVRangeKeyCmp::operator()(const TiKVRangeKey & x, const TiKVRangeKey & y) void RegionsRangeIndex::add(const RegionPtr & new_region) { - auto region_range = new_region->getRange(); - const auto & new_range = region_range->comparableKeys(); - auto begin_it = split(new_range.first); - auto end_it = split(new_range.second); + auto new_region_range = new_region->getRange(); + const auto & new_range_keys = new_region_range->comparableKeys(); + auto begin_it = split(new_range_keys.first); + auto end_it = split(new_range_keys.second); if (begin_it == end_it) throw Exception( std::string(__PRETTY_FUNCTION__) + ": range of region " + toString(new_region->id()) + " is empty", @@ -90,6 +90,11 @@ void RegionsRangeIndex::clear() max_it = root.emplace(TiKVRangeKey::makeTiKVRangeKey(TiKVKey()), IndexNode{}).first; } +void RegionsRangeIndex::tryMergeEmpty() +{ + tryMergeEmpty(root.begin()); +} + void RegionsRangeIndex::tryMergeEmpty(RootMap::iterator remove_it) { if (!remove_it->second.region_map.empty()) diff --git a/dbms/src/Storages/Transaction/RegionsRangeIndex.h b/dbms/src/Storages/Transaction/RegionsRangeIndex.h index 671053a9bd4..b4ccb6fee11 100644 --- a/dbms/src/Storages/Transaction/RegionsRangeIndex.h +++ b/dbms/src/Storages/Transaction/RegionsRangeIndex.h @@ -14,19 +14,19 @@ #pragma once +#include #include #include namespace DB { - class Region; using RegionPtr = std::shared_ptr; using RegionMap = std::unordered_map; struct TiKVRangeKey; -using RegionRange = std::pair; +using RegionRange = RegionRangeKeys::RegionRange; struct TiKVRangeKeyCmp { @@ -55,9 +55,12 @@ class RegionsRangeIndex : private boost::noncopyable void clear(); + // TODO Used by RegionKVStoreTest, using a friend decl here. + RootMap::iterator split(const TiKVRangeKey & new_start); + void tryMergeEmpty(); + private: void tryMergeEmpty(RootMap::iterator remove_it); - RootMap::iterator split(const TiKVRangeKey & new_start); private: RootMap root; diff --git a/dbms/src/Storages/Transaction/TMTContext.cpp b/dbms/src/Storages/Transaction/TMTContext.cpp index 523af11353f..b1de0210ea0 100644 --- a/dbms/src/Storages/Transaction/TMTContext.cpp +++ b/dbms/src/Storages/Transaction/TMTContext.cpp @@ -354,6 +354,10 @@ UInt64 TMTContext::waitIndexTimeout() const { return wait_index_timeout_ms.load(std::memory_order_relaxed); } +void TMTContext::debugSetWaitIndexTimeout(UInt64 timeout) +{ + return wait_index_timeout_ms.store(timeout, std::memory_order_relaxed); +} Int64 TMTContext::waitRegionReadyTimeout() const { return wait_region_ready_timeout_sec.load(std::memory_order_relaxed); diff --git a/dbms/src/Storages/Transaction/TMTContext.h b/dbms/src/Storages/Transaction/TMTContext.h index f8f2591ac46..d4c08ab5864 100644 --- a/dbms/src/Storages/Transaction/TMTContext.h +++ b/dbms/src/Storages/Transaction/TMTContext.h @@ -80,6 +80,10 @@ class TMTContext : private boost::noncopyable public: const KVStorePtr & getKVStore() const; KVStorePtr & getKVStore(); + void debugSetKVStore(const KVStorePtr & new_kvstore) + { + kvstore = new_kvstore; + } const ManagedStorages & getStorages() const; ManagedStorages & getStorages(); @@ -137,6 +141,7 @@ class TMTContext : private boost::noncopyable UInt64 batchReadIndexTimeout() const; // timeout for wait index (ms). "0" means wait infinitely UInt64 waitIndexTimeout() const; + void debugSetWaitIndexTimeout(UInt64 timeout); Int64 waitRegionReadyTimeout() const; uint64_t readIndexWorkerTick() const; diff --git a/dbms/src/Storages/Transaction/tests/gtest_kvstore.cpp b/dbms/src/Storages/Transaction/tests/gtest_kvstore.cpp index ca1557038db..775c7705def 100644 --- a/dbms/src/Storages/Transaction/tests/gtest_kvstore.cpp +++ b/dbms/src/Storages/Transaction/tests/gtest_kvstore.cpp @@ -55,7 +55,7 @@ TEST_F(RegionKVStoreTest, ReadIndex) createDefaultRegions(); auto ctx = TiFlashTestEnv::getGlobalContext(); - // start mock proxy in other thread + // Start mock proxy in other thread std::atomic_bool over{false}; auto proxy_runner = std::thread([&]() { proxy_instance->testRunNormal(over); @@ -316,15 +316,18 @@ void RegionKVStoreTest::testRaftSplit(KVStore & kvs, TMTContext & tmt) RegionID region_id2 = 7; auto source_region = kvs.getRegion(region_id); auto old_epoch = source_region->mutMeta().getMetaRegion().region_epoch(); - auto && [request, response] = MockRaftStoreProxy::composeBatchSplit({region_id, region_id2}, {{RecordKVFormat::genKey(1, 5), RecordKVFormat::genKey(1, 10)}, {RecordKVFormat::genKey(1, 0), RecordKVFormat::genKey(1, 5)}}, old_epoch); + auto & ori_source_range = source_region->getRange()->comparableKeys(); + RegionRangeKeys::RegionRange new_source_range = RegionRangeKeys::makeComparableKeys(RecordKVFormat::genKey(1, 5), RecordKVFormat::genKey(1, 10)); + RegionRangeKeys::RegionRange new_target_range = RegionRangeKeys::makeComparableKeys(RecordKVFormat::genKey(1, 0), RecordKVFormat::genKey(1, 5)); + auto && [request, response] = MockRaftStoreProxy::composeBatchSplit({region_id, region_id2}, regionRangeToEncodeKeys(new_source_range, new_target_range), old_epoch); kvs.handleAdminRaftCmd(raft_cmdpb::AdminRequest(request), raft_cmdpb::AdminResponse(response), 1, 20, 5, tmt); { - auto mmp = kvs.getRegionsByRangeOverlap(RegionRangeKeys::makeComparableKeys(RecordKVFormat::genKey(1, 0), RecordKVFormat::genKey(1, 5))); + auto mmp = kvs.getRegionsByRangeOverlap(new_target_range); ASSERT_TRUE(mmp.count(7) != 0); ASSERT_EQ(mmp.size(), 1); } { - auto mmp = kvs.getRegionsByRangeOverlap(RegionRangeKeys::makeComparableKeys(RecordKVFormat::genKey(1, 5), RecordKVFormat::genKey(1, 10))); + auto mmp = kvs.getRegionsByRangeOverlap(new_source_range); ASSERT_TRUE(mmp.count(1) != 0); ASSERT_EQ(mmp.size(), 1); } @@ -340,7 +343,7 @@ void RegionKVStoreTest::testRaftSplit(KVStore & kvs, TMTContext & tmt) { auto task_lock = kvs.genTaskLock(); auto lock = kvs.genRegionWriteLock(task_lock); - auto region = makeRegion(1, RecordKVFormat::genKey(1, 0), RecordKVFormat::genKey(1, 10)); + auto region = makeRegion(1, ori_source_range.first.key, ori_source_range.second.key); lock.regions.emplace(1, region); lock.index.add(region); } @@ -357,7 +360,7 @@ void RegionKVStoreTest::testRaftSplit(KVStore & kvs, TMTContext & tmt) } { // Region 1 and 7 overlaps. - auto mmp = kvs.getRegionsByRangeOverlap(RegionRangeKeys::makeComparableKeys(RecordKVFormat::genKey(1, 0), RecordKVFormat::genKey(1, 5))); + auto mmp = kvs.getRegionsByRangeOverlap(new_target_range); ASSERT_TRUE(mmp.count(7) != 0); ASSERT_TRUE(mmp.count(1) != 0); ASSERT_EQ(mmp.size(), 2); @@ -365,12 +368,12 @@ void RegionKVStoreTest::testRaftSplit(KVStore & kvs, TMTContext & tmt) // Split again kvs.handleAdminRaftCmd(raft_cmdpb::AdminRequest(request), raft_cmdpb::AdminResponse(response), 1, 20, 5, tmt); { - auto mmp = kvs.getRegionsByRangeOverlap(RegionRangeKeys::makeComparableKeys(RecordKVFormat::genKey(1, 0), RecordKVFormat::genKey(1, 5))); + auto mmp = kvs.getRegionsByRangeOverlap(new_target_range); ASSERT_TRUE(mmp.count(7) != 0); ASSERT_EQ(mmp.size(), 1); } { - auto mmp = kvs.getRegionsByRangeOverlap(RegionRangeKeys::makeComparableKeys(RecordKVFormat::genKey(1, 5), RecordKVFormat::genKey(1, 10))); + auto mmp = kvs.getRegionsByRangeOverlap(new_source_range); ASSERT_TRUE(mmp.count(1) != 0); ASSERT_EQ(mmp.size(), 1); } @@ -850,6 +853,7 @@ try auto table_id = 101; auto region_id = 19; auto region_id_str = std::to_string(region_id); + ASSERT_NE(proxy_helper->sst_reader_interfaces.fn_key, nullptr); auto settings_backup = ctx.getGlobalContext().getSettings(); ctx.getGlobalContext().getSettingsRef().dt_segment_limit_rows = 50; @@ -897,8 +901,6 @@ try }, }; { - RegionMockTest mock_test(kvstore.get(), region); - kvs.handleApplySnapshot( region->cloneMetaRegion(), 2, @@ -933,8 +935,6 @@ try }, }; { - RegionMockTest mock_test(kvstore.get(), region); - kvs.handleApplySnapshot( region->cloneMetaRegion(), 2, @@ -983,8 +983,8 @@ try createDefaultRegions(); auto ctx = TiFlashTestEnv::getGlobalContext(); KVStore & kvs = getKVS(); - // In this test we only deal with meta, - ASSERT_EQ(proxy_helper->sst_reader_interfaces.fn_key, nullptr); + // In this test we only deal with meta though, + ASSERT_NE(proxy_helper->sst_reader_interfaces.fn_key, nullptr); { auto region_id = 19; auto region = makeRegion(region_id, RecordKVFormat::genKey(1, 50), RecordKVFormat::genKey(1, 60)); @@ -1167,6 +1167,15 @@ TEST_F(RegionKVStoreTest, Restore) TEST_F(RegionKVStoreTest, RegionRange) { + { + // Test util functions. + RegionsRangeIndex region_index; + region_index.split(TiKVRangeKey::makeTiKVRangeKey(TiKVKey())); + region_index.split(TiKVRangeKey::makeTiKVRangeKey(TiKVKey())); + region_index.tryMergeEmpty(); + const auto & root_map = region_index.getRoot(); + ASSERT_EQ(root_map.size(), 2); + } { // Test findByRangeOverlap. RegionsRangeIndex region_index; diff --git a/dbms/src/Storages/Transaction/tests/gtest_new_kvstore.cpp b/dbms/src/Storages/Transaction/tests/gtest_new_kvstore.cpp index 17745af4a21..5e7bc1899ef 100644 --- a/dbms/src/Storages/Transaction/tests/gtest_new_kvstore.cpp +++ b/dbms/src/Storages/Transaction/tests/gtest_new_kvstore.cpp @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include + #include "kvstore_helper.h" namespace DB @@ -305,10 +307,7 @@ static void validate(KVStore & kvs, std::unique_ptr & proxy_ { auto kvr1 = kvs.getRegion(region_id); auto r1 = proxy_instance->getRegion(region_id); - auto proxy_helper = std::make_unique(MockRaftStoreProxy::SetRaftStoreProxyFFIHelper( - RaftStoreProxyPtr{proxy_instance.get()})); - // Bind ffi to MockSSTReader. - proxy_helper->sst_reader_interfaces = make_mock_sst_reader_interface(); + auto proxy_helper = proxy_instance->generateProxyHelper(); auto ssts = cf_data.ssts(); ASSERT_EQ(ssts.size(), sst_size); auto make_inner_func = [](const TiFlashRaftProxyHelper * proxy_helper, SSTView snap, SSTReader::RegionRangeFilter range) -> std::unique_ptr { @@ -337,6 +336,7 @@ TEST_F(RegionKVStoreTest, KVStoreSnapshotV1) try { auto ctx = TiFlashTestEnv::getGlobalContext(); + ASSERT_NE(proxy_helper->sst_reader_interfaces.fn_key, nullptr); { UInt64 region_id = 1; TableID table_id; @@ -511,6 +511,7 @@ TEST_F(RegionKVStoreTest, KVStoreSnapshotV2) try { auto ctx = TiFlashTestEnv::getGlobalContext(); + ASSERT_NE(proxy_helper->sst_reader_interfaces.fn_key, nullptr); UInt64 region_id = 1; TableID table_id; { @@ -576,5 +577,70 @@ try } CATCH +TEST_F(RegionKVStoreTest, LearnerRead) +try +{ + auto ctx = TiFlashTestEnv::getGlobalContext(); + auto region_id = 1; + KVStore & kvs = getKVS(); + ctx.getTMTContext().debugSetKVStore(kvstore); + initStorages(); + + ctx.getTMTContext().debugSetWaitIndexTimeout(1); + + startReadIndexUtils(ctx); + SCOPE_EXIT({ + stopReadIndexUtils(); + }); + + auto table_id = proxy_instance->bootstrapTable(ctx, kvs, ctx.getTMTContext()); + proxy_instance->bootstrapWithRegion(kvs, ctx.getTMTContext(), region_id, std::nullopt); + auto kvr1 = kvs.getRegion(region_id); + ctx.getTMTContext().getRegionTable().updateRegion(*kvr1); + + std::vector keys{RecordKVFormat::genKey(table_id, 3).toString(), RecordKVFormat::genKey(table_id, 3, 5).toString(), RecordKVFormat::genKey(table_id, 3, 8).toString()}; + std::vector vals({RecordKVFormat::encodeLockCfValue(RecordKVFormat::CFModifyFlag::PutFlag, "PK", 3, 20).toString(), TiKVValue("value1").toString(), RecordKVFormat::encodeWriteCfValue(RecordKVFormat::CFModifyFlag::PutFlag, 5).toString()}); + auto ops = std::vector{ + ColumnFamilyType::Lock, + ColumnFamilyType::Default, + ColumnFamilyType::Write, + }; + auto [index, term] = proxy_instance->rawWrite(region_id, std::move(keys), std::move(vals), {WriteCmdType::Put, WriteCmdType::Put, WriteCmdType::Put}, std::move(ops)); + ASSERT_EQ(index, 6); + ASSERT_EQ(kvr1->appliedIndex(), 5); + ASSERT_EQ(term, 5); + + auto mvcc_query_info = MvccQueryInfo(false, 10); + auto f = [&] { + auto discard = doLearnerRead( + table_id, + mvcc_query_info, + false, + ctx, + log); + UNUSED(discard); + }; + EXPECT_THROW(f(), RegionException); + + // We can't `doApply`, since the TiKVValue is not valid. + auto r1 = proxy_instance->getRegion(region_id); + r1->updateAppliedIndex(index); + kvr1->setApplied(index, term); + auto regions_snapshot = doLearnerRead( + table_id, + mvcc_query_info, + false, + ctx, + log); + // 0 unavailable regions + ASSERT_EQ(regions_snapshot.size(), 1); + + // No throw + auto mvcc_query_info2 = MvccQueryInfo(false, 10); + mvcc_query_info2.regions_query_info.emplace_back(1, kvr1->version(), kvr1->confVer(), table_id, kvr1->getRange()->rawKeys()); + validateQueryInfo(mvcc_query_info2, regions_snapshot, ctx.getTMTContext(), log); +} +CATCH + } // namespace tests } // namespace DB diff --git a/dbms/src/Storages/Transaction/tests/kvstore_helper.h b/dbms/src/Storages/Transaction/tests/kvstore_helper.h index eec095f68de..f1b99969994 100644 --- a/dbms/src/Storages/Transaction/tests/kvstore_helper.h +++ b/dbms/src/Storages/Transaction/tests/kvstore_helper.h @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +#pragma once + #include #include #include @@ -82,8 +84,7 @@ class RegionKVStoreTest : public ::testing::Test reloadKVSFromDisk(); proxy_instance = std::make_unique(); - proxy_helper = std::make_unique(MockRaftStoreProxy::SetRaftStoreProxyFFIHelper( - RaftStoreProxyPtr{proxy_instance.get()})); + proxy_helper = proxy_instance->generateProxyHelper(); kvstore->restore(*path_pool, proxy_helper.get()); { auto store = metapb::Store{}; @@ -134,6 +135,35 @@ class RegionKVStoreTest : public ::testing::Test p = path + "/data/"; TiFlashTestEnv::tryCreatePath(p); } + void startReadIndexUtils(Context & ctx) + { + if (proxy_runner) + { + return; + } + over.store(false); + ctx.getTMTContext().setStatusRunning(); + // Start mock proxy in other thread + proxy_runner.reset(new std::thread([&]() { + proxy_instance->testRunNormal(over); + })); + ASSERT_EQ(kvstore->getProxyHelper(), proxy_helper.get()); + kvstore->initReadIndexWorkers( + []() { + return std::chrono::milliseconds(10); + }, + 1); + ASSERT_NE(kvstore->read_index_worker_manager, nullptr); + kvstore->asyncRunReadIndexWorkers(); + } + void stopReadIndexUtils() + { + kvstore->stopReadIndexWorkers(); + kvstore->releaseReadIndexWorkers(); + over = true; + proxy_instance->wakeNotifier(); + proxy_runner->join(); + } protected: static void testRaftSplit(KVStore & kvs, TMTContext & tmt); @@ -161,10 +191,14 @@ class RegionKVStoreTest : public ::testing::Test std::string test_path; std::unique_ptr path_pool; - std::unique_ptr kvstore; + std::shared_ptr kvstore; std::unique_ptr proxy_instance; std::unique_ptr proxy_helper; + std::unique_ptr proxy_runner; + + LoggerPtr log = DB::Logger::get("RegionKVStoreTest"); + std::atomic_bool over{false}; }; } // namespace tests } // namespace DB