From ce93bb6703c42367117b0801e93e3abb85a9c9b9 Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Thu, 27 Apr 2023 16:20:03 +0800 Subject: [PATCH 1/7] f Signed-off-by: CalvinNeo --- dbms/src/Debug/MockRaftStoreProxy.cpp | 2 +- .../Storages/Transaction/ApplySnapshot.cpp | 6 ++-- dbms/src/Storages/Transaction/KVStore.cpp | 36 ++++++++++++------- dbms/src/Storages/Transaction/KVStore.h | 4 +-- dbms/src/Storages/Transaction/Region.cpp | 8 ++--- dbms/src/Storages/Transaction/Region.h | 2 +- dbms/src/Storages/Transaction/RegionTable.cpp | 2 +- .../Transaction/tests/gtest_kvstore.cpp | 12 +++---- .../Transaction/tests/gtest_new_kvstore.cpp | 8 ++--- 9 files changed, 46 insertions(+), 34 deletions(-) diff --git a/dbms/src/Debug/MockRaftStoreProxy.cpp b/dbms/src/Debug/MockRaftStoreProxy.cpp index 36f7fefcd2e..3d22d9b19d5 100644 --- a/dbms/src/Debug/MockRaftStoreProxy.cpp +++ b/dbms/src/Debug/MockRaftStoreProxy.cpp @@ -547,7 +547,7 @@ void MockRaftStoreProxy::doApply( return; auto old_applied = kvs.getRegion(region_id)->appliedIndex(); - auto old_applied_term = kvs.getRegion(region_id)->appliedIndexTerm(); + auto old_applied_term = 0; //kvs.getRegion(region_id)->appliedIndexTerm(); if (cmd.has_raw_write_request()) { // TiFlash write diff --git a/dbms/src/Storages/Transaction/ApplySnapshot.cpp b/dbms/src/Storages/Transaction/ApplySnapshot.cpp index 5ab1cc87479..81b64f50c9a 100644 --- a/dbms/src/Storages/Transaction/ApplySnapshot.cpp +++ b/dbms/src/Storages/Transaction/ApplySnapshot.cpp @@ -86,7 +86,7 @@ void KVStore::checkAndApplyPreHandledSnapshot(const RegionPtrWrap & new_region, old_region->setStateApplying(); tmt.getRegionTable().tryFlushRegion(old_region, false); tryFlushRegionCacheInStorage(tmt, *old_region, log); - persistRegion(*old_region, region_lock, "save previous region before apply"); + persistRegion(*old_region, ®ion_lock, "save previous region before apply"); } } @@ -261,7 +261,7 @@ void KVStore::onSnapshot(const RegionPtrWrap & new_region_wrap, RegionPtr old_re manage_lock.index.add(new_region); } - persistRegion(*new_region, region_lock, "save current region after apply"); + persistRegion(*new_region, ®ion_lock, "save current region after apply"); tmt.getRegionTable().shrinkRegionRange(*new_region); } @@ -533,7 +533,7 @@ EngineStoreApplyRes KVStore::handleIngestSST(UInt64 region_id, const SSTViewVec } else { - persistRegion(*region, region_task_lock, __FUNCTION__); + persistRegion(*region, ®ion_task_lock, __FUNCTION__); return EngineStoreApplyRes::Persist; } } diff --git a/dbms/src/Storages/Transaction/KVStore.cpp b/dbms/src/Storages/Transaction/KVStore.cpp index 51ec448d7d7..59f3bd587bb 100644 --- a/dbms/src/Storages/Transaction/KVStore.cpp +++ b/dbms/src/Storages/Transaction/KVStore.cpp @@ -172,15 +172,12 @@ bool KVStore::tryFlushRegionCacheInStorage(TMTContext & tmt, const Region & regi return true; } -void KVStore::tryPersist(RegionID region_id) +void KVStore::tryPersistRegion(RegionID region_id) { auto region = getRegion(region_id); if (region) { - LOG_INFO(log, "Try to persist {}", region->toString(false)); - RUNTIME_CHECK_MSG(region_persister, "try access to region_persister without initialization, stack={}", StackTrace().toString()); - region_persister->persist(*region); - LOG_INFO(log, "After persisted {}, cache {} bytes", region->toString(false), region->dataSize()); + persistRegion(*region, std::nullopt, ""); } } @@ -333,12 +330,27 @@ void KVStore::setRegionCompactLogConfig(UInt64 sec, UInt64 rows, UInt64 bytes) bytes); } -void KVStore::persistRegion(const Region & region, const RegionTaskLock & region_task_lock, const char * caller) +void KVStore::persistRegion(const Region & region, std::optional region_task_lock, const char * caller) { - LOG_INFO(log, "Start to persist {}, cache size: {} bytes for `{}`", region.toString(true), region.dataSize(), caller); RUNTIME_CHECK_MSG(region_persister, "try access to region_persister without initialization, stack={}", StackTrace().toString()); - region_persister->persist(region, region_task_lock); - LOG_DEBUG(log, "Persist {} done", region.toString(false)); + if (region_task_lock.has_value()) + { + LOG_INFO(log, "Start to persist {}, cache size: {} bytes for `{}`", region.toString(true), region.dataSize(), caller); + region_persister->persist(region, *region_task_lock.value()); + } + else + { + LOG_INFO(log, "Try to persist {}", region.toString(false)); + region_persister->persist(region); + } + if (region_task_lock.has_value()) + { + LOG_DEBUG(log, "Persist {} done", region.toString(false)); + } + else + { + LOG_INFO(log, "After persisted {}, cache {} bytes", region.toString(false), region.dataSize()); + } } bool KVStore::needFlushRegionData(UInt64 region_id, TMTContext & tmt) @@ -422,7 +434,7 @@ bool KVStore::forceFlushRegionDataImpl(Region & curr_region, bool try_until_succ } if (tryFlushRegionCacheInStorage(tmt, curr_region, log, try_until_succeed)) { - persistRegion(curr_region, region_task_lock, "tryFlushRegionData"); + persistRegion(curr_region, ®ion_task_lock, "tryFlushRegionData"); curr_region.markCompactLog(); curr_region.cleanApproxMemCacheInfo(); GET_METRIC(tiflash_raft_apply_write_command_duration_seconds, type_flush_region).Observe(watch.elapsedSeconds()); @@ -474,7 +486,7 @@ EngineStoreApplyRes KVStore::handleUselessAdminRaftCmd( || cmd_type == raft_cmdpb::AdminCmdType::BatchSwitchWitness) { tryFlushRegionCacheInStorage(tmt, curr_region, log); - persistRegion(curr_region, region_task_lock, fmt::format("admin cmd useless {}", cmd_type).c_str()); + persistRegion(curr_region, ®ion_task_lock, fmt::format("admin cmd useless {}", cmd_type).c_str()); return EngineStoreApplyRes::Persist; } return EngineStoreApplyRes::None; @@ -549,7 +561,7 @@ EngineStoreApplyRes KVStore::handleAdminRaftCmd(raft_cmdpb::AdminRequest && requ const auto persist_and_sync = [&](const Region & region) { tryFlushRegionCacheInStorage(tmt, region, log); - persistRegion(region, region_task_lock, "admin raft cmd"); + persistRegion(region, ®ion_task_lock, "admin raft cmd"); }; const auto handle_batch_split = [&](Regions & split_regions) { diff --git a/dbms/src/Storages/Transaction/KVStore.h b/dbms/src/Storages/Transaction/KVStore.h index 9c8a18f960c..0ece457eace 100644 --- a/dbms/src/Storages/Transaction/KVStore.h +++ b/dbms/src/Storages/Transaction/KVStore.h @@ -93,7 +93,7 @@ class KVStore final : private boost::noncopyable void gcRegionPersistedCache(Seconds gc_persist_period = Seconds(60 * 5)); - void tryPersist(RegionID region_id); + void tryPersistRegion(RegionID region_id); static bool tryFlushRegionCacheInStorage(TMTContext & tmt, const Region & region, const LoggerPtr & log, bool try_until_succeed = true); @@ -246,7 +246,7 @@ class KVStore final : private boost::noncopyable bool canFlushRegionDataImpl(const RegionPtr & curr_region_ptr, UInt8 flush_if_possible, bool try_until_succeed, TMTContext & tmt, const RegionTaskLock & region_task_lock, UInt64 index, UInt64 term); bool forceFlushRegionDataImpl(Region & curr_region, bool try_until_succeed, TMTContext & tmt, const RegionTaskLock & region_task_lock, UInt64 index, UInt64 term); - void persistRegion(const Region & region, const RegionTaskLock & region_task_lock, const char * caller); + void persistRegion(const Region & region, std::optional region_task_lock, const char * caller); void releaseReadIndexWorkers(); void handleDestroy(UInt64 region_id, TMTContext & tmt, const KVStoreTaskLock &); diff --git a/dbms/src/Storages/Transaction/Region.cpp b/dbms/src/Storages/Transaction/Region.cpp index 9265085e32b..69c3d773d18 100644 --- a/dbms/src/Storages/Transaction/Region.cpp +++ b/dbms/src/Storages/Transaction/Region.cpp @@ -95,10 +95,10 @@ UInt64 Region::appliedIndex() const return meta.appliedIndex(); } -UInt64 Region::appliedIndexTerm() const -{ - return meta.appliedIndexTerm(); -} +// UInt64 Region::appliedIndexTerm() const +// { +// return meta.appliedIndexTerm(); +// } void Region::setApplied(UInt64 index, UInt64 term) { diff --git a/dbms/src/Storages/Transaction/Region.h b/dbms/src/Storages/Transaction/Region.h index 94778157c57..ad95395a119 100644 --- a/dbms/src/Storages/Transaction/Region.h +++ b/dbms/src/Storages/Transaction/Region.h @@ -167,7 +167,7 @@ class Region : public std::enable_shared_from_this std::tuple waitIndex(UInt64 index, const UInt64 timeout_ms, std::function && check_running); UInt64 appliedIndex() const; - UInt64 appliedIndexTerm() const; + // UInt64 appliedIndexTerm() const; void notifyApplied() { meta.notifyAll(); } // Export for tests. diff --git a/dbms/src/Storages/Transaction/RegionTable.cpp b/dbms/src/Storages/Transaction/RegionTable.cpp index bd490015479..1fac8a46071 100644 --- a/dbms/src/Storages/Transaction/RegionTable.cpp +++ b/dbms/src/Storages/Transaction/RegionTable.cpp @@ -140,7 +140,7 @@ RegionDataReadInfoList RegionTable::flushRegion(const RegionPtrWithBlock & regio if (try_persist) { KVStore::tryFlushRegionCacheInStorage(tmt, *region, log); - tmt.getKVStore()->tryPersist(region->id()); + tmt.getKVStore()->tryPersistRegion(region->id()); } } diff --git a/dbms/src/Storages/Transaction/tests/gtest_kvstore.cpp b/dbms/src/Storages/Transaction/tests/gtest_kvstore.cpp index b27445a572f..2cb0b089425 100644 --- a/dbms/src/Storages/Transaction/tests/gtest_kvstore.cpp +++ b/dbms/src/Storages/Transaction/tests/gtest_kvstore.cpp @@ -35,7 +35,7 @@ TEST_F(RegionKVStoreTest, NewProxy) } } { - kvs.tryPersist(1); + kvs.tryPersistRegion(1); kvs.gcRegionPersistedCache(Seconds{0}); } { @@ -308,7 +308,7 @@ void RegionKVStoreTest::testRaftMergeRollback(KVStore & kvs, TMTContext & tmt) ASSERT_EQ(region->peerState(), raft_serverpb::PeerState::Applying); region->setPeerState(raft_serverpb::PeerState::Merging); - region->meta.region_state.getMutMergeState().set_commit(1234); + // region->meta.region_state.getMutMergeState().set_commit(1234); try { kvs.handleAdminRaftCmd(std::move(request), @@ -796,7 +796,7 @@ TEST_F(RegionKVStoreTest, KVStore) } } { - kvs.tryPersist(1); + kvs.tryPersistRegion(1); kvs.gcRegionPersistedCache(Seconds{0}); } { @@ -1329,9 +1329,9 @@ TEST_F(RegionKVStoreTest, KVStoreRestore) lock.index.add(region); } } - kvs.tryPersist(1); - kvs.tryPersist(2); - kvs.tryPersist(3); + kvs.tryPersistRegion(1); + kvs.tryPersistRegion(2); + kvs.tryPersistRegion(3); } { KVStore & kvs = reloadKVSFromDisk(); diff --git a/dbms/src/Storages/Transaction/tests/gtest_new_kvstore.cpp b/dbms/src/Storages/Transaction/tests/gtest_new_kvstore.cpp index e0fc4979d43..9b8676609be 100644 --- a/dbms/src/Storages/Transaction/tests/gtest_new_kvstore.cpp +++ b/dbms/src/Storages/Transaction/tests/gtest_new_kvstore.cpp @@ -40,7 +40,7 @@ try proxy_instance->doApply(kvs, ctx.getTMTContext(), cond, region_id, index); ASSERT_EQ(r1->getLatestAppliedIndex(), applied_index + 1); ASSERT_EQ(kvr1->appliedIndex(), applied_index + 1); - kvs.tryPersist(region_id); + kvs.tryPersistRegion(region_id); } { const KVStore & kvs = reloadKVSFromDisk(); @@ -70,7 +70,7 @@ try proxy_instance->doApply(kvs, ctx.getTMTContext(), cond, region_id, index); ASSERT_EQ(r1->getLatestAppliedIndex(), applied_index); ASSERT_EQ(kvr1->appliedIndex(), applied_index); - kvs.tryPersist(region_id); + kvs.tryPersistRegion(region_id); } { KVStore & kvs = reloadKVSFromDisk(); @@ -103,7 +103,7 @@ try proxy_instance->doApply(kvs, ctx.getTMTContext(), cond, region_id, index); ASSERT_EQ(r1->getLatestAppliedIndex(), applied_index); ASSERT_EQ(kvr1->appliedIndex(), applied_index); - kvs.tryPersist(region_id); + kvs.tryPersistRegion(region_id); } { KVStore & kvs = reloadKVSFromDisk(); @@ -135,7 +135,7 @@ try proxy_instance->doApply(kvs, ctx.getTMTContext(), cond, region_id, index); ASSERT_EQ(r1->getLatestAppliedIndex(), applied_index); ASSERT_EQ(kvr1->appliedIndex(), applied_index + 1); - kvs.tryPersist(region_id); + kvs.tryPersistRegion(region_id); } { MockRaftStoreProxy::FailCond cond; From 4eb7f4718fcbcf874e3eebefa086d8348bafdf30 Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Thu, 27 Apr 2023 16:31:47 +0800 Subject: [PATCH 2/7] zzz Signed-off-by: CalvinNeo --- dbms/src/Debug/dbgFuncRegion.cpp | 2 +- dbms/src/Storages/Transaction/ApplySnapshot.cpp | 6 +++--- dbms/src/Storages/Transaction/KVStore.cpp | 2 +- dbms/src/Storages/Transaction/RegionTable.cpp | 8 ++++---- dbms/src/Storages/Transaction/RegionTable.h | 4 ++-- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/dbms/src/Debug/dbgFuncRegion.cpp b/dbms/src/Debug/dbgFuncRegion.cpp index f65a18b8fd0..34d788bf788 100644 --- a/dbms/src/Debug/dbgFuncRegion.cpp +++ b/dbms/src/Debug/dbgFuncRegion.cpp @@ -116,7 +116,7 @@ void dbgFuncTryFlushRegion(Context & context, const ASTs & args, DBGInvoker::Pri auto region_id = static_cast(safeGet(typeid_cast(*args[0]).value)); TMTContext & tmt = context.getTMTContext(); - tmt.getRegionTable().tryFlushRegion(region_id); + tmt.getRegionTable().tryWriteBlockByRegionAndFlush(region_id); output(fmt::format("region_table try flush region {}", region_id)); } diff --git a/dbms/src/Storages/Transaction/ApplySnapshot.cpp b/dbms/src/Storages/Transaction/ApplySnapshot.cpp index 81b64f50c9a..08d7fd5fe0d 100644 --- a/dbms/src/Storages/Transaction/ApplySnapshot.cpp +++ b/dbms/src/Storages/Transaction/ApplySnapshot.cpp @@ -84,7 +84,7 @@ void KVStore::checkAndApplyPreHandledSnapshot(const RegionPtrWrap & new_region, // engine may delete data unsafely. auto region_lock = region_manager.genRegionTaskLock(old_region->id()); old_region->setStateApplying(); - tmt.getRegionTable().tryFlushRegion(old_region, false); + tmt.getRegionTable().tryWriteBlockByRegionAndFlush(old_region, false); tryFlushRegionCacheInStorage(tmt, *old_region, log); persistRegion(*old_region, ®ion_lock, "save previous region before apply"); } @@ -209,7 +209,7 @@ void KVStore::onSnapshot(const RegionPtrWrap & new_region_wrap, RegionPtr old_re { try { - auto tmp = region_table.tryFlushRegion(new_region_wrap, false); + auto tmp = region_table.tryWriteBlockByRegionAndFlush(new_region_wrap, false); { std::lock_guard lock(bg_gc_region_data_mutex); bg_gc_region_data.push_back(std::move(tmp)); @@ -506,7 +506,7 @@ EngineStoreApplyRes KVStore::handleIngestSST(UInt64 region_id, const SSTViewVec return; try { - tmt.getRegionTable().tryFlushRegion(region, false); + tmt.getRegionTable().tryWriteBlockByRegionAndFlush(region, false); tryFlushRegionCacheInStorage(tmt, *region, log); } catch (Exception & e) diff --git a/dbms/src/Storages/Transaction/KVStore.cpp b/dbms/src/Storages/Transaction/KVStore.cpp index 59f3bd587bb..3275eefe508 100644 --- a/dbms/src/Storages/Transaction/KVStore.cpp +++ b/dbms/src/Storages/Transaction/KVStore.cpp @@ -551,7 +551,7 @@ EngineStoreApplyRes KVStore::handleAdminRaftCmd(raft_cmdpb::AdminRequest && requ const auto try_to_flush_region = [&tmt](const RegionPtr & region) { try { - tmt.getRegionTable().tryFlushRegion(region, false); + tmt.getRegionTable().tryWriteBlockByRegionAndFlush(region, false); } catch (...) { diff --git a/dbms/src/Storages/Transaction/RegionTable.cpp b/dbms/src/Storages/Transaction/RegionTable.cpp index 1fac8a46071..1b8d0186065 100644 --- a/dbms/src/Storages/Transaction/RegionTable.cpp +++ b/dbms/src/Storages/Transaction/RegionTable.cpp @@ -302,7 +302,7 @@ void RegionTable::removeRegion(const RegionID region_id, bool remove_data, const } } -RegionDataReadInfoList RegionTable::tryFlushRegion(RegionID region_id, bool try_persist) +RegionDataReadInfoList RegionTable::tryWriteBlockByRegionAndFlush(RegionID region_id, bool try_persist) { auto region = context->getTMTContext().getKVStore()->getRegion(region_id); if (!region) @@ -311,10 +311,10 @@ RegionDataReadInfoList RegionTable::tryFlushRegion(RegionID region_id, bool try_ return {}; } - return tryFlushRegion(region, try_persist); + return tryWriteBlockByRegionAndFlush(region, try_persist); } -RegionDataReadInfoList RegionTable::tryFlushRegion(const RegionPtrWithBlock & region, bool try_persist) +RegionDataReadInfoList RegionTable::tryWriteBlockByRegionAndFlush(const RegionPtrWithBlock & region, bool try_persist) { RegionID region_id = region->id(); @@ -414,7 +414,7 @@ bool RegionTable::tryFlushRegions() { if (RegionID region_to_flush = pickRegionToFlush(); region_to_flush != InvalidRegionID) { - tryFlushRegion(region_to_flush, true); + tryWriteBlockByRegionAndFlush(region_to_flush, true); return true; } diff --git a/dbms/src/Storages/Transaction/RegionTable.h b/dbms/src/Storages/Transaction/RegionTable.h index 6936eb648da..0c24b250f35 100644 --- a/dbms/src/Storages/Transaction/RegionTable.h +++ b/dbms/src/Storages/Transaction/RegionTable.h @@ -160,8 +160,8 @@ class RegionTable : private boost::noncopyable void removeRegion(RegionID region_id, bool remove_data, const RegionTaskLock &); bool tryFlushRegions(); - RegionDataReadInfoList tryFlushRegion(RegionID region_id, bool try_persist = false); - RegionDataReadInfoList tryFlushRegion(const RegionPtrWithBlock & region, bool try_persist); + RegionDataReadInfoList tryWriteBlockByRegionAndFlush(RegionID region_id, bool try_persist = false); + RegionDataReadInfoList tryWriteBlockByRegionAndFlush(const RegionPtrWithBlock & region, bool try_persist); void handleInternalRegionsByTable(KeyspaceID keyspace_id, TableID table_id, std::function && callback) const; std::vector> getRegionsByTable(KeyspaceID keyspace_id, TableID table_id) const; From aaf4c291238faf3c88e83bbfbecaefe541ea15fa Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Thu, 27 Apr 2023 17:31:25 +0800 Subject: [PATCH 3/7] z Signed-off-by: CalvinNeo --- dbms/src/Debug/dbgFuncRegion.cpp | 2 +- dbms/src/Storages/Transaction/RegionTable.cpp | 6 +++--- dbms/src/Storages/Transaction/RegionTable.h | 15 ++++++++++++--- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/dbms/src/Debug/dbgFuncRegion.cpp b/dbms/src/Debug/dbgFuncRegion.cpp index 34d788bf788..ed6e256cb61 100644 --- a/dbms/src/Debug/dbgFuncRegion.cpp +++ b/dbms/src/Debug/dbgFuncRegion.cpp @@ -101,7 +101,7 @@ void dbgFuncPutRegion(Context & context, const ASTs & args, DBGInvoker::Printer void dbgFuncTryFlush(Context & context, const ASTs &, DBGInvoker::Printer output) { TMTContext & tmt = context.getTMTContext(); - tmt.getRegionTable().tryFlushRegions(); + tmt.getRegionTable().writeBlockForAllRegionAndFlush(); output("region_table try flush regions"); } diff --git a/dbms/src/Storages/Transaction/RegionTable.cpp b/dbms/src/Storages/Transaction/RegionTable.cpp index 1b8d0186065..2ee196283e0 100644 --- a/dbms/src/Storages/Transaction/RegionTable.cpp +++ b/dbms/src/Storages/Transaction/RegionTable.cpp @@ -120,7 +120,7 @@ bool RegionTable::shouldFlush(const InternalRegion & region) const return false; } -RegionDataReadInfoList RegionTable::flushRegion(const RegionPtrWithBlock & region, bool try_persist) const +RegionDataReadInfoList RegionTable::writeBlockByRegionAndFlush(const RegionPtrWithBlock & region, bool try_persist) const { auto & tmt = context->getTMTContext(); @@ -349,7 +349,7 @@ RegionDataReadInfoList RegionTable::tryWriteBlockByRegionAndFlush(const RegionPt RegionDataReadInfoList data_list_to_remove; try { - data_list_to_remove = flushRegion(region, try_persist); + data_list_to_remove = writeBlockByRegionAndFlush(region, try_persist); } catch (const Exception & e) { @@ -410,7 +410,7 @@ RegionID RegionTable::pickRegionToFlush() return InvalidRegionID; } -bool RegionTable::tryFlushRegions() +bool RegionTable::writeBlockForAllRegionAndFlush() { if (RegionID region_to_flush = pickRegionToFlush(); region_to_flush != InvalidRegionID) { diff --git a/dbms/src/Storages/Transaction/RegionTable.h b/dbms/src/Storages/Transaction/RegionTable.h index 0c24b250f35..3054df799d1 100644 --- a/dbms/src/Storages/Transaction/RegionTable.h +++ b/dbms/src/Storages/Transaction/RegionTable.h @@ -159,7 +159,14 @@ class RegionTable : private boost::noncopyable void removeRegion(RegionID region_id, bool remove_data, const RegionTaskLock &); - bool tryFlushRegions(); + // Find all regions with data, call writeBlockByRegionAndFlush with try_persist = true. + // This function is only for debug. + // The original name for this function is tryFlushRegions. + bool writeBlockForAllRegionAndFlush(); + + // Protects writeBlockByRegionAndFlush and ensures it's executed by only one thread at the smae time. + // Only one thread can do this at the same time. + // The original name for this function is tryFlushRegion. RegionDataReadInfoList tryWriteBlockByRegionAndFlush(RegionID region_id, bool try_persist = false); RegionDataReadInfoList tryWriteBlockByRegionAndFlush(const RegionPtrWithBlock & region, bool try_persist); @@ -191,7 +198,6 @@ class RegionTable : private boost::noncopyable /// extend range for possible InternalRegion or add one. void extendRegionRange(RegionID region_id, const RegionRangeKeys & region_range_keys); - void updateSafeTS(UInt64 region_id, UInt64 leader_safe_ts, UInt64 self_safe_ts); // unit: ms. If safe_ts diff is larger than 2min, we think the data synchronization progress is far behind the leader. @@ -211,7 +217,10 @@ class RegionTable : private boost::noncopyable InternalRegion & insertRegion(Table & table, const Region & region); InternalRegion & doGetInternalRegion(KeyspaceTableID ks_tb_id, RegionID region_id); - RegionDataReadInfoList flushRegion(const RegionPtrWithBlock & region, bool try_persist) const; + // Try write the committed kvs into cache of columnar DeltaMergeStore. + // Flush the cache if try_persist is set to true. + // The original name for this method is flushRegion. + RegionDataReadInfoList writeBlockByRegionAndFlush(const RegionPtrWithBlock & region, bool try_persist) const; bool shouldFlush(const InternalRegion & region) const; RegionID pickRegionToFlush(); From 61b28e17fec4e02dac68064ad03f2eb7f0a0aafb Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Thu, 27 Apr 2023 17:43:09 +0800 Subject: [PATCH 4/7] fix tyoe Signed-off-by: CalvinNeo --- dbms/src/Debug/MockRaftStoreProxy.cpp | 2 +- dbms/src/Storages/Transaction/Region.cpp | 8 ++++---- dbms/src/Storages/Transaction/Region.h | 2 +- dbms/src/Storages/Transaction/tests/gtest_kvstore.cpp | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/dbms/src/Debug/MockRaftStoreProxy.cpp b/dbms/src/Debug/MockRaftStoreProxy.cpp index 3d22d9b19d5..36f7fefcd2e 100644 --- a/dbms/src/Debug/MockRaftStoreProxy.cpp +++ b/dbms/src/Debug/MockRaftStoreProxy.cpp @@ -547,7 +547,7 @@ void MockRaftStoreProxy::doApply( return; auto old_applied = kvs.getRegion(region_id)->appliedIndex(); - auto old_applied_term = 0; //kvs.getRegion(region_id)->appliedIndexTerm(); + auto old_applied_term = kvs.getRegion(region_id)->appliedIndexTerm(); if (cmd.has_raw_write_request()) { // TiFlash write diff --git a/dbms/src/Storages/Transaction/Region.cpp b/dbms/src/Storages/Transaction/Region.cpp index 69c3d773d18..9265085e32b 100644 --- a/dbms/src/Storages/Transaction/Region.cpp +++ b/dbms/src/Storages/Transaction/Region.cpp @@ -95,10 +95,10 @@ UInt64 Region::appliedIndex() const return meta.appliedIndex(); } -// UInt64 Region::appliedIndexTerm() const -// { -// return meta.appliedIndexTerm(); -// } +UInt64 Region::appliedIndexTerm() const +{ + return meta.appliedIndexTerm(); +} void Region::setApplied(UInt64 index, UInt64 term) { diff --git a/dbms/src/Storages/Transaction/Region.h b/dbms/src/Storages/Transaction/Region.h index ad95395a119..94778157c57 100644 --- a/dbms/src/Storages/Transaction/Region.h +++ b/dbms/src/Storages/Transaction/Region.h @@ -167,7 +167,7 @@ class Region : public std::enable_shared_from_this std::tuple waitIndex(UInt64 index, const UInt64 timeout_ms, std::function && check_running); UInt64 appliedIndex() const; - // UInt64 appliedIndexTerm() const; + UInt64 appliedIndexTerm() const; void notifyApplied() { meta.notifyAll(); } // Export for tests. diff --git a/dbms/src/Storages/Transaction/tests/gtest_kvstore.cpp b/dbms/src/Storages/Transaction/tests/gtest_kvstore.cpp index 2cb0b089425..4f91ef788bf 100644 --- a/dbms/src/Storages/Transaction/tests/gtest_kvstore.cpp +++ b/dbms/src/Storages/Transaction/tests/gtest_kvstore.cpp @@ -308,7 +308,7 @@ void RegionKVStoreTest::testRaftMergeRollback(KVStore & kvs, TMTContext & tmt) ASSERT_EQ(region->peerState(), raft_serverpb::PeerState::Applying); region->setPeerState(raft_serverpb::PeerState::Merging); - // region->meta.region_state.getMutMergeState().set_commit(1234); + region->meta.region_state.getMutMergeState().set_commit(1234); try { kvs.handleAdminRaftCmd(std::move(request), From 2f8ecd8fdaa2d6cb0ba54164da3acda077c07d7e Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Thu, 27 Apr 2023 18:29:57 +0800 Subject: [PATCH 5/7] f Signed-off-by: CalvinNeo --- dbms/src/Debug/dbgFuncRegion.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/dbms/src/Debug/dbgFuncRegion.cpp b/dbms/src/Debug/dbgFuncRegion.cpp index ed6e256cb61..359e718bbb6 100644 --- a/dbms/src/Debug/dbgFuncRegion.cpp +++ b/dbms/src/Debug/dbgFuncRegion.cpp @@ -98,12 +98,9 @@ void dbgFuncPutRegion(Context & context, const ASTs & args, DBGInvoker::Printer } } -void dbgFuncTryFlush(Context & context, const ASTs &, DBGInvoker::Printer output) +void dbgFuncTryFlush(Context &, const ASTs &, DBGInvoker::Printer output) { - TMTContext & tmt = context.getTMTContext(); - tmt.getRegionTable().writeBlockForAllRegionAndFlush(); - - output("region_table try flush regions"); + output("DEPRECATED region_table try flush regions"); } void dbgFuncTryFlushRegion(Context & context, const ASTs & args, DBGInvoker::Printer output) From 49fdab868fe7b55e686111c4cf9fe7322edd8297 Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Thu, 27 Apr 2023 18:32:52 +0800 Subject: [PATCH 6/7] gggg Signed-off-by: CalvinNeo --- dbms/src/Debug/dbgFuncRegion.cpp | 7 +++++-- dbms/src/Storages/Transaction/RegionTable.cpp | 2 +- dbms/src/Storages/Transaction/RegionTable.h | 3 +-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/dbms/src/Debug/dbgFuncRegion.cpp b/dbms/src/Debug/dbgFuncRegion.cpp index 359e718bbb6..34d788bf788 100644 --- a/dbms/src/Debug/dbgFuncRegion.cpp +++ b/dbms/src/Debug/dbgFuncRegion.cpp @@ -98,9 +98,12 @@ void dbgFuncPutRegion(Context & context, const ASTs & args, DBGInvoker::Printer } } -void dbgFuncTryFlush(Context &, const ASTs &, DBGInvoker::Printer output) +void dbgFuncTryFlush(Context & context, const ASTs &, DBGInvoker::Printer output) { - output("DEPRECATED region_table try flush regions"); + TMTContext & tmt = context.getTMTContext(); + tmt.getRegionTable().tryFlushRegions(); + + output("region_table try flush regions"); } void dbgFuncTryFlushRegion(Context & context, const ASTs & args, DBGInvoker::Printer output) diff --git a/dbms/src/Storages/Transaction/RegionTable.cpp b/dbms/src/Storages/Transaction/RegionTable.cpp index 2ee196283e0..c85d95ebe81 100644 --- a/dbms/src/Storages/Transaction/RegionTable.cpp +++ b/dbms/src/Storages/Transaction/RegionTable.cpp @@ -410,7 +410,7 @@ RegionID RegionTable::pickRegionToFlush() return InvalidRegionID; } -bool RegionTable::writeBlockForAllRegionAndFlush() +bool RegionTable::tryFlushRegions() { if (RegionID region_to_flush = pickRegionToFlush(); region_to_flush != InvalidRegionID) { diff --git a/dbms/src/Storages/Transaction/RegionTable.h b/dbms/src/Storages/Transaction/RegionTable.h index 3054df799d1..176b28f8937 100644 --- a/dbms/src/Storages/Transaction/RegionTable.h +++ b/dbms/src/Storages/Transaction/RegionTable.h @@ -161,8 +161,7 @@ class RegionTable : private boost::noncopyable // Find all regions with data, call writeBlockByRegionAndFlush with try_persist = true. // This function is only for debug. - // The original name for this function is tryFlushRegions. - bool writeBlockForAllRegionAndFlush(); + bool tryFlushRegions(); // Protects writeBlockByRegionAndFlush and ensures it's executed by only one thread at the smae time. // Only one thread can do this at the same time. From 7947cda887e4317e1badc3085ebe92cc0dad48fc Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Fri, 28 Apr 2023 15:16:27 +0800 Subject: [PATCH 7/7] ad Signed-off-by: CalvinNeo --- dbms/src/Storages/Transaction/KVStore.cpp | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/dbms/src/Storages/Transaction/KVStore.cpp b/dbms/src/Storages/Transaction/KVStore.cpp index 3275eefe508..0be67d40e7c 100644 --- a/dbms/src/Storages/Transaction/KVStore.cpp +++ b/dbms/src/Storages/Transaction/KVStore.cpp @@ -337,18 +337,12 @@ void KVStore::persistRegion(const Region & region, std::optionalpersist(region, *region_task_lock.value()); + LOG_DEBUG(log, "Persist {} done", region.toString(false)); } else { LOG_INFO(log, "Try to persist {}", region.toString(false)); region_persister->persist(region); - } - if (region_task_lock.has_value()) - { - LOG_DEBUG(log, "Persist {} done", region.toString(false)); - } - else - { LOG_INFO(log, "After persisted {}, cache {} bytes", region.toString(false), region.dataSize()); } }