From 9678db5e8b45368efd1637efd529f19d8015efbe Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Wed, 24 Jul 2024 17:07:25 +0800 Subject: [PATCH 01/10] a Signed-off-by: CalvinNeo --- .../tests/gtest_kvstore_fast_add_peer.cpp | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/dbms/src/Storages/KVStore/tests/gtest_kvstore_fast_add_peer.cpp b/dbms/src/Storages/KVStore/tests/gtest_kvstore_fast_add_peer.cpp index 3717bea6ee3..17251a82eb5 100644 --- a/dbms/src/Storages/KVStore/tests/gtest_kvstore_fast_add_peer.cpp +++ b/dbms/src/Storages/KVStore/tests/gtest_kvstore_fast_add_peer.cpp @@ -925,6 +925,34 @@ try } CATCH +TEST_F(RegionKVStoreTestFAP, TableNotFound) +try +{ + CheckpointRegionInfoAndData mock_data = prepareForRestart(FAPTestOpt{}); + RegionPtr kv_region = std::get<1>(mock_data); + + auto & global_context = TiFlashTestEnv::getGlobalContext(); + auto & tmt = global_context.getTMTContext(); + uint64_t region_id = 1; + + auto keyspace_id = kv_region->getKeyspaceID(); + auto table_id = kv_region->getMappedTableID(); + auto fap_context = global_context.getSharedContextDisagg()->fap_context; + + std::mutex exe_mut; + std::unique_lock exe_lock(exe_mut); + fap_context->tasks_trace->addTask(region_id, [&]() { + // Keep the task in `tasks_trace` to prevent from canceling. + std::scoped_lock wait_exe_lock(exe_mut); + return genFastAddPeerRes(FastAddPeerStatus::NoSuitable, "", ""); + }); + + auto & storages = tmt.getStorages(); + storages.remove(keyspace_id, table_id); + // Will generate and persist some information in local ps, which will not be uploaded. + FastAddPeerImplWrite(global_context.getTMTContext(), proxy_helper.get(), region_id, 2333, std::move(mock_data), 0); +} +CATCH } // namespace tests } // namespace DB From 6e465dba1086127fd6a77e9da28725a82d54523b Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Thu, 25 Jul 2024 00:44:57 +0800 Subject: [PATCH 02/10] a Signed-off-by: CalvinNeo --- .../Storages/KVStore/MultiRaft/Disagg/FastAddPeer.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/dbms/src/Storages/KVStore/MultiRaft/Disagg/FastAddPeer.cpp b/dbms/src/Storages/KVStore/MultiRaft/Disagg/FastAddPeer.cpp index 9a13986dea3..095a7ba8447 100644 --- a/dbms/src/Storages/KVStore/MultiRaft/Disagg/FastAddPeer.cpp +++ b/dbms/src/Storages/KVStore/MultiRaft/Disagg/FastAddPeer.cpp @@ -307,6 +307,15 @@ FastAddPeerRes FastAddPeerImplWrite( auto keyspace_id = region->getKeyspaceID(); auto table_id = region->getMappedTableID(); const auto [table_drop_lock, storage, schema_snap] = AtomicGetStorageSchema(region, tmt); + if (!storage) { + LOG_WARNING( + log, + "FAP failed because the table can not be found, region_id={} keyspace_id={} table_id={}", + region_id, + keyspace_id, + table_id); + return genFastAddPeerRes(FastAddPeerStatus::BadData, "", ""); + } UNUSED(schema_snap); RUNTIME_CHECK_MSG(storage->engineType() == TiDB::StorageEngine::DT, "ingest into unsupported storage engine"); auto dm_storage = std::dynamic_pointer_cast(storage); From 7f2c43ebc3c0c01dd5361a31256917da88f22af7 Mon Sep 17 00:00:00 2001 From: Calvin Neo Date: Fri, 26 Jul 2024 12:45:17 +0800 Subject: [PATCH 03/10] a Signed-off-by: Calvin Neo --- dbms/src/Storages/KVStore/MultiRaft/Disagg/FastAddPeer.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/dbms/src/Storages/KVStore/MultiRaft/Disagg/FastAddPeer.cpp b/dbms/src/Storages/KVStore/MultiRaft/Disagg/FastAddPeer.cpp index 095a7ba8447..40062f228bf 100644 --- a/dbms/src/Storages/KVStore/MultiRaft/Disagg/FastAddPeer.cpp +++ b/dbms/src/Storages/KVStore/MultiRaft/Disagg/FastAddPeer.cpp @@ -193,6 +193,8 @@ std::variant FastAddPeerImplSelect( Stopwatch watch; std::unordered_map checked_seq_map; auto fap_ctx = tmt.getContext().getSharedContextDisagg()->fap_context; + RUNTIME_CHECK(fap_ctx != nullptr); + RUNTIME_CHECK(fap_ctx->tasks_trace != nullptr); auto cancel_handle = fap_ctx->tasks_trace->getCancelHandleFromExecutor(region_id); // Get candidate stores. @@ -307,7 +309,8 @@ FastAddPeerRes FastAddPeerImplWrite( auto keyspace_id = region->getKeyspaceID(); auto table_id = region->getMappedTableID(); const auto [table_drop_lock, storage, schema_snap] = AtomicGetStorageSchema(region, tmt); - if (!storage) { + if (!storage) + { LOG_WARNING( log, "FAP failed because the table can not be found, region_id={} keyspace_id={} table_id={}", @@ -382,6 +385,7 @@ FastAddPeerRes FastAddPeerImplWrite( // Currently, FAP only handle when the peer is newly created in this store. // TODO(fap) However, Move this to `ApplyFapSnapshot` and clean stale data, if FAP can later handle all snapshots. UniversalWriteBatch wb; + RUNTIME_CHECK(checkpoint_info->temp_ps != nullptr); RaftDataReader raft_data_reader(*(checkpoint_info->temp_ps)); raft_data_reader.traverseRemoteRaftLogForRegion( region_id, @@ -396,6 +400,7 @@ FastAddPeerRes FastAddPeerImplWrite( }); GET_METRIC(tiflash_fap_task_duration_seconds, type_write_stage_raft).Observe(watch.elapsedSecondsFromLastTime()); auto wn_ps = tmt.getContext().getWriteNodePageStorage(); + RUNTIME_CHECK(wn_ps != nullptr); wn_ps->write(std::move(wb)); SYNC_FOR("in_FastAddPeerImplWrite::after_write_raft_log"); if (cancel_handle->isCanceled()) From 146dc9b2ae4b942f841e7f7d40634496071bb2c6 Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Fri, 26 Jul 2024 13:06:57 +0800 Subject: [PATCH 04/10] Refine func AtomicGetStorageSchema --- .../KVStore/Decode/PartitionStreams.cpp | 38 +++++++++++-------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/dbms/src/Storages/KVStore/Decode/PartitionStreams.cpp b/dbms/src/Storages/KVStore/Decode/PartitionStreams.cpp index 9d39958e923..8e55016e017 100644 --- a/dbms/src/Storages/KVStore/Decode/PartitionStreams.cpp +++ b/dbms/src/Storages/KVStore/Decode/PartitionStreams.cpp @@ -505,6 +505,10 @@ RegionTable::ResolveLocksAndWriteRegionRes RegionTable::resolveLocksAndWriteRegi region_data_lock); } +// Note that there could be a chance that the table have been totally removed from TiKV +// and TiFlash can not get the IStorage instance. +// - Check whether the StorageDeltaMerge is nullptr or not before you accessing to it. +// - You should hold the `TableLockHolder` when you're accessing to the IStorage instance. std::tuple, DecodingStorageSchemaSnapshotConstPtr> // AtomicGetStorageSchema(const RegionPtr & region, TMTContext & tmt) { @@ -514,7 +518,12 @@ AtomicGetStorageSchema(const RegionPtr & region, TMTContext & tmt) auto keyspace_id = region->getKeyspaceID(); auto table_id = region->getMappedTableID(); - LOG_DEBUG(Logger::get(__PRETTY_FUNCTION__), "Get schema, keyspace={} table_id={}", keyspace_id, table_id); + LOG_DEBUG( + Logger::get("AtomicGetStorageSchema"), + "Get schema, keyspace={} table_id={} region_id={}", + keyspace_id, + table_id, + region->id()); auto & context = tmt.getContext(); const auto atomic_get = [&](bool force_decode) -> bool { auto storage = tmt.getStorages().get(keyspace_id, table_id); @@ -522,8 +531,8 @@ AtomicGetStorageSchema(const RegionPtr & region, TMTContext & tmt) { if (!force_decode) return false; - if (storage == nullptr) // Table must have just been GC-ed - return true; + // Else force_decode == true, and storage instance still not exist, table must have just been GC-ed + return true; } // Get a structure read lock. It will throw exception if the table has been dropped, // the caller should handle this situation. @@ -539,22 +548,19 @@ AtomicGetStorageSchema(const RegionPtr & region, TMTContext & tmt) { GET_METRIC(tiflash_schema_trigger_count, type_raft_decode).Increment(); Stopwatch watch; - tmt.getSchemaSyncerManager()->syncTableSchema(context, keyspace_id, table_id); + auto sync_schema_ok = tmt.getSchemaSyncerManager()->syncTableSchema(context, keyspace_id, table_id); auto schema_sync_cost = watch.elapsedMilliseconds(); LOG_INFO( Logger::get("AtomicGetStorageSchema"), - "sync schema cost {} ms, keyspace={} table_id={}", + "sync schema cost {} ms, sync_schema_ok={} keyspace={} table_id={} region_id={}", schema_sync_cost, + sync_schema_ok, keyspace_id, - table_id); + table_id, + region->id()); - if (!atomic_get(true)) - throw Exception( - ErrorCodes::LOGICAL_ERROR, - "AtomicGetStorageSchema failed, region={} keyspace={} table_id={}", - region->toString(), - keyspace_id, - table_id); + // try get with `force_decode == true` + atomic_get(true); } return {std::move(drop_lock), std::move(dm_storage), std::move(schema_snapshot)}; @@ -567,8 +573,10 @@ static Block sortColumnsBySchemaSnap(Block && ori, const DM::ColumnDefines & sch if (ori.columns() != schema.size()) { throw Exception( - "Try to sortColumnsBySchemaSnap with different column size [block_columns=" + DB::toString(ori.columns()) - + "] [schema_columns=" + DB::toString(schema.size()) + "]"); + ErrorCodes::LOGICAL_ERROR, + "Try to sortColumnsBySchemaSnap with different column size [block_columns={}] [schema_columns={}]", + ori.columns(), + schema.size()); } #endif From b405e313c747dbbd5356a65d1a36dc5dfb0d729b Mon Sep 17 00:00:00 2001 From: Calvin Neo Date: Fri, 26 Jul 2024 13:27:14 +0800 Subject: [PATCH 05/10] Update dbms/src/Storages/KVStore/tests/gtest_kvstore_fast_add_peer.cpp Co-authored-by: JaySon --- dbms/src/Storages/KVStore/tests/gtest_kvstore_fast_add_peer.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/dbms/src/Storages/KVStore/tests/gtest_kvstore_fast_add_peer.cpp b/dbms/src/Storages/KVStore/tests/gtest_kvstore_fast_add_peer.cpp index 17251a82eb5..d44641a8382 100644 --- a/dbms/src/Storages/KVStore/tests/gtest_kvstore_fast_add_peer.cpp +++ b/dbms/src/Storages/KVStore/tests/gtest_kvstore_fast_add_peer.cpp @@ -947,6 +947,7 @@ try return genFastAddPeerRes(FastAddPeerStatus::NoSuitable, "", ""); }); + // Mock that the storage instance have been dropped auto & storages = tmt.getStorages(); storages.remove(keyspace_id, table_id); // Will generate and persist some information in local ps, which will not be uploaded. From 9b3dec7ee4a1ccd51e5bae7329787412b73d7f4d Mon Sep 17 00:00:00 2001 From: Calvin Neo Date: Fri, 26 Jul 2024 13:31:07 +0800 Subject: [PATCH 06/10] addr Signed-off-by: Calvin Neo --- .../src/Storages/KVStore/tests/gtest_kvstore_fast_add_peer.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dbms/src/Storages/KVStore/tests/gtest_kvstore_fast_add_peer.cpp b/dbms/src/Storages/KVStore/tests/gtest_kvstore_fast_add_peer.cpp index 17251a82eb5..cff99dc5ee6 100644 --- a/dbms/src/Storages/KVStore/tests/gtest_kvstore_fast_add_peer.cpp +++ b/dbms/src/Storages/KVStore/tests/gtest_kvstore_fast_add_peer.cpp @@ -950,7 +950,8 @@ try auto & storages = tmt.getStorages(); storages.remove(keyspace_id, table_id); // Will generate and persist some information in local ps, which will not be uploaded. - FastAddPeerImplWrite(global_context.getTMTContext(), proxy_helper.get(), region_id, 2333, std::move(mock_data), 0); + auto res = FastAddPeerImplWrite(global_context.getTMTContext(), proxy_helper.get(), region_id, 2333, std::move(mock_data), 0); + ASSERT_EQ(res.status, FastAddPeerStatus::BadData); } CATCH From 66ba9d50e32c21d75b36b54c0ee8761dfd9edb57 Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Fri, 26 Jul 2024 13:29:24 +0800 Subject: [PATCH 07/10] Refine the sign of AtomicGetStorageSchema --- .../Storages/KVStore/Decode/PartitionStreams.cpp | 16 +++++++--------- .../Storages/KVStore/Decode/PartitionStreams.h | 2 +- .../KVStore/MultiRaft/Disagg/FastAddPeer.cpp | 2 +- .../KVStore/MultiRaft/PrehandleSnapshot.cpp | 6 +++--- 4 files changed, 12 insertions(+), 14 deletions(-) diff --git a/dbms/src/Storages/KVStore/Decode/PartitionStreams.cpp b/dbms/src/Storages/KVStore/Decode/PartitionStreams.cpp index 8e55016e017..d8490348738 100644 --- a/dbms/src/Storages/KVStore/Decode/PartitionStreams.cpp +++ b/dbms/src/Storages/KVStore/Decode/PartitionStreams.cpp @@ -510,20 +510,18 @@ RegionTable::ResolveLocksAndWriteRegionRes RegionTable::resolveLocksAndWriteRegi // - Check whether the StorageDeltaMerge is nullptr or not before you accessing to it. // - You should hold the `TableLockHolder` when you're accessing to the IStorage instance. std::tuple, DecodingStorageSchemaSnapshotConstPtr> // -AtomicGetStorageSchema(const RegionPtr & region, TMTContext & tmt) +AtomicGetStorageSchema(RegionID region_id, KeyspaceID keyspace_id, TableID table_id, TMTContext & tmt) { TableLockHolder drop_lock = nullptr; std::shared_ptr dm_storage; DecodingStorageSchemaSnapshotConstPtr schema_snapshot; - auto keyspace_id = region->getKeyspaceID(); - auto table_id = region->getMappedTableID(); LOG_DEBUG( Logger::get("AtomicGetStorageSchema"), - "Get schema, keyspace={} table_id={} region_id={}", + "Get schema, region_id={} keyspace={} table_id={}", + region_id, keyspace_id, - table_id, - region->id()); + table_id); auto & context = tmt.getContext(); const auto atomic_get = [&](bool force_decode) -> bool { auto storage = tmt.getStorages().get(keyspace_id, table_id); @@ -552,12 +550,12 @@ AtomicGetStorageSchema(const RegionPtr & region, TMTContext & tmt) auto schema_sync_cost = watch.elapsedMilliseconds(); LOG_INFO( Logger::get("AtomicGetStorageSchema"), - "sync schema cost {} ms, sync_schema_ok={} keyspace={} table_id={} region_id={}", + "sync schema cost {} ms, sync_schema_ok={} region_id={} keyspace={} table_id={}", schema_sync_cost, sync_schema_ok, + region_id, keyspace_id, - table_id, - region->id()); + table_id); // try get with `force_decode == true` atomic_get(true); diff --git a/dbms/src/Storages/KVStore/Decode/PartitionStreams.h b/dbms/src/Storages/KVStore/Decode/PartitionStreams.h index ee3c28a0a2b..ff380e1b417 100644 --- a/dbms/src/Storages/KVStore/Decode/PartitionStreams.h +++ b/dbms/src/Storages/KVStore/Decode/PartitionStreams.h @@ -37,7 +37,7 @@ void RemoveRegionCommitCache( bool lock_region = true); std::tuple, DecodingStorageSchemaSnapshotConstPtr> // -AtomicGetStorageSchema(const RegionPtr & region, TMTContext & tmt); +AtomicGetStorageSchema(RegionID region_id, KeyspaceID keyspace_id, TableID table_id, TMTContext & tmt); Block GenRegionBlockDataWithSchema( const RegionPtr & region, // diff --git a/dbms/src/Storages/KVStore/MultiRaft/Disagg/FastAddPeer.cpp b/dbms/src/Storages/KVStore/MultiRaft/Disagg/FastAddPeer.cpp index 40062f228bf..10aabaafbce 100644 --- a/dbms/src/Storages/KVStore/MultiRaft/Disagg/FastAddPeer.cpp +++ b/dbms/src/Storages/KVStore/MultiRaft/Disagg/FastAddPeer.cpp @@ -308,7 +308,7 @@ FastAddPeerRes FastAddPeerImplWrite( auto keyspace_id = region->getKeyspaceID(); auto table_id = region->getMappedTableID(); - const auto [table_drop_lock, storage, schema_snap] = AtomicGetStorageSchema(region, tmt); + const auto [table_drop_lock, storage, schema_snap] = AtomicGetStorageSchema(region_id, keyspace_id, table_id, tmt); if (!storage) { LOG_WARNING( diff --git a/dbms/src/Storages/KVStore/MultiRaft/PrehandleSnapshot.cpp b/dbms/src/Storages/KVStore/MultiRaft/PrehandleSnapshot.cpp index ffaa688ea45..255064b8ff5 100644 --- a/dbms/src/Storages/KVStore/MultiRaft/PrehandleSnapshot.cpp +++ b/dbms/src/Storages/KVStore/MultiRaft/PrehandleSnapshot.cpp @@ -659,7 +659,7 @@ PrehandleResult KVStore::preHandleSSTsToDTFiles( }); PrehandleResult prehandle_result; - TableID physical_table_id = InvalidTableID; + TableID physical_table_id = new_region->getMappedTableID(); auto region_id = new_region->id(); auto prehandle_task = prehandling_trace.registerTask(region_id); @@ -671,7 +671,8 @@ PrehandleResult KVStore::preHandleSSTsToDTFiles( { // Get storage schema atomically, will do schema sync if the storage does not exists. // Will return the storage even if it is tombstone. - const auto [table_drop_lock, storage, schema_snap] = AtomicGetStorageSchema(new_region, tmt); + const auto [table_drop_lock, storage, schema_snap] + = AtomicGetStorageSchema(region_id, keyspace_id, physical_table_id, tmt); if (unlikely(storage == nullptr)) { // The storage must be physically dropped, throw exception and do cleanup. @@ -688,7 +689,6 @@ PrehandleResult KVStore::preHandleSSTsToDTFiles( /* ignore_cache= */ false, context.getSettingsRef().safe_point_update_interval_seconds); } - physical_table_id = storage->getTableInfo().id; auto opt = DM::SSTFilesToBlockInputStreamOpts{ .log_prefix = fmt::format("keyspace={} table_id={}", keyspace_id, physical_table_id), From bc3c92efd512982eb035f4660ac18cbbbc323641 Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Fri, 26 Jul 2024 13:35:29 +0800 Subject: [PATCH 08/10] Align logging of keyspace --- dbms/src/Storages/KVStore/MultiRaft/ApplySnapshot.cpp | 2 +- .../KVStore/MultiRaft/Disagg/CheckpointIngestInfo.cpp | 4 ++-- .../Storages/KVStore/MultiRaft/Disagg/FastAddPeer.cpp | 10 +++++----- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/dbms/src/Storages/KVStore/MultiRaft/ApplySnapshot.cpp b/dbms/src/Storages/KVStore/MultiRaft/ApplySnapshot.cpp index 7bb8d5e945a..0f1bc76b438 100644 --- a/dbms/src/Storages/KVStore/MultiRaft/ApplySnapshot.cpp +++ b/dbms/src/Storages/KVStore/MultiRaft/ApplySnapshot.cpp @@ -190,7 +190,7 @@ void KVStore::onSnapshot( LOG_INFO( log, "clear old range before apply snapshot, region_id={} old_range={} new_range={} " - "keyspace_id={} table_id={}", + "keyspace={} table_id={}", region_id, old_key_range.toDebugString(), new_key_range.toDebugString(), diff --git a/dbms/src/Storages/KVStore/MultiRaft/Disagg/CheckpointIngestInfo.cpp b/dbms/src/Storages/KVStore/MultiRaft/Disagg/CheckpointIngestInfo.cpp index c25fe5f9871..8c90c841c42 100644 --- a/dbms/src/Storages/KVStore/MultiRaft/Disagg/CheckpointIngestInfo.cpp +++ b/dbms/src/Storages/KVStore/MultiRaft/Disagg/CheckpointIngestInfo.cpp @@ -217,7 +217,7 @@ void CheckpointIngestInfo::deleteWrittenData(TMTContext & tmt, RegionPtr region, { LOG_INFO( log, - "No found storage in clean stale FAP data region_id={} keyspace_id={} table_id={}", + "No found storage in clean stale FAP data region_id={} keyspace={} table_id={}", region_id, keyspace_id, table_id); @@ -241,7 +241,7 @@ void CheckpointIngestInfo::deleteWrittenData(TMTContext & tmt, RegionPtr region, LOG_INFO( log, - "Finish clean stale FAP data region_id={} keyspace_id={} table_id={}", + "Finish clean stale FAP data region_id={} keyspace={} table_id={}", region_id, keyspace_id, table_id); diff --git a/dbms/src/Storages/KVStore/MultiRaft/Disagg/FastAddPeer.cpp b/dbms/src/Storages/KVStore/MultiRaft/Disagg/FastAddPeer.cpp index 10aabaafbce..d74cdf80de1 100644 --- a/dbms/src/Storages/KVStore/MultiRaft/Disagg/FastAddPeer.cpp +++ b/dbms/src/Storages/KVStore/MultiRaft/Disagg/FastAddPeer.cpp @@ -313,7 +313,7 @@ FastAddPeerRes FastAddPeerImplWrite( { LOG_WARNING( log, - "FAP failed because the table can not be found, region_id={} keyspace_id={} table_id={}", + "FAP failed because the table can not be found, region_id={} keyspace={} table_id={}", region_id, keyspace_id, table_id); @@ -332,7 +332,7 @@ FastAddPeerRes FastAddPeerImplWrite( { LOG_INFO( log, - "FAP is canceled before write, region_id={} keyspace_id={} table_id={}", + "FAP is canceled before write, region_id={} keyspace={} table_id={}", region_id, keyspace_id, table_id); @@ -372,7 +372,7 @@ FastAddPeerRes FastAddPeerImplWrite( { LOG_INFO( log, - "FAP is canceled after write segments, region_id={} keyspace_id={} table_id={}", + "FAP is canceled after write segments, region_id={} keyspace={} table_id={}", region_id, keyspace_id, table_id); @@ -407,7 +407,7 @@ FastAddPeerRes FastAddPeerImplWrite( { LOG_INFO( log, - "FAP is canceled after write raft log, region_id={} keyspace_id={} table_id={}", + "FAP is canceled after write raft log, region_id={} keyspace={} table_id={}", region_id, keyspace_id, table_id); @@ -417,7 +417,7 @@ FastAddPeerRes FastAddPeerImplWrite( } LOG_DEBUG( log, - "Finish write FAP snapshot, region_id={} keyspace_id={} table_id={}", + "Finish write FAP snapshot, region_id={} keyspace={} table_id={}", region_id, keyspace_id, table_id); From 2d75a6e0dc318217ad6577e58540d092bd4f0dbf Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Fri, 26 Jul 2024 13:59:51 +0800 Subject: [PATCH 09/10] Format files --- .../KVStore/MultiRaft/Disagg/CheckpointIngestInfo.cpp | 7 +------ .../Storages/KVStore/MultiRaft/Disagg/FastAddPeer.cpp | 7 +------ .../KVStore/tests/gtest_kvstore_fast_add_peer.cpp | 9 +++++++-- 3 files changed, 9 insertions(+), 14 deletions(-) diff --git a/dbms/src/Storages/KVStore/MultiRaft/Disagg/CheckpointIngestInfo.cpp b/dbms/src/Storages/KVStore/MultiRaft/Disagg/CheckpointIngestInfo.cpp index 8c90c841c42..be220967915 100644 --- a/dbms/src/Storages/KVStore/MultiRaft/Disagg/CheckpointIngestInfo.cpp +++ b/dbms/src/Storages/KVStore/MultiRaft/Disagg/CheckpointIngestInfo.cpp @@ -239,12 +239,7 @@ void CheckpointIngestInfo::deleteWrittenData(TMTContext & tmt, RegionPtr region, }); wn_ps->write(std::move(wb)); - LOG_INFO( - log, - "Finish clean stale FAP data region_id={} keyspace={} table_id={}", - region_id, - keyspace_id, - table_id); + LOG_INFO(log, "Finish clean stale FAP data region_id={} keyspace={} table_id={}", region_id, keyspace_id, table_id); } bool CheckpointIngestInfo::cleanOnSuccess(TMTContext & tmt, UInt64 region_id) diff --git a/dbms/src/Storages/KVStore/MultiRaft/Disagg/FastAddPeer.cpp b/dbms/src/Storages/KVStore/MultiRaft/Disagg/FastAddPeer.cpp index d74cdf80de1..2b30f6b02ac 100644 --- a/dbms/src/Storages/KVStore/MultiRaft/Disagg/FastAddPeer.cpp +++ b/dbms/src/Storages/KVStore/MultiRaft/Disagg/FastAddPeer.cpp @@ -415,12 +415,7 @@ FastAddPeerRes FastAddPeerImplWrite( GET_METRIC(tiflash_fap_task_result, type_failed_cancel).Increment(); return genFastAddPeerRes(FastAddPeerStatus::Canceled, "", ""); } - LOG_DEBUG( - log, - "Finish write FAP snapshot, region_id={} keyspace={} table_id={}", - region_id, - keyspace_id, - table_id); + LOG_DEBUG(log, "Finish write FAP snapshot, region_id={} keyspace={} table_id={}", region_id, keyspace_id, table_id); return genFastAddPeerRes( FastAddPeerStatus::Ok, apply_state.SerializeAsString(), diff --git a/dbms/src/Storages/KVStore/tests/gtest_kvstore_fast_add_peer.cpp b/dbms/src/Storages/KVStore/tests/gtest_kvstore_fast_add_peer.cpp index d309aaeb3b7..15e2373a34f 100644 --- a/dbms/src/Storages/KVStore/tests/gtest_kvstore_fast_add_peer.cpp +++ b/dbms/src/Storages/KVStore/tests/gtest_kvstore_fast_add_peer.cpp @@ -950,8 +950,13 @@ try // Mock that the storage instance have been dropped auto & storages = tmt.getStorages(); storages.remove(keyspace_id, table_id); - // Will generate and persist some information in local ps, which will not be uploaded. - auto res = FastAddPeerImplWrite(global_context.getTMTContext(), proxy_helper.get(), region_id, 2333, std::move(mock_data), 0); + auto res = FastAddPeerImplWrite( + global_context.getTMTContext(), + proxy_helper.get(), + region_id, + 2333, + std::move(mock_data), + 0); ASSERT_EQ(res.status, FastAddPeerStatus::BadData); } CATCH From 3425a426a85cd5133ba8ac0edd9a08f64e808f01 Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Fri, 26 Jul 2024 14:19:01 +0800 Subject: [PATCH 10/10] Fix lint --- dbms/src/Storages/KVStore/Decode/PartitionStreams.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/dbms/src/Storages/KVStore/Decode/PartitionStreams.cpp b/dbms/src/Storages/KVStore/Decode/PartitionStreams.cpp index d8490348738..4497f1e9c49 100644 --- a/dbms/src/Storages/KVStore/Decode/PartitionStreams.cpp +++ b/dbms/src/Storages/KVStore/Decode/PartitionStreams.cpp @@ -527,10 +527,9 @@ AtomicGetStorageSchema(RegionID region_id, KeyspaceID keyspace_id, TableID table auto storage = tmt.getStorages().get(keyspace_id, table_id); if (storage == nullptr) { - if (!force_decode) - return false; + // If force_decode == false, then should sync schema and check again // Else force_decode == true, and storage instance still not exist, table must have just been GC-ed - return true; + return force_decode; } // Get a structure read lock. It will throw exception if the table has been dropped, // the caller should handle this situation.