From c88e53e22151403bed63e3b4c3379e44282266bb Mon Sep 17 00:00:00 2001 From: Tong Zhigao Date: Mon, 11 Nov 2019 16:51:51 +0800 Subject: [PATCH 01/12] finished --- dbms/src/Debug/MockTiDB.h | 6 +- dbms/src/Debug/dbgFuncRegion.cpp | 2 +- dbms/src/Flash/Coprocessor/InterpreterDAG.cpp | 2 +- dbms/src/Flash/CoprocessorHandler.cpp | 9 +- .../MergeTree/MergeTreeDataSelectExecutor.cpp | 26 +- dbms/src/Storages/StorageMergeTree.cpp | 5 +- dbms/src/Storages/Transaction/KVStore.cpp | 54 +-- dbms/src/Storages/Transaction/KVStore.h | 2 +- .../Storages/Transaction/PartitionStreams.cpp | 24 +- .../Storages/Transaction/RaftCommandResult.h | 3 +- dbms/src/Storages/Transaction/Region.cpp | 168 ++++---- dbms/src/Storages/Transaction/Region.h | 74 ++-- .../Storages/Transaction/RegionCFDataBase.cpp | 91 +---- .../Storages/Transaction/RegionCFDataBase.h | 14 +- dbms/src/Storages/Transaction/RegionData.cpp | 82 ++-- dbms/src/Storages/Transaction/RegionData.h | 16 +- .../Storages/Transaction/RegionException.h | 32 +- .../src/Storages/Transaction/RegionHelper.hpp | 18 +- dbms/src/Storages/Transaction/RegionMeta.cpp | 1 + .../Storages/Transaction/RegionRangeKeys.h | 3 + dbms/src/Storages/Transaction/RegionState.cpp | 23 +- dbms/src/Storages/Transaction/RegionTable.cpp | 370 ++++++------------ dbms/src/Storages/Transaction/RegionTable.h | 115 ++---- .../Transaction/RegionTableCheckOptimize.cpp | 4 +- dbms/src/Storages/Transaction/Types.h | 3 - .../Storages/Transaction/applySnapshot.cpp | 28 +- dbms/src/Storages/Transaction/applySnapshot.h | 4 +- 27 files changed, 477 insertions(+), 702 deletions(-) diff --git a/dbms/src/Debug/MockTiDB.h b/dbms/src/Debug/MockTiDB.h index 20afa87144a..2a32759a311 100644 --- a/dbms/src/Debug/MockTiDB.h +++ b/dbms/src/Debug/MockTiDB.h @@ -55,8 +55,8 @@ class MockTiDB : public ext::singleton using TablePtr = std::shared_ptr; public: - TableID newTable(const String & database_name, const String & table_name, - const ColumnsDescription & columns, Timestamp tso, const String & handle_pk_name); + TableID newTable(const String & database_name, const String & table_name, const ColumnsDescription & columns, Timestamp tso, + const String & handle_pk_name); DatabaseID newDataBase(const String & database_name); @@ -109,7 +109,7 @@ class MockTiDB : public ext::singleton std::unordered_map version_diff; - std::atomic table_id_allocator = MaxSystemTableID + 1; + std::atomic table_id_allocator = 30; Int64 version = 0; }; diff --git a/dbms/src/Debug/dbgFuncRegion.cpp b/dbms/src/Debug/dbgFuncRegion.cpp index 1e8131454d0..aece9492b66 100644 --- a/dbms/src/Debug/dbgFuncRegion.cpp +++ b/dbms/src/Debug/dbgFuncRegion.cpp @@ -154,7 +154,7 @@ void dbgFuncRegionSnapshotWithData(Context & context, const ASTs & args, DBGInvo MockTiKV::instance().getRaftIndex(region_id); } - applySnapshot(tmt.getKVStore(), region, &context); + applySnapshot(tmt.getKVStore(), region, &context, false); std::stringstream ss; ss << "put region #" << region_id << ", range[" << start << ", " << end << ")" diff --git a/dbms/src/Flash/Coprocessor/InterpreterDAG.cpp b/dbms/src/Flash/Coprocessor/InterpreterDAG.cpp index 9e5122ec71a..dd8b3424cf4 100644 --- a/dbms/src/Flash/Coprocessor/InterpreterDAG.cpp +++ b/dbms/src/Flash/Coprocessor/InterpreterDAG.cpp @@ -266,7 +266,7 @@ void InterpreterDAG::executeTS(const tipb::TableScan & ts, Pipeline & pipeline) { std::vector region_ids; region_ids.push_back(info.region_id); - throw RegionException(std::move(region_ids), RegionTable::RegionReadStatus::NOT_FOUND); + throw RegionException(std::move(region_ids), RegionException::NOT_FOUND); } if (!checkKeyRanges(dag.getKeyRanges(), table_id, storage->pkIsUInt64(), current_region->getRange())) throw Exception("Cop request only support full range scan for given region", ErrorCodes::COP_BAD_DAG_REQUEST); diff --git a/dbms/src/Flash/CoprocessorHandler.cpp b/dbms/src/Flash/CoprocessorHandler.cpp index 510e343d2e7..315f12df519 100644 --- a/dbms/src/Flash/CoprocessorHandler.cpp +++ b/dbms/src/Flash/CoprocessorHandler.cpp @@ -22,8 +22,7 @@ CoprocessorHandler::CoprocessorHandler( : cop_context(cop_context_), cop_request(cop_request_), cop_response(cop_response_), log(&Logger::get("CoprocessorHandler")) {} -grpc::Status CoprocessorHandler::execute() -try +grpc::Status CoprocessorHandler::execute() try { switch (cop_request->tp()) { @@ -81,12 +80,12 @@ catch (const RegionException & e) errorpb::Error * region_err; switch (e.status) { - case RegionTable::RegionReadStatus::NOT_FOUND: - case RegionTable::RegionReadStatus::PENDING_REMOVE: + case RegionException::RegionReadStatus::NOT_FOUND: + case RegionException::RegionReadStatus::PENDING_REMOVE: region_err = cop_response->mutable_region_error(); region_err->mutable_region_not_found()->set_region_id(cop_request->context().region_id()); break; - case RegionTable::RegionReadStatus::VERSION_ERROR: + case RegionException::RegionReadStatus::VERSION_ERROR: region_err = cop_response->mutable_region_error(); region_err->mutable_epoch_not_match(); break; diff --git a/dbms/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp b/dbms/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp index 80929e0a3fc..9bfc2c3fc14 100644 --- a/dbms/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp +++ b/dbms/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp @@ -223,7 +223,7 @@ BlockInputStreams MergeTreeDataSelectExecutor::read(const Names & column_names_t // the index of column is constant after MergeTreeBlockInputStream is constructed. exception will be thrown if not found. const size_t handle_column_index = 0, version_column_index = 1, delmark_column_index = 2; - const auto func_throw_retry_region = [&](RegionTable::RegionReadStatus status) { + const auto func_throw_retry_region = [&](RegionException::RegionReadStatus status) { std::vector region_ids; region_ids.reserve(regions_executor_data.size()); for (const auto & query_info : regions_executor_data) @@ -315,7 +315,7 @@ BlockInputStreams MergeTreeDataSelectExecutor::read(const Names & column_names_t if (region == nullptr) { LOG_WARNING(log, "[region " << query_info.info.region_id << "] is not found in KVStore, try again"); - func_throw_retry_region(RegionTable::RegionReadStatus::NOT_FOUND); + func_throw_retry_region(RegionException::RegionReadStatus::NOT_FOUND); } kvstore_region.emplace(query_info.info.region_id, std::move(region)); } @@ -332,13 +332,13 @@ BlockInputStreams MergeTreeDataSelectExecutor::read(const Names & column_names_t auto start_time = Clock::now(); const size_t mem_region_num = regions_executor_data.size(); const size_t batch_size = mem_region_num / concurrent_num; - std::atomic_uint8_t region_status = RegionTable::RegionReadStatus::OK; + std::atomic_uint8_t region_status = RegionException::RegionReadStatus::OK; const auto func_run_learner_read = [&](const size_t region_begin) { const size_t region_end = std::min(region_begin + batch_size, mem_region_num); for (size_t region_index = region_begin; region_index < region_end; ++region_index) { - if (region_status != RegionTable::RegionReadStatus::OK) + if (region_status != RegionException::RegionReadStatus::OK) return; RegionQueryInfo & region_query_info = regions_executor_data[region_index].info; @@ -353,13 +353,13 @@ BlockInputStreams MergeTreeDataSelectExecutor::read(const Names & column_names_t kvstore_region[region_query_info.region_id], region_query_info.version, region_query_info.conf_version, mvcc_query_info.resolve_locks, mvcc_query_info.read_tso, region_query_info.range_in_table); - if (status != RegionTable::OK) + if (status != RegionException::RegionReadStatus::OK) { LOG_WARNING(log, "Check memory cache, region " << region_query_info.region_id << ", version " << region_query_info.version << ", handle range [" << region_query_info.range_in_table.first.toString() << ", " << region_query_info.range_in_table.second.toString() << ") , status " - << RegionTable::RegionReadStatusString(status)); + << RegionException::RegionReadStatusString(status)); region_status = status; } else if (block) @@ -380,8 +380,8 @@ BlockInputStreams MergeTreeDataSelectExecutor::read(const Names & column_names_t func_run_learner_read(0); } - if (region_status != RegionTable::RegionReadStatus::OK) - func_throw_retry_region(static_cast(region_status.load())); + if (region_status != RegionException::RegionReadStatus::OK) + func_throw_retry_region(static_cast(region_status.load())); auto end_time = Clock::now(); LOG_DEBUG(log, @@ -846,13 +846,13 @@ BlockInputStreams MergeTreeDataSelectExecutor::read(const Names & column_names_t // check all data, include special region. { auto region = tmt.getKVStore()->getRegion(region_query_info.region_id); - RegionTable::RegionReadStatus status = RegionTable::OK; + RegionException::RegionReadStatus status = RegionException::RegionReadStatus::OK; if (region != kvstore_region[region_query_info.region_id]) - status = RegionTable::NOT_FOUND; + status = RegionException::RegionReadStatus::NOT_FOUND; else if (region->version() != region_query_info.version) - status = RegionTable::VERSION_ERROR; + status = RegionException::RegionReadStatus::VERSION_ERROR; - if (status != RegionTable::OK) + if (status != RegionException::RegionReadStatus::OK) { // ABA problem may cause because one region is removed and inserted back. // if the version of region is changed, the part may has less data because of compaction. @@ -861,7 +861,7 @@ BlockInputStreams MergeTreeDataSelectExecutor::read(const Names & column_names_t << region_query_info.version << ", handle range [" << region_query_info.range_in_table.first.toString() << ", " << region_query_info.range_in_table.second.toString() << ") , status " - << RegionTable::RegionReadStatusString(status)); + << RegionException::RegionReadStatusString(status)); // throw exception and exit. func_throw_retry_region(status); } diff --git a/dbms/src/Storages/StorageMergeTree.cpp b/dbms/src/Storages/StorageMergeTree.cpp index 111a82fef0d..e8662a2e392 100644 --- a/dbms/src/Storages/StorageMergeTree.cpp +++ b/dbms/src/Storages/StorageMergeTree.cpp @@ -115,7 +115,6 @@ void StorageMergeTree::shutdown() { TMTContext & tmt_context = context.getTMTContext(); tmt_context.getStorages().remove(data.table_info->id); - tmt_context.getRegionTable().removeTable(data.table_info->id); } } @@ -365,7 +364,7 @@ void StorageMergeTree::alterInternal( else if (param.type == AlterCommand::RENAME_COLUMN) { rename_column = true; - if (params.size() != 1) + if (params.size() != 1) { throw Exception("There is an internal error for rename columns", ErrorCodes::LOGICAL_ERROR); } @@ -717,7 +716,7 @@ void StorageMergeTree::dropPartition(const ASTPtr & /*query*/, const ASTPtr & pa void StorageMergeTree::truncate(const ASTPtr & /*query*/, const Context & /*context*/) { auto lock = lockForAlter(__PRETTY_FUNCTION__); - + MergeTreeData::DataParts parts = data.getDataParts(); for (const auto & part : parts) diff --git a/dbms/src/Storages/Transaction/KVStore.cpp b/dbms/src/Storages/Transaction/KVStore.cpp index 7e82cfa2df3..644ed017b4c 100644 --- a/dbms/src/Storages/Transaction/KVStore.cpp +++ b/dbms/src/Storages/Transaction/KVStore.cpp @@ -90,12 +90,25 @@ void KVStore::traverseRegions(std::function & callback(it->first, it->second); } -bool KVStore::onSnapshot(RegionPtr new_region, Context * context, const RegionsAppliedindexMap & regions_to_check) +bool KVStore::onSnapshot(RegionPtr new_region, Context * context, const RegionsAppliedindexMap & regions_to_check, bool try_flush_region) { RegionID region_id = new_region->id(); + + if (context) { - auto region_lock = region_manager.genRegionTaskLock(region_id); - region_persister.persist(*new_region, region_lock); + const auto range = new_region->getRange(); + auto & region_table = context->getTMTContext().getRegionTable(); + // extend region to make sure data won't be removed. + region_table.extendRegionRange(region_id, *range); + // try to flush data into ch first. + try + { + if (try_flush_region) + region_table.tryFlushRegion(new_region, false); + } + catch (...) + { + } } { @@ -108,20 +121,15 @@ bool KVStore::onSnapshot(RegionPtr new_region, Context * context, const RegionsA if (auto it = regions().find(region_info.first); it != regions().end()) { - if (it->second != region) + if (it->second != region || region->appliedIndex() != region_info.second.second) { - LOG_WARNING(log, "[onSnapshot] " << it->second->toString() << " instance changed"); - return false; - } - if (region->appliedIndex() != region_info.second.second) - { - LOG_WARNING(log, "[onSnapshot] " << it->second->toString() << " instance changed"); + LOG_WARNING(log, __FUNCTION__ << ": instance changed region " << region_info.first); return false; } } else { - LOG_WARNING(log, "[onSnapshot] " << region->toString(false) << " not found"); + LOG_WARNING(log, __FUNCTION__ << ": not found " << region->toString(false)); return false; } } @@ -129,7 +137,7 @@ bool KVStore::onSnapshot(RegionPtr new_region, Context * context, const RegionsA RegionPtr old_region = getRegion(region_id); if (old_region != nullptr) { - LOG_DEBUG(log, "[onSnapshot] previous " << old_region->toString(true) << " ; new " << new_region->toString(true)); + LOG_DEBUG(log, __FUNCTION__ << ": previous " << old_region->toString(true) << " ; new " << new_region->toString(true)); region_range_index.remove(old_region->makeRaftCommandDelegate(task_lock).getRange().comparableKeys(), region_id); old_region->assignRegion(std::move(*new_region)); new_region = old_region; @@ -140,14 +148,14 @@ bool KVStore::onSnapshot(RegionPtr new_region, Context * context, const RegionsA regionsMut().emplace(region_id, new_region); } + region_persister.persist(*new_region, region_lock); region_range_index.add(new_region); - } - // if the operation about RegionTable is out of the protection of task_mutex, we should make sure that it can't delete any mapping relation. - if (context) - { - context->getRaftService().addRegionToDecode(new_region); - context->getTMTContext().getRegionTable().applySnapshotRegion(*new_region); + if (context) + { + context->getRaftService().addRegionToDecode(new_region); + context->getTMTContext().getRegionTable().shrinkRegionRange(*new_region); + } } return true; @@ -253,7 +261,7 @@ void KVStore::onServiceCommand(enginepb::CommandRequestBatch && cmds, RaftContex // is always >= range in KVStore. for (const auto & new_region : split_regions) { - region_table->updateRegionForSplit(*new_region, curr_region_id); + region_table->updateRegion(*new_region); raft_service->addRegionToFlush(*new_region); } region_table->shrinkRegionRange(curr_region); @@ -274,9 +282,9 @@ void KVStore::onServiceCommand(enginepb::CommandRequestBatch && cmds, RaftContex report_sync_log(); }; - const auto handle_update_table_ids = [&](const TableIDSet & table_ids) { + const auto handle_update_table = [&]() { if (region_table) - region_table->updateRegion(curr_region, table_ids); + region_table->updateRegion(curr_region); persist_and_sync(); }; @@ -299,8 +307,8 @@ void KVStore::onServiceCommand(enginepb::CommandRequestBatch && cmds, RaftContex case RaftCommandResult::Type::BatchSplit: handle_batch_split(result.split_regions); break; - case RaftCommandResult::Type::UpdateTableID: - handle_update_table_ids(result.table_ids); + case RaftCommandResult::Type::UpdateTable: + handle_update_table(); raft_service->addRegionToDecode(curr_region_ptr); break; case RaftCommandResult::Type::Default: diff --git a/dbms/src/Storages/Transaction/KVStore.h b/dbms/src/Storages/Transaction/KVStore.h index 10726c3678f..ee758dcd4b3 100644 --- a/dbms/src/Storages/Transaction/KVStore.h +++ b/dbms/src/Storages/Transaction/KVStore.h @@ -44,7 +44,7 @@ class KVStore final : private boost::noncopyable void traverseRegions(std::function && callback) const; - bool onSnapshot(RegionPtr new_region, Context * context, const RegionsAppliedindexMap & regions_to_check = {}); + bool onSnapshot(RegionPtr new_region, Context * context, const RegionsAppliedindexMap & regions_to_check = {}, bool try_flush_region = false); // TODO: remove RaftContext and use Context + CommandServerReaderWriter void onServiceCommand(enginepb::CommandRequestBatch && cmds, RaftContext & context); diff --git a/dbms/src/Storages/Transaction/PartitionStreams.cpp b/dbms/src/Storages/Transaction/PartitionStreams.cpp index 2f2c5efe186..0660540edef 100644 --- a/dbms/src/Storages/Transaction/PartitionStreams.cpp +++ b/dbms/src/Storages/Transaction/PartitionStreams.cpp @@ -20,16 +20,15 @@ namespace ErrorCodes extern const int LOGICAL_ERROR; } // namespace ErrorCodes -void RegionTable::writeBlockByRegion( - Context & context, TableID table_id, RegionPtr region, RegionDataReadInfoList & data_list_to_remove, Logger * log) +void RegionTable::writeBlockByRegion(Context & context, RegionPtr region, RegionDataReadInfoList & data_list_to_remove, Logger * log) { const auto & tmt = context.getTMTContext(); - + TableID table_id = region->getFlashTableID(); UInt64 region_read_cost = -1, region_decode_cost = -1, write_part_cost = -1; RegionDataReadInfoList data_list_read; { - auto scanner = region->createCommittedScanner(table_id); + auto scanner = region->createCommittedScanner(); /// Some sanity checks for region meta. { @@ -117,7 +116,7 @@ void RegionTable::writeBlockByRegion( << ", region decode " << region_decode_cost << ", write part " << write_part_cost << "] ms"); } -std::tuple RegionTable::readBlockByRegion(const TiDB::TableInfo & table_info, +std::tuple RegionTable::readBlockByRegion(const TiDB::TableInfo & table_info, const ColumnsDescription & columns, const Names & column_names_to_read, const RegionPtr & region, @@ -128,20 +127,23 @@ std::tuple RegionTable::readBlockByRegion( DB::HandleRange & handle_range) { if (!region) - throw Exception("[RegionTable::readBlockByRegion] region is null", ErrorCodes::LOGICAL_ERROR); + throw Exception(std::string(__PRETTY_FUNCTION__) + ": region is null", ErrorCodes::LOGICAL_ERROR); + + if (region->getFlashTableID() != table_info.id) + throw Exception(std::string(__PRETTY_FUNCTION__) + ": table id not match", ErrorCodes::LOGICAL_ERROR); RegionDataReadInfoList data_list_read; { - auto scanner = region->createCommittedScanner(table_info.id); + auto scanner = region->createCommittedScanner(); /// Some sanity checks for region meta. { if (region->isPendingRemove()) - return {Block(), PENDING_REMOVE}; + return {Block(), RegionException::PENDING_REMOVE}; const auto & [version, conf_ver, key_range] = region->dumpVersionRange(); if (version != region_version || conf_ver != conf_version) - return {Block(), VERSION_ERROR}; + return {Block(), RegionException::VERSION_ERROR}; handle_range = key_range->getHandleRangeByTable(table_info.id); } @@ -164,7 +166,7 @@ std::tuple RegionTable::readBlockByRegion( { // Shortcut for empty region. if (!scanner.hasNext()) - return {Block(), OK}; + return {Block(), RegionException::OK}; data_list_read.reserve(scanner.writeMapSize()); @@ -188,7 +190,7 @@ std::tuple RegionTable::readBlockByRegion( ErrorCodes::LOGICAL_ERROR); } - return {std::move(block), OK}; + return {std::move(block), RegionException::OK}; } } // namespace DB diff --git a/dbms/src/Storages/Transaction/RaftCommandResult.h b/dbms/src/Storages/Transaction/RaftCommandResult.h index 3af0d202087..5d981ee1257 100644 --- a/dbms/src/Storages/Transaction/RaftCommandResult.h +++ b/dbms/src/Storages/Transaction/RaftCommandResult.h @@ -20,7 +20,7 @@ struct RaftCommandResult : private boost::noncopyable Default, IndexError, BatchSplit, - UpdateTableID, + UpdateTable, ChangePeer }; @@ -28,7 +28,6 @@ struct RaftCommandResult : private boost::noncopyable Type type = Type::Default; std::vector split_regions{}; - TableIDSet table_ids{}; ImutRegionRangePtr range_before_split; }; diff --git a/dbms/src/Storages/Transaction/Region.cpp b/dbms/src/Storages/Transaction/Region.cpp index 086b089a7d3..a700bf7b058 100644 --- a/dbms/src/Storages/Transaction/Region.cpp +++ b/dbms/src/Storages/Transaction/Region.cpp @@ -22,73 +22,75 @@ const std::string Region::default_cf_name = "default"; const std::string Region::write_cf_name = "write"; const std::string Region::log_name = "Region"; -RegionData::WriteCFIter Region::removeDataByWriteIt(const TableID & table_id, const RegionData::WriteCFIter & write_it) -{ - return data.removeDataByWriteIt(table_id, write_it); -} +RegionData::WriteCFIter Region::removeDataByWriteIt(const RegionData::WriteCFIter & write_it) { return data.removeDataByWriteIt(write_it); } -RegionDataReadInfo Region::readDataByWriteIt(const TableID & table_id, const RegionData::ConstWriteCFIter & write_it, bool need_value) const +RegionDataReadInfo Region::readDataByWriteIt(const RegionData::ConstWriteCFIter & write_it, bool need_value) const { - return data.readDataByWriteIt(table_id, write_it, need_value); + return data.readDataByWriteIt(write_it, need_value); } -LockInfoPtr Region::getLockInfo(TableID expected_table_id, UInt64 start_ts) const { return data.getLockInfo(expected_table_id, start_ts); } +LockInfoPtr Region::getLockInfo(UInt64 start_ts) const { return data.getLockInfo(start_ts); } -TableID Region::insert(const std::string & cf, TiKVKey && key, TiKVValue && value) +void Region::insert(const std::string & cf, TiKVKey && key, TiKVValue && value) { std::unique_lock lock(mutex); return doInsert(cf, std::move(key), std::move(value)); } -TableID Region::doInsert(const std::string & cf, TiKVKey && key, TiKVValue && value) +void Region::doInsert(const std::string & cf, TiKVKey && key, TiKVValue && value) { auto raw_key = RecordKVFormat::decodeTiKVKey(key); - auto table_id = checkRecordAndValidTable(raw_key); - if (table_id == InvalidTableID) - return InvalidTableID; + doCheckTable(raw_key); auto type = getCf(cf); - return data.insert(type, std::move(key), raw_key, std::move(value)); + data.insert(type, std::move(key), raw_key, std::move(value)); } -TableID Region::remove(const std::string & cf, const TiKVKey & key) +void Region::doCheckTable(const DB::DecodedTiKVKey & raw_key) const +{ + auto table_id = RecordKVFormat::getTableId(raw_key); + if (table_id != getFlashTableID()) + { + LOG_ERROR(log, __FUNCTION__ << ": table id not match, except " << getFlashTableID() << ", got " << table_id); + throw Exception(std::string(__PRETTY_FUNCTION__) + ": table id not match"); + } +} + +void Region::remove(const std::string & cf, const TiKVKey & key) { std::unique_lock lock(mutex); - return doRemove(cf, key); + doRemove(cf, key); } -TableID Region::doRemove(const std::string & cf, const TiKVKey & key) +void Region::doRemove(const std::string & cf, const TiKVKey & key) { auto raw_key = RecordKVFormat::decodeTiKVKey(key); - auto table_id = checkRecordAndValidTable(raw_key); - if (table_id == InvalidTableID) - return InvalidTableID; + doCheckTable(raw_key); auto type = getCf(cf); switch (type) { case Lock: - data.removeLockCF(table_id, raw_key); + data.removeLockCF(raw_key); break; case Default: { // there may be some prewrite data, may not exist, don't throw exception. - data.removeDefaultCF(table_id, key, raw_key); + data.removeDefaultCF(key, raw_key); break; } case Write: { // removed by gc, may not exist. - data.removeWriteCF(table_id, key, raw_key); + data.removeWriteCF(key, raw_key); break; } } - return table_id; } UInt64 Region::appliedIndex() const { return meta.appliedIndex(); } -RegionPtr Region::splitInto(RegionMeta meta) +RegionPtr Region::splitInto(RegionMeta && meta) { RegionPtr new_region; if (index_reader != nullptr) @@ -257,9 +259,6 @@ void RegionRaftCommandDelegate::onCommand(enginepb::CommandRequest && cmd, const } else { - TableIDSet table_ids; - TableID table_id = InvalidTableID; - std::unique_lock lock(mutex); for (auto && req : *cmd.mutable_requests()) @@ -280,7 +279,7 @@ void RegionRaftCommandDelegate::onCommand(enginepb::CommandRequest && cmd, const try { - table_id = doInsert(put.cf(), std::move(tikv_key), std::move(tikv_value)); + doInsert(put.cf(), std::move(tikv_key), std::move(tikv_value)); } catch (Exception & e) { @@ -290,11 +289,7 @@ void RegionRaftCommandDelegate::onCommand(enginepb::CommandRequest && cmd, const e.rethrow(); } - if (table_id != InvalidTableID) - { - table_ids.emplace(table_id); - is_dirty = true; - } + is_dirty = true; break; } case raft_cmdpb::CmdType::Delete: @@ -306,7 +301,7 @@ void RegionRaftCommandDelegate::onCommand(enginepb::CommandRequest && cmd, const try { - table_id = doRemove(del.cf(), tikv_key); + doRemove(del.cf(), tikv_key); } catch (Exception & e) { @@ -316,11 +311,7 @@ void RegionRaftCommandDelegate::onCommand(enginepb::CommandRequest && cmd, const e.rethrow(); } - if (table_id != InvalidTableID) - { - table_ids.emplace(table_id); - is_dirty = true; - } + is_dirty = true; break; } case raft_cmdpb::CmdType::Snap: @@ -350,8 +341,7 @@ void RegionRaftCommandDelegate::onCommand(enginepb::CommandRequest && cmd, const } } meta.setApplied(index, term); - result.type = RaftCommandResult::Type::UpdateTableID; - result.table_ids = std::move(table_ids); + result.type = RaftCommandResult::Type::UpdateTable; } meta.notifyAll(); @@ -388,8 +378,9 @@ RegionPtr Region::deserialize(ReadBuffer & buf, const IndexReaderCreateFunc * in "[Region::deserialize] unexpected version: " + DB::toString(version) + ", expected: " + DB::toString(CURRENT_VERSION), ErrorCodes::UNKNOWN_FORMAT_VERSION); - auto region = index_reader_create == nullptr ? std::make_shared(RegionMeta::deserialize(buf)) - : std::make_shared(RegionMeta::deserialize(buf), *index_reader_create); + auto meta = RegionMeta::deserialize(buf); + auto region = index_reader_create == nullptr ? std::make_shared(std::move(meta)) + : std::make_shared(std::move(meta), *index_reader_create); RegionData::deserialize(buf, region->data); @@ -457,15 +448,9 @@ void Region::decDirtyFlag(size_t x) const { dirty_flag -= x; } void Region::incDirtyFlag() { dirty_flag++; } -Region::CommittedScanner Region::createCommittedScanner(TableID expected_table_id) -{ - return Region::CommittedScanner(this->shared_from_this(), expected_table_id); -} +Region::CommittedScanner Region::createCommittedScanner() { return Region::CommittedScanner(this->shared_from_this()); } -Region::CommittedRemover Region::createCommittedRemover(TableID expected_table_id) -{ - return Region::CommittedRemover(this->shared_from_this(), expected_table_id); -} +Region::CommittedRemover Region::createCommittedRemover() { return Region::CommittedRemover(this->shared_from_this()); } std::string Region::toString(bool dump_status) const { return meta.toString(dump_status); } @@ -513,21 +498,15 @@ void Region::assignRegion(Region && new_region) bool Region::isPeerRemoved() const { return meta.isPeerRemoved(); } -TableIDSet Region::getAllWriteCFTables() const -{ - std::shared_lock lock(mutex); - return data.getAllWriteCFTables(); -} - -void Region::compareAndCompleteSnapshot(HandleMap & handle_map, const TableID table_id, const Timestamp safe_point) +void Region::compareAndCompleteSnapshot(HandleMap & handle_map, const Timestamp safe_point) { std::unique_lock lock(mutex); if (handle_map.empty()) return; - auto & region_data = data.writeCF().getDataMut(); - auto & write_map = region_data[table_id]; + auto table_id = getFlashTableID(); + auto & write_map = data.writeCF().getDataMut(); size_t deleted_gc_cnt = 0, ori_write_map_size = write_map.size(); @@ -548,9 +527,9 @@ void Region::compareAndCompleteSnapshot(HandleMap & handle_map, const TableID ta if (is_deleted != ori_del) { LOG_ERROR(log, - "[compareAndCompleteSnapshot] WriteType is not equal, handle: " << handle << ", tso: " << ts << ", original: " - << ori_del << " , current: " << is_deleted); - throw Exception("[Region::compareAndCompleteSnapshot] original ts >= gc safe point", ErrorCodes::LOGICAL_ERROR); + __FUNCTION__ << ": WriteType is not equal, handle: " << handle << ", tso: " << ts << ", original: " << ori_del + << " , current: " << is_deleted); + throw Exception(std::string(__PRETTY_FUNCTION__) + ": original ts >= gc safe point", ErrorCodes::LOGICAL_ERROR); } handle_map.erase(it); } @@ -567,7 +546,7 @@ void Region::compareAndCompleteSnapshot(HandleMap & handle_map, const TableID ta std::ignore = ori_del; if (ori_ts >= safe_point) - throw Exception("[Region::compareAndCompleteSnapshot] original ts >= gc safe point", ErrorCodes::LOGICAL_ERROR); + throw Exception(std::string(__PRETTY_FUNCTION__) + ": original ts >= gc safe point", ErrorCodes::LOGICAL_ERROR); auto raw_key = RecordKVFormat::genRawKey(table_id, handle); TiKVKey key = RecordKVFormat::encodeAsTiKVKey(raw_key); @@ -579,10 +558,10 @@ void Region::compareAndCompleteSnapshot(HandleMap & handle_map, const TableID ta } LOG_DEBUG(log, - "[compareAndCompleteSnapshot] table " << table_id << ", gc safe point " << safe_point << ", original write map size " - << ori_write_map_size << ", remain size " << write_map.size()); + __FUNCTION__ << ": table " << table_id << ", gc safe point " << safe_point << ", original write map size " << ori_write_map_size + << ", remain size " << write_map.size()); if (deleted_gc_cnt) - LOG_INFO(log, "[compareAndCompleteSnapshot] add deleted gc: " << deleted_gc_cnt); + LOG_INFO(log, __FUNCTION__ << ": add deleted gc: " << deleted_gc_cnt); } RegionRaftCommandDelegate & Region::makeRaftCommandDelegate(const KVStoreTaskLock & lock) @@ -593,43 +572,37 @@ RegionRaftCommandDelegate & Region::makeRaftCommandDelegate(const KVStoreTaskLoc return static_cast(*this); } -void Region::compareAndUpdateHandleMaps(const Region & source_region, std::unordered_map & handle_maps) +void Region::compareAndUpdateHandleMaps(const Region & source_region, HandleMap & handle_map) { const auto range = getRange(); const auto & [start_key, end_key] = range->comparableKeys(); { std::shared_lock source_lock(source_region.mutex); - const auto & region_data = source_region.data.writeCF().getData(); - for (auto & [table_id, write_map] : region_data) + const auto & write_map = source_region.data.writeCF().getData(); + if (write_map.empty()) + return; + + for (auto write_map_it = write_map.begin(); write_map_it != write_map.end(); ++write_map_it) { - if (write_map.empty()) + const auto & key = RegionWriteCFData::getTiKVKey(write_map_it->second); + + if (start_key.compare(key) <= 0 && end_key.compare(key) > 0) + ; + else continue; - auto & handle_map = handle_maps[table_id]; - for (auto write_map_it = write_map.begin(); write_map_it != write_map.end(); ++write_map_it) + const auto & [handle, ts] = write_map_it->first; + const HandleMap::mapped_type cur_ele = {ts, RegionData::getWriteType(write_map_it) == DelFlag}; + auto [it, ok] = handle_map.emplace(handle, cur_ele); + if (!ok) { - const auto & key = RegionWriteCFData::getTiKVKey(write_map_it->second); - - if (start_key.compare(key) <= 0 && end_key.compare(key) > 0) - ; - else - continue; - - const auto & [handle, ts] = write_map_it->first; - const HandleMap::mapped_type cur_ele = {ts, RegionData::getWriteType(write_map_it) == DelFlag}; - auto [it, ok] = handle_map.emplace(handle, cur_ele); - if (!ok) - { - auto & ele = it->second; - ele = std::max(ele, cur_ele); - } + auto & ele = it->second; + ele = std::max(ele, cur_ele); } - - LOG_DEBUG(log, - "[compareAndUpdateHandleMaps] memory cache: source " << source_region.toString(false) << ", table " << table_id - << ", record size " << write_map.size()); } + + LOG_DEBUG(log, __FUNCTION__ << ": memory cache: source " << source_region.toString(false) << ", record size " << write_map.size()); } } @@ -647,6 +620,17 @@ void Region::tryPreDecodeTiKVValue() DB::tryPreDecodeTiKVValue(data.writeCF().getExtra().popAll()); } +Region::Region(RegionMeta && meta_) : Region(std::move(meta_), [](pingcap::kv::RegionVerID) { return nullptr; }) {} + +Region::Region(DB::RegionMeta && meta_, const DB::IndexReaderCreateFunc & index_reader_create) + : meta(std::move(meta_)), + index_reader(index_reader_create(meta.getRegionVerID())), + log(&Logger::get(log_name)), + flash_table_id(meta.getRange()->getFlashTableID()) +{} + +TableID Region::getFlashTableID() const { return flash_table_id; } + const RegionRangeKeys & RegionRaftCommandDelegate::getRange() { return *meta.makeRaftCommandDelegate().regionState().getRange(); } UInt64 RegionRaftCommandDelegate::appliedIndex() { return meta.makeRaftCommandDelegate().applyState().applied_index(); } metapb::Region Region::getMetaRegion() const { return meta.getMetaRegion(); } diff --git a/dbms/src/Storages/Transaction/Region.h b/dbms/src/Storages/Transaction/Region.h index 1300adb3b8b..9ab63cf7058 100644 --- a/dbms/src/Storages/Transaction/Region.h +++ b/dbms/src/Storages/Transaction/Region.h @@ -39,23 +39,20 @@ class Region : public std::enable_shared_from_this class CommittedScanner : private boost::noncopyable { public: - CommittedScanner(const RegionPtr & store_, TableID expected_table_id_) - : store(store_), lock(store_->mutex), expected_table_id(expected_table_id_) + CommittedScanner(const RegionPtr & store_) : store(store_), lock(store_->mutex) { const auto & data = store->data.writeCF().getData(); - if (auto it = data.find(expected_table_id); it != data.end()) - { - write_map_size = it->second.size(); - write_map_it = it->second.begin(); - write_map_it_end = it->second.end(); - } + + write_map_size = data.size(); + write_map_it = data.begin(); + write_map_it_end = data.end(); } bool hasNext() const { return write_map_size && write_map_it != write_map_it_end; } - auto next(bool need_value = true) { return store->readDataByWriteIt(expected_table_id, write_map_it++, need_value); } + auto next(bool need_value = true) { return store->readDataByWriteIt(write_map_it++, need_value); } - LockInfoPtr getLockInfo(UInt64 start_ts) { return store->getLockInfo(expected_table_id, start_ts); } + LockInfoPtr getLockInfo(UInt64 start_ts) { return store->getLockInfo(start_ts); } size_t writeMapSize() const { return write_map_size; } @@ -64,7 +61,6 @@ class Region : public std::enable_shared_from_this std::shared_lock lock; size_t write_map_size = 0; - TableID expected_table_id; RegionData::ConstWriteCFIter write_map_it; RegionData::ConstWriteCFIter write_map_it_end; }; @@ -72,41 +68,29 @@ class Region : public std::enable_shared_from_this class CommittedRemover : private boost::noncopyable { public: - CommittedRemover(const RegionPtr & store_, TableID expected_table_id_) : store(store_), lock(store_->mutex) - { - auto & data = store->data.writeCF().getDataMut(); - write_cf_data_it = data.find(expected_table_id_); - found = write_cf_data_it != data.end(); - } + CommittedRemover(const RegionPtr & store_) : store(store_), lock(store_->mutex) {} void remove(const RegionWriteCFData::Key & key) { - if (!found) - return; - if (auto it = write_cf_data_it->second.find(key); it != write_cf_data_it->second.end()) - store->removeDataByWriteIt(write_cf_data_it->first, it); + auto & write_cf_data = store->data.writeCF().getDataMut(); + if (auto it = write_cf_data.find(key); it != write_cf_data.end()) + store->removeDataByWriteIt(it); } private: RegionPtr store; std::unique_lock lock; - - bool found; - RegionWriteCFData::Data::iterator write_cf_data_it; }; public: - explicit Region(RegionMeta meta_) : meta(std::move(meta_)), index_reader(nullptr), log(&Logger::get(log_name)) {} + explicit Region(RegionMeta && meta_); + explicit Region(RegionMeta && meta_, const IndexReaderCreateFunc & index_reader_create); - explicit Region(RegionMeta meta_, const IndexReaderCreateFunc & index_reader_create) - : meta(std::move(meta_)), index_reader(index_reader_create(meta.getRegionVerID())), log(&Logger::get(log_name)) - {} + void insert(const std::string & cf, TiKVKey && key, TiKVValue && value); + void remove(const std::string & cf, const TiKVKey & key); - TableID insert(const std::string & cf, TiKVKey && key, TiKVValue && value); - TableID remove(const std::string & cf, const TiKVKey & key); - - CommittedScanner createCommittedScanner(TableID expected_table_id); - CommittedRemover createCommittedRemover(TableID expected_table_id); + CommittedScanner createCommittedScanner(); + CommittedRemover createCommittedRemover(); std::tuple serialize(WriteBuffer & buf) const; static RegionPtr deserialize(ReadBuffer & buf, const IndexReaderCreateFunc * index_reader_create = nullptr); @@ -156,15 +140,13 @@ class Region : public std::enable_shared_from_this void assignRegion(Region && new_region); - TableIDSet getAllWriteCFTables() const; - 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 TableID table_id, const Timestamp safe_point); + void compareAndCompleteSnapshot(HandleMap & handle_map, const Timestamp safe_point); /// Traverse all data in source_region and get handle with largest version. - void compareAndUpdateHandleMaps(const Region & source_region, std::unordered_map & handle_maps); + void compareAndUpdateHandleMaps(const Region & source_region, HandleMap & handle_map); static ColumnFamilyType getCf(const std::string & cf); RegionRaftCommandDelegate & makeRaftCommandDelegate(const KVStoreTaskLock &); @@ -173,23 +155,25 @@ class Region : public std::enable_shared_from_this void tryPreDecodeTiKVValue(); + TableID getFlashTableID() const; + private: Region() = delete; friend class RegionRaftCommandDelegate; // Private methods no need to lock mutex, normally - TableID doInsert(const std::string & cf, TiKVKey && key, TiKVValue && value); - TableID doRemove(const std::string & cf, const TiKVKey & key); + void doInsert(const std::string & cf, TiKVKey && key, TiKVValue && value); + void doCheckTable(const DecodedTiKVKey & key) const; + void doRemove(const std::string & cf, const TiKVKey & key); void doDeleteRange(const std::string & cf, const RegionRange & range); - RegionDataReadInfo readDataByWriteIt( - const TableID & table_id, const RegionData::ConstWriteCFIter & write_it, bool need_value = true) const; - RegionData::WriteCFIter removeDataByWriteIt(const TableID & table_id, const RegionData::WriteCFIter & write_it); + RegionDataReadInfo readDataByWriteIt(const RegionData::ConstWriteCFIter & write_it, bool need_value = true) const; + RegionData::WriteCFIter removeDataByWriteIt(const RegionData::WriteCFIter & write_it); - LockInfoPtr getLockInfo(TableID expected_table_id, UInt64 start_ts) const; + LockInfoPtr getLockInfo(UInt64 start_ts) const; - RegionPtr splitInto(RegionMeta meta); + RegionPtr splitInto(RegionMeta && meta); private: RegionData data; @@ -205,6 +189,8 @@ class Region : public std::enable_shared_from_this mutable std::atomic dirty_flag = 1; Logger * log; + + const TableID flash_table_id; }; class RegionRaftCommandDelegate : public Region, private boost::noncopyable diff --git a/dbms/src/Storages/Transaction/RegionCFDataBase.cpp b/dbms/src/Storages/Transaction/RegionCFDataBase.cpp index 3884cf47dba..01aeac076ab 100644 --- a/dbms/src/Storages/Transaction/RegionCFDataBase.cpp +++ b/dbms/src/Storages/Transaction/RegionCFDataBase.cpp @@ -28,26 +28,26 @@ const TiKVValue & RegionCFDataBase::getTiKVValue(const Value & val) } template -TableID RegionCFDataBase::insert(TiKVKey && key, TiKVValue && value) +RegionDataRes RegionCFDataBase::insert(TiKVKey && key, TiKVValue && value) { const auto & raw_key = RecordKVFormat::decodeTiKVKey(key); return insert(std::move(key), std::move(value), raw_key); } template -TableID RegionCFDataBase::insert(TiKVKey && key, TiKVValue && value, const DecodedTiKVKey & raw_key) +RegionDataRes RegionCFDataBase::insert(TiKVKey && key, TiKVValue && value, const DecodedTiKVKey & raw_key) { Pair kv_pair = Trait::genKVPair(std::move(key), raw_key, std::move(value)); if (shouldIgnoreInsert(kv_pair.second)) - return InvalidTableID; + return false; - return insert(RecordKVFormat::getTableId(raw_key), std::move(kv_pair)); + return insert(std::move(kv_pair)); } template -TableID RegionCFDataBase::insert(const TableID table_id, std::pair && kv_pair) +RegionDataRes RegionCFDataBase::insert(std::pair && kv_pair) { - auto & map = data[table_id]; + auto & map = data; auto [it, ok] = map.emplace(std::move(kv_pair)); if (!ok) throw Exception("Found existing key in hex: " + getTiKVKey(it->second).toHex(), ErrorCodes::LOGICAL_ERROR); @@ -56,7 +56,7 @@ TableID RegionCFDataBase::insert(const TableID table_id, std::pairsecond)); else extra.add(getTiKVValuePtr(it->second)); - return table_id; + return true; } template @@ -102,9 +102,9 @@ bool RegionCFDataBase::shouldIgnoreInsert(const RegionCF } template -size_t RegionCFDataBase::remove(TableID table_id, const Key & key, bool quiet) +size_t RegionCFDataBase::remove(const Key & key, bool quiet) { - auto & map = data[table_id]; + auto & map = data; if (auto it = map.find(key); it != map.end()) { @@ -144,33 +144,13 @@ bool RegionCFDataBase::cmp(const Map & a, const Map & b) template bool RegionCFDataBase::operator==(const RegionCFDataBase & cf) const { - if (getSize() != cf.getSize()) - return false; - - const auto & cf_data = cf.data; - for (const auto & [table_id, map] : data) - { - if (map.empty()) - continue; - - if (auto it = cf_data.find(table_id); it != cf_data.end()) - { - if (!cmp(map, it->second)) - return false; - } - else - return false; - } - return true; + return cmp(cf.data, data); } template size_t RegionCFDataBase::getSize() const { - size_t size = 0; - for (auto data_it = data.begin(); data_it != data.end(); ++data_it) - size += data_it->second.size(); - return size; + return data.size(); } template @@ -191,17 +171,9 @@ size_t RegionCFDataBase::splitInto(const RegionRange & range, RegionCFDat const auto & [start_key, end_key] = range; size_t size_changed = 0; - for (auto data_it = data.begin(); data_it != data.end();) { - const auto & table_id = data_it->first; - auto & ori_map = data_it->second; - if (ori_map.empty()) - { - data_it = data.erase(data_it); - continue; - } - - auto & tar_map = new_region_data.data[table_id]; + auto & ori_map = data; + auto & tar_map = new_region_data.data; for (auto it = ori_map.begin(); it != ori_map.end();) { @@ -216,8 +188,6 @@ size_t RegionCFDataBase::splitInto(const RegionRange & range, RegionCFDat else ++it; } - - ++data_it; } return size_changed; } @@ -231,16 +201,12 @@ size_t RegionCFDataBase::serialize(WriteBuffer & buf) const total_size += writeBinary2(size, buf); - for (const auto & [table_id, map] : data) + for (const auto & ele : data) { - std::ignore = table_id; - for (const auto & ele : map) - { - const auto & key = getTiKVKey(ele.second); - const auto & value = getTiKVValue(ele.second); - total_size += key.serialize(buf); - total_size += value.serialize(buf); - } + const auto & key = getTiKVKey(ele.second); + const auto & value = getTiKVValue(ele.second); + total_size += key.serialize(buf); + total_size += value.serialize(buf); } return total_size; @@ -262,19 +228,6 @@ size_t RegionCFDataBase::deserialize(ReadBuffer & buf, RegionCFDataBase & return cf_data_size; } -template -TableIDSet RegionCFDataBase::getAllTables() const -{ - TableIDSet tables; - for (const auto & [table_id, map] : data) - { - if (map.empty()) - continue; - tables.insert(table_id); - } - return tables; -} - template const typename RegionCFDataBase::Data & RegionCFDataBase::getData() const { @@ -294,9 +247,8 @@ size_t RegionCFDataBase::deleteRange(const RegionRange & range) const auto & [start_key, end_key] = range; - for (auto data_it = data.begin(); data_it != data.end();) { - auto & ori_map = data_it->second; + auto & ori_map = data; for (auto it = ori_map.begin(); it != ori_map.end();) { @@ -310,11 +262,6 @@ size_t RegionCFDataBase::deleteRange(const RegionRange & range) else ++it; } - - if (ori_map.empty()) - data_it = data.erase(data_it); - else - ++data_it; } return size_changed; diff --git a/dbms/src/Storages/Transaction/RegionCFDataBase.h b/dbms/src/Storages/Transaction/RegionCFDataBase.h index 3024860f51c..dfc421d2c3d 100644 --- a/dbms/src/Storages/Transaction/RegionCFDataBase.h +++ b/dbms/src/Storages/Transaction/RegionCFDataBase.h @@ -22,6 +22,7 @@ enum CFModifyFlag : UInt8 struct TiKVRangeKey; using RegionRange = std::pair; +using RegionDataRes = bool; template struct RegionCFDataBase @@ -29,22 +30,23 @@ struct RegionCFDataBase using Key = typename Trait::Key; using Value = typename Trait::Value; using Map = typename Trait::Map; - using Data = std::unordered_map; + using Data = Map; using Pair = std::pair; + using Status = bool; static const TiKVKey & getTiKVKey(const Value & val); static const TiKVValue & getTiKVValue(const Value & val); - TableID insert(TiKVKey && key, TiKVValue && value); - TableID insert(const TableID table_id, std::pair && kv_pair); - TableID insert(TiKVKey && key, TiKVValue && value, const DecodedTiKVKey & raw_key); + RegionDataRes insert(TiKVKey && key, TiKVValue && value); + RegionDataRes insert(std::pair && kv_pair); + RegionDataRes insert(TiKVKey && key, TiKVValue && value, const DecodedTiKVKey & raw_key); static size_t calcTiKVKeyValueSize(const Value & value); static size_t calcTiKVKeyValueSize(const TiKVKey & key, const TiKVValue & value); - size_t remove(TableID table_id, const Key & key, bool quiet = false); + size_t remove(const Key & key, bool quiet = false); static bool cmp(const Map & a, const Map & b); @@ -66,8 +68,6 @@ struct RegionCFDataBase Data & getDataMut(); - TableIDSet getAllTables() const; - size_t deleteRange(const RegionRange & range); ExtraCFData & getExtra(); diff --git a/dbms/src/Storages/Transaction/RegionData.cpp b/dbms/src/Storages/Transaction/RegionData.cpp index ccb10b0e092..f0e3875840d 100644 --- a/dbms/src/Storages/Transaction/RegionData.cpp +++ b/dbms/src/Storages/Transaction/RegionData.cpp @@ -5,56 +5,57 @@ namespace DB { -TableID RegionData::insert(ColumnFamilyType cf, TiKVKey && key, const DecodedTiKVKey & raw_key, TiKVValue && value) +void RegionData::insert(ColumnFamilyType cf, TiKVKey && key, const DecodedTiKVKey & raw_key, TiKVValue && value) { switch (cf) { case Write: { size_t size = key.dataSize() + value.dataSize(); - auto table_id = write_cf.insert(std::move(key), std::move(value), raw_key); - if (table_id != InvalidTableID) + auto res = write_cf.insert(std::move(key), std::move(value), raw_key); + if (res) cf_data_size += size; - return table_id; + return; } case Default: { size_t size = key.dataSize() + value.dataSize(); - auto table_id = default_cf.insert(std::move(key), std::move(value), raw_key); + default_cf.insert(std::move(key), std::move(value), raw_key); cf_data_size += size; - return table_id; + return; } case Lock: { - return lock_cf.insert(std::move(key), std::move(value), raw_key); + lock_cf.insert(std::move(key), std::move(value), raw_key); + return; } default: - throw Exception("RegionData::insert with undefined CF, should not happen", ErrorCodes::LOGICAL_ERROR); + throw Exception(std::string(__PRETTY_FUNCTION__) + " with undefined CF, should not happen", ErrorCodes::LOGICAL_ERROR); } } -void RegionData::removeLockCF(const TableID & table_id, const DecodedTiKVKey & raw_key) +void RegionData::removeLockCF(const DecodedTiKVKey & raw_key) { HandleID handle_id = RecordKVFormat::getHandle(raw_key); - lock_cf.remove(table_id, handle_id); + lock_cf.remove(handle_id); } -void RegionData::removeDefaultCF(const TableID & table_id, const TiKVKey & key, const DecodedTiKVKey & raw_key) +void RegionData::removeDefaultCF(const TiKVKey & key, const DecodedTiKVKey & raw_key) { HandleID handle_id = RecordKVFormat::getHandle(raw_key); Timestamp ts = RecordKVFormat::getTs(key); - cf_data_size -= default_cf.remove(table_id, RegionDefaultCFData::Key{handle_id, ts}, true); + cf_data_size -= default_cf.remove(RegionDefaultCFData::Key{handle_id, ts}, true); } -void RegionData::removeWriteCF(const TableID & table_id, const TiKVKey & key, const DecodedTiKVKey & raw_key) +void RegionData::removeWriteCF(const TiKVKey & key, const DecodedTiKVKey & raw_key) { HandleID handle_id = RecordKVFormat::getHandle(raw_key); Timestamp ts = RecordKVFormat::getTs(key); - cf_data_size -= write_cf.remove(table_id, RegionWriteCFData::Key{handle_id, ts}, true); + cf_data_size -= write_cf.remove(RegionWriteCFData::Key{handle_id, ts}, true); } -RegionData::WriteCFIter RegionData::removeDataByWriteIt(const TableID & table_id, const WriteCFIter & write_it) +RegionData::WriteCFIter RegionData::removeDataByWriteIt(const WriteCFIter & write_it) { const auto & [key, value, decoded_val] = write_it->second; const auto & [handle, ts] = write_it->first; @@ -65,7 +66,7 @@ RegionData::WriteCFIter RegionData::removeDataByWriteIt(const TableID & table_id if (write_type == PutFlag && !short_str) { - auto & map = default_cf.getDataMut()[table_id]; + auto & map = default_cf.getDataMut(); if (auto data_it = map.find({handle, prewrite_ts}); data_it != map.end()) { @@ -78,10 +79,10 @@ RegionData::WriteCFIter RegionData::removeDataByWriteIt(const TableID & table_id cf_data_size -= RegionWriteCFData::calcTiKVKeyValueSize(write_it->second); - return write_cf.getDataMut()[table_id].erase(write_it); + return write_cf.getDataMut().erase(write_it); } -RegionDataReadInfo RegionData::readDataByWriteIt(const TableID & table_id, const ConstWriteCFIter & write_it, bool need_value) const +RegionDataReadInfo RegionData::readDataByWriteIt(const ConstWriteCFIter & write_it, bool need_value) const { const auto & [key, value, decoded_val] = write_it->second; const auto & [handle, ts] = write_it->first; @@ -99,41 +100,32 @@ RegionDataReadInfo RegionData::readDataByWriteIt(const TableID & table_id, const if (short_value) return std::make_tuple(handle, write_type, ts, short_value); - if (auto map_it = default_cf.getData().find(table_id); map_it != default_cf.getData().end()) - { - const auto & map = map_it->second; - if (auto data_it = map.find({handle, prewrite_ts}); data_it != map.end()) - return std::make_tuple(handle, write_type, ts, std::get<1>(data_it->second)); - else - throw Exception(" key [" + key->toString() + "] not found in data cf", ErrorCodes::LOGICAL_ERROR); - } + + const auto & map = default_cf.getData(); + if (auto data_it = map.find({handle, prewrite_ts}); data_it != map.end()) + return std::make_tuple(handle, write_type, ts, std::get<1>(data_it->second)); else - throw Exception(" table [" + toString(table_id) + "] not found in data cf", ErrorCodes::LOGICAL_ERROR); + throw Exception(" key [" + key->toString() + "] not found in data cf", ErrorCodes::LOGICAL_ERROR); } -LockInfoPtr RegionData::getLockInfo(TableID expected_table_id, Timestamp start_ts) const +LockInfoPtr RegionData::getLockInfo(Timestamp start_ts) const { - if (auto it = lock_cf.getData().find(expected_table_id); it != lock_cf.getData().end()) + for (const auto & [handle, value] : lock_cf.getData()) { - for (const auto & [handle, value] : it->second) - { - std::ignore = handle; + std::ignore = handle; - const auto & [tikv_key, tikv_val, decoded_val] = value; - const auto & [lock_type, primary, ts, ttl, data] = decoded_val; - std::ignore = tikv_val; - std::ignore = data; + const auto & [tikv_key, tikv_val, decoded_val] = value; + const auto & [lock_type, primary, ts, ttl, data] = decoded_val; + std::ignore = tikv_val; + std::ignore = data; - if (lock_type == DelFlag || ts > start_ts) - continue; + if (lock_type == DelFlag || ts > start_ts) + continue; - return std::make_unique(LockInfo{primary, ts, RecordKVFormat::decodeTiKVKey(*tikv_key), ttl}); - } - - return nullptr; + return std::make_unique(LockInfo{primary, ts, RecordKVFormat::decodeTiKVKey(*tikv_key), ttl}); } - else - return nullptr; + + return nullptr; } void RegionData::splitInto(const RegionRange & range, RegionData & new_region_data) @@ -185,8 +177,6 @@ const RegionWriteCFData & RegionData::writeCF() const { return write_cf; } const RegionDefaultCFData & RegionData::defaultCF() const { return default_cf; } const RegionLockCFData & RegionData::lockCF() const { return lock_cf; } -TableIDSet RegionData::getAllWriteCFTables() const { return writeCF().getAllTables(); } - bool RegionData::isEqual(const RegionData & r2) const { return default_cf == r2.default_cf && write_cf == r2.write_cf && lock_cf == r2.lock_cf && cf_data_size == r2.cf_data_size; diff --git a/dbms/src/Storages/Transaction/RegionData.h b/dbms/src/Storages/Transaction/RegionData.h index 1213e64b417..45f996871a6 100644 --- a/dbms/src/Storages/Transaction/RegionData.h +++ b/dbms/src/Storages/Transaction/RegionData.h @@ -25,17 +25,17 @@ class RegionData using WriteCFIter = RegionWriteCFData::Map::iterator; using ConstWriteCFIter = RegionWriteCFData::Map::const_iterator; - TableID insert(ColumnFamilyType cf, TiKVKey && key, const DecodedTiKVKey & raw_key, TiKVValue && value); + void insert(ColumnFamilyType cf, TiKVKey && key, const DecodedTiKVKey & raw_key, TiKVValue && value); - void removeLockCF(const TableID & table_id, const DecodedTiKVKey & raw_key); - void removeDefaultCF(const TableID & table_id, const TiKVKey & key, const DecodedTiKVKey & raw_key); - void removeWriteCF(const TableID & table_id, const TiKVKey & key, const DecodedTiKVKey & raw_key); + void removeLockCF(const DecodedTiKVKey & raw_key); + void removeDefaultCF(const TiKVKey & key, const DecodedTiKVKey & raw_key); + void removeWriteCF(const TiKVKey & key, const DecodedTiKVKey & raw_key); - WriteCFIter removeDataByWriteIt(const TableID & table_id, const WriteCFIter & write_it); + WriteCFIter removeDataByWriteIt(const WriteCFIter & write_it); - RegionDataReadInfo readDataByWriteIt(const TableID & table_id, const ConstWriteCFIter & write_it, bool need_value = true) const; + RegionDataReadInfo readDataByWriteIt(const ConstWriteCFIter & write_it, bool need_value = true) const; - LockInfoPtr getLockInfo(TableID expected_table_id, Timestamp start_ts) const; + LockInfoPtr getLockInfo(Timestamp start_ts) const; void splitInto(const RegionRange & range, RegionData & new_region_data); @@ -58,8 +58,6 @@ class RegionData const RegionDefaultCFData & defaultCF() const; const RegionLockCFData & lockCF() const; - TableIDSet getAllWriteCFTables() const; - RegionData() {} RegionData(RegionData && data); diff --git a/dbms/src/Storages/Transaction/RegionException.h b/dbms/src/Storages/Transaction/RegionException.h index dfa20262dd4..25a23ec307b 100644 --- a/dbms/src/Storages/Transaction/RegionException.h +++ b/dbms/src/Storages/Transaction/RegionException.h @@ -1,7 +1,6 @@ #pragma once #include -#include #include namespace DB @@ -10,12 +9,37 @@ namespace DB class RegionException : public Exception { public: - RegionException(std::vector && region_ids_, RegionTable::RegionReadStatus status_) - : Exception(RegionTable::RegionReadStatusString(status_)), region_ids(region_ids_), status(status_) + enum RegionReadStatus : UInt8 + { + OK, + NOT_FOUND, + VERSION_ERROR, + PENDING_REMOVE, + }; + + static const char * RegionReadStatusString(RegionReadStatus s) + { + switch (s) + { + case OK: + return "OK"; + case NOT_FOUND: + return "NOT_FOUND"; + case VERSION_ERROR: + return "VERSION_ERROR"; + case PENDING_REMOVE: + return "PENDING_REMOVE"; + } + return "Unknown"; + }; + +public: + RegionException(std::vector && region_ids_, RegionReadStatus status_) + : Exception(RegionReadStatusString(status_)), region_ids(region_ids_), status(status_) {} std::vector region_ids; - RegionTable::RegionReadStatus status; + RegionReadStatus status; }; } // namespace DB diff --git a/dbms/src/Storages/Transaction/RegionHelper.hpp b/dbms/src/Storages/Transaction/RegionHelper.hpp index e8fb3bb7169..b01fee9fdc0 100644 --- a/dbms/src/Storages/Transaction/RegionHelper.hpp +++ b/dbms/src/Storages/Transaction/RegionHelper.hpp @@ -3,21 +3,7 @@ namespace DB { -/// Ignoring all keys other than records. -inline TableID checkRecordAndValidTable(const DecodedTiKVKey & raw_key) -{ - // Ignoring all keys other than records. - if (!RecordKVFormat::isRecord(raw_key)) - return InvalidTableID; - - auto table_id = RecordKVFormat::getTableId(raw_key); - if (isTiDBSystemTable(table_id)) - return InvalidTableID; - - return table_id; -} - -void tryPreDecodeTiKVValue(std::optional && values) +inline void tryPreDecodeTiKVValue(std::optional && values) { if (!values) return; @@ -32,7 +18,7 @@ void tryPreDecodeTiKVValue(std::optional && values) } } -const metapb::Peer & findPeer(const metapb::Region & region, UInt64 store_id) +inline const metapb::Peer & findPeer(const metapb::Region & region, UInt64 store_id) { for (const auto & peer : region.peers()) { diff --git a/dbms/src/Storages/Transaction/RegionMeta.cpp b/dbms/src/Storages/Transaction/RegionMeta.cpp index e46be8e01f3..1d885ab03b7 100644 --- a/dbms/src/Storages/Transaction/RegionMeta.cpp +++ b/dbms/src/Storages/Transaction/RegionMeta.cpp @@ -1,5 +1,6 @@ #include #include +#include #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wunused-parameter" diff --git a/dbms/src/Storages/Transaction/RegionRangeKeys.h b/dbms/src/Storages/Transaction/RegionRangeKeys.h index 1e3bf0475b7..dfabdea8ef2 100644 --- a/dbms/src/Storages/Transaction/RegionRangeKeys.h +++ b/dbms/src/Storages/Transaction/RegionRangeKeys.h @@ -41,10 +41,13 @@ class RegionRangeKeys : boost::noncopyable const std::pair & rawKeys() const; HandleRange getHandleRangeByTable(const TableID table_id) const; explicit RegionRangeKeys(TiKVKey && start_key, TiKVKey && end_key); + TableID getFlashTableID() const; private: RegionRange ori; std::pair raw; + TableID flash_table_id; + HandleRange flash_handle_range; }; } // namespace DB diff --git a/dbms/src/Storages/Transaction/RegionState.cpp b/dbms/src/Storages/Transaction/RegionState.cpp index 7997be1ce4b..b5e686795a0 100644 --- a/dbms/src/Storages/Transaction/RegionState.cpp +++ b/dbms/src/Storages/Transaction/RegionState.cpp @@ -58,16 +58,35 @@ const RegionState::Base & RegionState::getBase() const { return *this; } const raft_serverpb::MergeState & RegionState::getMergeState() const { return merge_state(); } raft_serverpb::MergeState & RegionState::getMutMergeState() { return *mutable_merge_state(); } +TableID computeFlashTableID(const DecodedTiKVKey & key) +{ + // t table_id _r + if (key.size() >= (1 + 8 + 2) && key[0] == RecordKVFormat::TABLE_PREFIX + && memcmp(key.data() + 9, RecordKVFormat::RECORD_PREFIX_SEP, 2) == 0) + return RecordKVFormat::getTableId(key); + + throw Exception("Can't tell table id for region, should not happen", ErrorCodes::LOGICAL_ERROR); +} + RegionRangeKeys::RegionRangeKeys(TiKVKey && start_key, TiKVKey && end_key) : ori(RegionRangeKeys::makeComparableKeys(std::move(start_key), std::move(end_key))), raw(ori.first.key.empty() ? DecodedTiKVKey() : RecordKVFormat::decodeTiKVKey(ori.first.key), - ori.second.key.empty() ? DecodedTiKVKey() : RecordKVFormat::decodeTiKVKey(ori.second.key)) -{} + ori.second.key.empty() ? DecodedTiKVKey() : RecordKVFormat::decodeTiKVKey(ori.second.key)), + flash_table_id(computeFlashTableID(raw.first)), + flash_handle_range(TiKVRange::getHandleRangeByTable(rawKeys().first, rawKeys().second, flash_table_id)) +{ + if (flash_handle_range.first == flash_handle_range.second) + throw Exception(std::string(__PRETTY_FUNCTION__) + " got empty handle range", ErrorCodes::LOGICAL_ERROR); +} + +TableID RegionRangeKeys::getFlashTableID() const { return flash_table_id; } const std::pair & RegionRangeKeys::rawKeys() const { return raw; } HandleRange RegionRangeKeys::getHandleRangeByTable(const TableID table_id) const { + if (table_id == flash_table_id) + return flash_handle_range; return TiKVRange::getHandleRangeByTable(rawKeys().first, rawKeys().second, table_id); } diff --git a/dbms/src/Storages/Transaction/RegionTable.cpp b/dbms/src/Storages/Transaction/RegionTable.cpp index 28ff1e2019c..54b2cc6cdc2 100644 --- a/dbms/src/Storages/Transaction/RegionTable.cpp +++ b/dbms/src/Storages/Transaction/RegionTable.cpp @@ -21,8 +21,8 @@ RegionTable::Table & RegionTable::getOrCreateTable(const TableID table_id) if (it == tables.end()) { // Load persisted info. - it = tables.emplace(table_id, Table(table_id)).first; - LOG_INFO(log, "[getOrCreateTable] create new table " << table_id); + it = tables.emplace(table_id, table_id).first; + LOG_INFO(log, __FUNCTION__ << ": get new table " << table_id); } return it->second; } @@ -37,20 +37,25 @@ RegionTable::InternalRegion & RegionTable::insertRegion(Table & table, const Reg { auto & table_regions = table.regions; // Insert table mapping. - auto [it, ok] = table_regions.emplace(region_id, InternalRegion(region_id, region_range_keys.getHandleRangeByTable(table.table_id))); + auto [it, ok] = table_regions.insert({region_id, InternalRegion(region_id, region_range_keys.getHandleRangeByTable(table.table_id))}); if (!ok) throw Exception( - "[RegionTable::insertRegion] insert duplicate internal region " + DB::toString(region_id), ErrorCodes::LOGICAL_ERROR); + std::string(__PRETTY_FUNCTION__) + ": insert duplicate internal region " + DB::toString(region_id), ErrorCodes::LOGICAL_ERROR); // Insert region mapping. - RegionInfo & region_info = regions[region_id]; - region_info.emplace(table.table_id); + regions[region_id] = table.table_id; return it->second; } -RegionTable::InternalRegion & RegionTable::getOrInsertRegion(const TableID table_id, const Region & region) +RegionTable::InternalRegion & RegionTable::doGetInternalRegion(DB::TableID table_id, DB::RegionID region_id) { + return tables.find(table_id)->second.regions.find(region_id)->second; +} + +RegionTable::InternalRegion & RegionTable::getOrInsertRegion(const Region & region) +{ + auto table_id = region.getFlashTableID(); auto & table = getOrCreateTable(table_id); auto & table_regions = table.regions; if (auto it = table_regions.find(region.id()); it != table_regions.end()) @@ -62,47 +67,8 @@ RegionTable::InternalRegion & RegionTable::getOrInsertRegion(const TableID table void RegionTable::shrinkRegionRange(const Region & region) { std::lock_guard lock(mutex); - doShrinkRegionRange(region); -} - -void RegionTable::doShrinkRegionRange(const Region & region) -{ - auto region_id = region.id(); - const auto range = region.getRange(); - - auto it = regions.find(region_id); - // if this region does not exist already, then nothing to shrink. - if (it == regions.end()) - return; - - RegionInfo & region_info = it->second; - auto region_table_it = region_info.begin(); - while (region_table_it != region_info.end()) - { - auto table_id = *region_table_it; - - const auto handle_range = range->getHandleRangeByTable(table_id); - auto table_it = tables.find(table_id); - if (table_it == tables.end()) - throw Exception("Table " + DB::toString(table_id) + " not found in table map", ErrorCodes::LOGICAL_ERROR); - - Table & table = table_it->second; - if (handle_range.first < handle_range.second) - { - if (auto region_it = table.regions.find(region_id); region_it != table.regions.end()) - region_it->second.range_in_table = handle_range; - else - throw Exception("InternalRegion " + DB::toString(region_id) + " not found in table " + DB::toString(table_id), - ErrorCodes::LOGICAL_ERROR); - ++region_table_it; - } - else - { - // remove from table mapping - table.regions.erase(region_id); - region_table_it = region_info.erase(region_table_it); - } - } + auto & internal_region = getOrInsertRegion(region); + internal_region.range_in_table = region.getHandleRangeByTable(region.getFlashTableID()); } bool RegionTable::shouldFlush(const InternalRegion & region) const @@ -112,42 +78,32 @@ bool RegionTable::shouldFlush(const InternalRegion & region) const if (!region.cache_bytes) return false; auto period_time = Clock::now() - region.last_flush_time; - return flush_thresholds.traverse([&](const FlushThresholds::FlushThresholdsData & data) -> bool { - for (const auto & [th_bytes, th_duration] : data) - { - if (region.cache_bytes >= th_bytes && period_time >= th_duration) - return true; - } - return false; - }); + for (const auto & [th_bytes, th_duration] : *flush_thresholds.getData()) + { + if (region.cache_bytes >= th_bytes && period_time >= th_duration) + return true; + } + return false; } -void RegionTable::flushRegion(TableID table_id, RegionID region_id, const bool try_persist) const +void RegionTable::flushRegion(const RegionPtr & region, bool try_persist) const { const auto & tmt = context->getTMTContext(); - /// Store region ptr first - RegionPtr region = tmt.getKVStore()->getRegion(region_id); - { - if (!region) - { - LOG_WARNING(log, "[flushRegion] region " << region_id << " is not found"); - return; - } - - LOG_DEBUG(log, "[flushRegion] table " << table_id << ", [region " << region_id << "] original " << region->dataSize() << " bytes"); - } + LOG_DEBUG(log, + __FUNCTION__ << ": table " << region->getFlashTableID() << ", " << region->toString(false) << " original " << region->dataSize() + << " bytes"); /// Write region data into corresponding storage. RegionDataReadInfoList data_list_to_remove; { - writeBlockByRegion(*context, table_id, region, data_list_to_remove, log); + writeBlockByRegion(*context, region, data_list_to_remove, log); } /// Remove data in region. { { - auto remover = region->createCommittedRemover(table_id); + auto remover = region->createCommittedRemover(); for (const auto & [handle, write_type, commit_ts, value] : data_list_to_remove) { std::ignore = write_type; @@ -162,12 +118,14 @@ void RegionTable::flushRegion(TableID table_id, RegionID region_id, const bool t if (cache_size == 0) { if (try_persist) - tmt.getKVStore()->tryPersist(region_id); + tmt.getKVStore()->tryPersist(region->id()); else region->incDirtyFlag(); } - LOG_DEBUG(log, "[flushRegion] table " << table_id << ", [region " << region_id << "] after flush " << cache_size << " bytes"); + LOG_DEBUG(log, + __FUNCTION__ << ": table " << region->getFlashTableID() << ", " << region->toString(false) << " after flush " << cache_size + << " bytes"); } } @@ -194,37 +152,7 @@ void RegionTable::restore() const auto & tmt = context->getTMTContext(); - // first get all table from storage. - for (const auto & storage : tmt.getStorages().getAllStorage()) - getOrCreateTable(storage.first); - - LOG_INFO(log, "Get " << tables.size() << " tables from TMTStorages"); - - // find region whose range may be overlapped with table. - for (const auto & table : tables) - { - const auto table_id = table.first; - - const auto table_range = RegionRangeKeys::makeComparableKeys(RecordKVFormat::genKey(table_id, std::numeric_limits::min()), - RecordKVFormat::genKey(table_id, std::numeric_limits::max())); - - tmt.getKVStore()->handleRegionsByRangeOverlap(table_range, [&](RegionMap region_map, const KVStoreTaskLock &) { - for (const auto & region : region_map) - doUpdateRegion(*region.second, table_id); - }); - } - - // traverse regions and update because there may be table id not mapped in RegionTable. - tmt.getKVStore()->traverseRegions([this](const RegionID, const RegionPtr & region) { applySnapshotRegion(*region); }); - - // if table schema exists but there is no relative region, remove it; - for (auto it = tables.begin(); it != tables.end();) - { - if (it->second.regions.empty()) - it = tables.erase(it); - else - it++; - } + tmt.getKVStore()->traverseRegions([this](const RegionID, const RegionPtr & region) { updateRegion(*region); }); LOG_INFO(log, "Restore " << tables.size() << " tables"); } @@ -240,62 +168,23 @@ void RegionTable::removeTable(TableID table_id) // Remove from region list. for (const auto & region_info : table.regions) - { - regions[region_info.first].erase(table.table_id); - } + regions.erase(region_info.first); // Remove from table map. tables.erase(it); - LOG_INFO(log, "[removeTable] remove table " << table_id << " in RegionTable success"); + LOG_INFO(log, __FUNCTION__ << ": remove table " << table_id << " in RegionTable success"); } -void RegionTable::updateRegion(const Region & region, const TableIDSet & relative_table_ids) +void RegionTable::updateRegion(const Region & region) { std::lock_guard lock(mutex); - - for (const auto table_id : relative_table_ids) - doUpdateRegion(region, table_id); -} - -void RegionTable::doUpdateRegion(const Region & region, TableID table_id) -{ - auto & internal_region = getOrInsertRegion(table_id, region); + auto & internal_region = getOrInsertRegion(region); internal_region.cache_bytes = region.dataSize(); if (internal_region.cache_bytes) dirty_regions.insert(internal_region.region_id); } -void RegionTable::applySnapshotRegion(const Region & region) -{ - // make operation about snapshot can only add mapping relations rather than delete. - auto table_ids = region.getAllWriteCFTables(); - updateRegion(region, table_ids); -} - -void RegionTable::updateRegionForSplit(const Region & split_region, const RegionID source_region) -{ - std::lock_guard lock(mutex); - - auto it = regions.find(source_region); - - if (it == regions.end()) - { - // If source_region doesn't exist, usually means it does not contain any data we interested. Just ignore it. - return; - } - - for (const auto table_id : it->second) - { - const auto handle_range = split_region.getHandleRangeByTable(table_id); - - if (handle_range.first >= handle_range.second) - continue; - - doUpdateRegion(split_region, table_id); - } -} - TableID RegionTable::popOneTableToOptimize() { TableID res = InvalidTableID; @@ -310,89 +199,61 @@ TableID RegionTable::popOneTableToOptimize() void RegionTable::removeRegion(const RegionID region_id) { - std::unordered_set tables; - { - std::lock_guard lock(mutex); - - auto it = regions.find(region_id); - if (it == regions.end()) - { - LOG_WARNING(log, "[removeRegion] region " << region_id << " does not exist."); - return; - } - RegionInfo & region_info = it->second; - tables.swap(region_info); - - regions.erase(region_id); + std::lock_guard lock(mutex); - for (const auto table_id : tables) - { - auto & table = getOrCreateTable(table_id); - table.regions.erase(region_id); - if (table.regions.empty()) - table_to_optimize.emplace(table_id); - } + if (auto it = regions.find(region_id); it == regions.end()) + { + LOG_WARNING(log, __FUNCTION__ << ": region " << region_id << " does not exist."); + return; + } + else + { + TableID table_id = it->second; + regions.erase(it); + auto & table = tables.find(table_id)->second; + table.regions.erase(region_id); + if (table.regions.empty()) + table_to_optimize.insert(table_id); } } -void RegionTable::tryFlushRegion(RegionID region_id) +void RegionTable::tryFlushRegion(RegionID region_id, bool try_persist) { - TableIDSet table_ids; + auto region = context->getTMTContext().getKVStore()->getRegion(region_id); + if (!region) { - std::lock_guard lock(mutex); - if (auto it = regions.find(region_id); it != regions.end()) - { - if (it->second.empty()) - { - LOG_DEBUG(log, "[tryFlushRegion] region " << region_id << " fail, no table for mapping"); - return; - } - // maybe this region contains more than one table, just flush the first one. - table_ids = it->second; - } - else - { - LOG_DEBUG(log, "[tryFlushRegion] region " << region_id << " fail, internal region not exist"); - return; - } + LOG_WARNING(log, __FUNCTION__ << ": region " << region_id << " not found"); + return; } - for (const auto table_id : table_ids) - tryFlushRegion(region_id, table_id, false); + tryFlushRegion(region, try_persist); } -void RegionTable::tryFlushRegion(RegionID region_id, TableID table_id, const bool try_persist) +void RegionTable::tryFlushRegion(const RegionPtr & region, bool try_persist) { + RegionID region_id = region->id(); + const auto func_update_region = [&](std::function && callback) -> bool { std::lock_guard lock(mutex); - if (auto table_it = tables.find(table_id); table_it != tables.end()) + if (regions.find(region_id) != regions.end()) { - auto & internal_region_map = table_it->second.regions; - if (auto region_it = internal_region_map.find(region_id); region_it != internal_region_map.end()) - { - InternalRegion & region = region_it->second; - return callback(region); - } - else - { - LOG_DEBUG(log, "[tryFlushRegion] region " << region_id << " fail, internal region might be removed"); - return false; - } + auto & internal_region = doGetInternalRegion(region->getFlashTableID(), region_id); + return callback(internal_region); } else { - LOG_DEBUG(log, "[tryFlushRegion] region " << region_id << " fail, table not exist"); + LOG_DEBUG(log, __FUNCTION__ << ": internal region " << region_id << " might be removed"); return false; } }; - bool status = func_update_region([&](InternalRegion & region) -> bool { - if (region.pause_flush) + bool status = func_update_region([&](InternalRegion & internal_region) -> bool { + if (internal_region.pause_flush) { - LOG_INFO(log, "[tryFlushRegion] internal region " << region_id << " pause flush, may be being flushed"); + LOG_INFO(log, __FUNCTION__ << ": internal region " << region_id << " pause flush, may be being flushed"); return false; } - region.pause_flush = true; + internal_region.pause_flush = true; return true; }); @@ -403,7 +264,7 @@ void RegionTable::tryFlushRegion(RegionID region_id, TableID table_id, const boo try { - flushRegion(table_id, region_id, try_persist); + flushRegion(region, try_persist); } catch (...) { @@ -412,12 +273,8 @@ void RegionTable::tryFlushRegion(RegionID region_id, TableID table_id, const boo func_update_region([&](InternalRegion & internal_region) -> bool { internal_region.pause_flush = false; - size_t cache_bytes = 0; - if (auto region = context->getTMTContext().getKVStore()->getRegion(internal_region.region_id); region) - cache_bytes = region->dataSize(); - - internal_region.cache_bytes = cache_bytes; - if (cache_bytes) + internal_region.cache_bytes = region->dataSize(); + if (internal_region.cache_bytes) dirty_regions.insert(region_id); else dirty_regions.erase(region_id); @@ -440,43 +297,48 @@ bool RegionTable::tryFlushRegions() struct DataToFlush { - TableID table_id; RegionID region_id; bool got = false; + RegionID useless_region = InvalidRegionID; }; + DataToFlush to_flush; - { // judge choose region to flush - const auto traverse = [this](std::function && callback) { + { + // judge choose region to flush + const auto traverse_dirty_regions = [&](std::function && callback) { std::lock_guard lock(mutex); for (const auto & region_id : dirty_regions) { if (auto it = regions.find(region_id); it != regions.end()) { - for (auto & table_id : it->second) - { - if (callback(table_id, tables.find(table_id)->second.regions.find(region_id)->second)) - return; - } + auto table_id = it->second; + if (callback(doGetInternalRegion(table_id, region_id))) + return; } + else + to_flush.useless_region = region_id; } }; - traverse([&](TableID table_id, InternalRegion & region) { + traverse_dirty_regions([&](InternalRegion & region) { if (shouldFlush(region)) { - to_flush = DataToFlush{table_id, region.region_id, true}; + to_flush = DataToFlush{region.region_id, true}; dirty_regions.erase(region.region_id); return true; } return false; }); + if (to_flush.useless_region != InvalidRegionID) + dirty_regions.erase(to_flush.useless_region); + if (!to_flush.got) return false; } - tryFlushRegion(to_flush.region_id, to_flush.table_id, true); + tryFlushRegion(to_flush.region_id, true); return true; } @@ -493,19 +355,13 @@ std::vector> RegionTable::getRegionsByTable(const { auto & kvstore = context->getTMTContext().getKVStore(); std::vector> regions; - { - std::lock_guard lock(mutex); - - if (auto it = tables.find(table_id); it != tables.end()) + handleInternalRegionsByTable(table_id, [&](const InternalRegions & internal_regions) { + for (const auto & region_info : internal_regions) { - auto & table = it->second; - for (const auto & region_info : table.regions) - { - auto region = kvstore->getRegion(region_info.second.region_id); - regions.emplace_back(region_info.second.region_id, region); - } + auto region = kvstore->getRegion(region_info.first); + regions.emplace_back(region_info.first, std::move(region)); } - } + }); return regions; } @@ -514,24 +370,44 @@ void RegionTable::setFlushThresholds(const FlushThresholds::FlushThresholdsData flush_thresholds.setFlushThresholds(flush_thresholds_); } -TableIDSet RegionTable::getAllMappedTables(const RegionID region_id) const +void RegionTable::extendRegionRange(const RegionID region_id, const RegionRangeKeys & region_range_keys) { std::lock_guard lock(mutex); - if (auto it = regions.find(region_id); it != regions.end()) - return it->second; - - return {}; -} + auto table_id = region_range_keys.getFlashTableID(); + auto new_handle_range = region_range_keys.getHandleRangeByTable(table_id); -void RegionTable::dumpRegionsByTable(const TableID table_id, size_t & count, InternalRegions * regions) const -{ - std::lock_guard lock(mutex); - if (auto it = tables.find(table_id); it != tables.end()) + if (auto it = regions.find(region_id); it != regions.end()) + { + if (table_id != it->second) + throw Exception(std::string(__PRETTY_FUNCTION__) + ": table id " + std::to_string(table_id) + " not match previous one " + + std::to_string(it->second) + " in regions " + std::to_string(region_id), + ErrorCodes::LOGICAL_ERROR); + + InternalRegion & internal_region = doGetInternalRegion(table_id, region_id); + if (internal_region.range_in_table.first <= new_handle_range.first + && internal_region.range_in_table.second >= new_handle_range.second) + { + LOG_INFO(log, __FUNCTION__ << ": table " << table_id << ", internal region " << region_id << " has larger range"); + } + else + { + const auto ori_range = internal_region.range_in_table; + internal_region.range_in_table.first = std::min(new_handle_range.first, internal_region.range_in_table.first); + internal_region.range_in_table.second = std::max(new_handle_range.second, internal_region.range_in_table.second); + + LOG_INFO(log, + __FUNCTION__ << ": table " << table_id << ", internal region " << region_id << " extend range from [" + << ori_range.first.toString() << "," << ori_range.second.toString() << ") to [" + << internal_region.range_in_table.first.toString() << "," << internal_region.range_in_table.second.toString() + << ")"); + } + } + else { - count = it->second.regions.size(); - if (regions) - *regions = it->second.regions; + auto & table = getOrCreateTable(table_id); + insertRegion(table, region_range_keys, region_id); + LOG_INFO(log, __FUNCTION__ << ": table " << table_id << " insert internal region " << region_id); } } diff --git a/dbms/src/Storages/Transaction/RegionTable.h b/dbms/src/Storages/Transaction/RegionTable.h index 70b2bfcce93..656c0b9b1e1 100644 --- a/dbms/src/Storages/Transaction/RegionTable.h +++ b/dbms/src/Storages/Transaction/RegionTable.h @@ -6,6 +6,7 @@ #include #include +#include #include #include @@ -29,7 +30,6 @@ using BlockInputStreamPtr = std::shared_ptr; class Block; // for debug struct MockTiDBTable; -using RegionMap = std::unordered_map; class RegionRangeKeys; class RegionTable : private boost::noncopyable @@ -37,7 +37,6 @@ class RegionTable : private boost::noncopyable public: struct InternalRegion { - InternalRegion(const InternalRegion & p) : region_id(p.region_id), range_in_table(p.range_in_table) {} InternalRegion(const RegionID region_id_, const HandleRange & range_in_table_ = {0, 0}) : region_id(region_id_), range_in_table(range_in_table_) {} @@ -51,69 +50,51 @@ class RegionTable : private boost::noncopyable using InternalRegions = std::unordered_map; - struct Table + struct Table : boost::noncopyable { Table(const TableID table_id_) : table_id(table_id_) {} TableID table_id; InternalRegions regions; }; - enum RegionReadStatus : UInt8 - { - OK, - NOT_FOUND, - VERSION_ERROR, - PENDING_REMOVE, - }; + using TableMap = std::unordered_map; + using RegionInfoMap = std::unordered_map; - static const char * RegionReadStatusString(RegionReadStatus s) + struct TableOptimizeChecker { - switch (s) - { - case OK: - return "OK"; - case NOT_FOUND: - return "NOT_FOUND"; - case VERSION_ERROR: - return "VERSION_ERROR"; - case PENDING_REMOVE: - return "PENDING_REMOVE"; - } - return "Unknown"; + std::mutex mutex; + bool is_checking = false; + double threshold = 1.0; + Timepoint last_check_time = Clock::now(); }; - using RegionInfo = std::unordered_set; - using TableMap = std::unordered_map; - using RegionInfoMap = std::unordered_map; + using DirtyRegions = std::unordered_set; + using TableToOptimize = std::unordered_set; struct FlushThresholds { using FlushThresholdsData = std::vector>; - FlushThresholdsData data; - mutable std::mutex mutex; - - FlushThresholds(const FlushThresholdsData & data_) { data = data_; } - FlushThresholds(FlushThresholdsData && data_) { data = std::move(data_); } + FlushThresholds(FlushThresholdsData && data_) : data(std::make_shared(std::move(data_))) {} void setFlushThresholds(const FlushThresholdsData & flush_thresholds_) { - std::lock_guard lock(mutex); - data = flush_thresholds_; + auto flush_thresholds = std::make_shared(flush_thresholds_); + { + std::lock_guard lock(mutex); + data = std::move(flush_thresholds); + } } - const FlushThresholdsData & getData() const + auto getData() const { std::lock_guard lock(mutex); return data; } - template - T traverse(std::function && f) const - { - std::lock_guard lock(mutex); - return f(data); - } + private: + std::shared_ptr data; + mutable std::mutex mutex; }; RegionTable(Context & context_); @@ -121,15 +102,7 @@ class RegionTable : private boost::noncopyable void setFlushThresholds(const FlushThresholds::FlushThresholdsData & flush_thresholds_); - /// Remove a table and associated regions. - void removeTable(TableID table_id); - - /// After the region is updated (insert or delete KVs). - void updateRegion(const Region & region, const TableIDSet & relative_table_ids); - /// A new region arrived by apply snapshot command, this function store the region into selected partitions. - void applySnapshotRegion(const Region & region); - - void updateRegionForSplit(const Region & split_region, const RegionID source_region); + void updateRegion(const Region & region); /// This functional only shrink the table range of this region_id void shrinkRegionRange(const Region & region); @@ -138,14 +111,9 @@ class RegionTable : private boost::noncopyable TableID popOneTableToOptimize(); - /// Try pick some regions and flush. - /// Note that flush is organized by partition. i.e. if a regions is selected to be flushed, all regions belong to its partition will also flushed. - /// This function will be called constantly by background threads. - /// Returns whether this function has done any meaningful job. bool tryFlushRegions(); - - void tryFlushRegion(RegionID region_id); - void tryFlushRegion(RegionID region_id, TableID table_id, const bool try_persist); + void tryFlushRegion(RegionID region_id, bool try_persist = false); + void tryFlushRegion(const RegionPtr & region, bool try_persist); void handleInternalRegionsByTable(const TableID table_id, std::function && callback) const; std::vector> getRegionsByTable(const TableID table_id) const; @@ -154,13 +122,12 @@ class RegionTable : private boost::noncopyable /// Will trigger schema sync on read error for only once, /// assuming that newer schema can always apply to older data by setting force_decode to true in readRegionBlock. /// Note that table schema must be keep unchanged throughout the process of read then write, we take good care of the lock. - static void writeBlockByRegion( - Context & context, TableID table_id, RegionPtr region, RegionDataReadInfoList & data_list_to_remove, Logger * log); + static void writeBlockByRegion(Context & context, RegionPtr region, RegionDataReadInfoList & data_list_to_remove, Logger * log); /// Read the data of the given region into block, take good care of learner read and locks. /// Assuming that the schema has been properly synced by outer, i.e. being new enough to decode data before start_ts, /// we directly ask readRegionBlock to perform a read with the given start_ts and force_decode being true. - static std::tuple readBlockByRegion(const TiDB::TableInfo & table_info, + static std::tuple readBlockByRegion(const TiDB::TableInfo & table_info, const ColumnsDescription & columns, const Names & column_names_to_read, const RegionPtr & region, @@ -170,40 +137,32 @@ class RegionTable : private boost::noncopyable Timestamp start_ts, DB::HandleRange & handle_range); - TableIDSet getAllMappedTables(const RegionID region_id) const; - void dumpRegionsByTable(const TableID table_id, size_t & count, InternalRegions * regions) const; - void checkTableOptimize(); void checkTableOptimize(TableID, const double); void setTableCheckerThreshold(double); + /// traverse all table and try to extend range for possible InternalRegion or add one. + void extendRegionRange(const RegionID region_id, const RegionRangeKeys & region_range_keys); + private: - Table & getOrCreateTable(const TableID table_id); + friend class MockTiDB; + Table & getOrCreateTable(const TableID table_id); + void removeTable(TableID table_id); InternalRegion & insertRegion(Table & table, const Region & region); - InternalRegion & getOrInsertRegion(TableID table_id, const Region & region); + InternalRegion & getOrInsertRegion(const Region & region); InternalRegion & insertRegion(Table & table, const RegionRangeKeys & region_range_keys, const RegionID region_id); + InternalRegion & doGetInternalRegion(TableID table_id, RegionID region_id); bool shouldFlush(const InternalRegion & region) const; - void flushRegion(TableID table_id, RegionID region_id, const bool try_persist = true) const; - - void doShrinkRegionRange(const Region & region); - void doUpdateRegion(const Region & region, TableID table_id); - - struct TableOptimizeChecker - { - std::mutex mutex; - bool is_checking = false; - double threshold = 1.0; - Timepoint last_check_time = Clock::now(); - }; + void flushRegion(const RegionPtr & region, bool try_persist) const; private: TableMap tables; RegionInfoMap regions; - std::unordered_set dirty_regions; - std::unordered_set table_to_optimize; + DirtyRegions dirty_regions; + TableToOptimize table_to_optimize; FlushThresholds flush_thresholds; diff --git a/dbms/src/Storages/Transaction/RegionTableCheckOptimize.cpp b/dbms/src/Storages/Transaction/RegionTableCheckOptimize.cpp index ffce827eca6..2ed3a273754 100644 --- a/dbms/src/Storages/Transaction/RegionTableCheckOptimize.cpp +++ b/dbms/src/Storages/Transaction/RegionTableCheckOptimize.cpp @@ -126,7 +126,7 @@ void RegionTable::checkTableOptimize() std::vector table_to_check; { std::lock_guard lock(mutex); - for (const auto table : tables) + for (const auto & table : tables) { if (!table.second.regions.empty()) table_to_check.emplace_back(table.first); @@ -159,7 +159,7 @@ void RegionTable::checkTableOptimize(DB::TableID table_id, const double threshol { LOG_INFO(log, "table " << table_id << " need to be optimized"); std::lock_guard lock(mutex); - table_to_optimize.emplace(table_id); + table_to_optimize.insert(table_id); } } diff --git a/dbms/src/Storages/Transaction/Types.h b/dbms/src/Storages/Transaction/Types.h index 517bcca1cbb..bc04e087881 100644 --- a/dbms/src/Storages/Transaction/Types.h +++ b/dbms/src/Storages/Transaction/Types.h @@ -14,11 +14,8 @@ using TableIDSet = std::unordered_set; enum : TableID { InvalidTableID = 0, - MaxSystemTableID = 29 }; -inline bool isTiDBSystemTable(TableID table_id) { return table_id <= MaxSystemTableID; } - using DatabaseID = Int64; using ColumnID = Int64; diff --git a/dbms/src/Storages/Transaction/applySnapshot.cpp b/dbms/src/Storages/Transaction/applySnapshot.cpp index 5de54561917..afd3eb435c7 100644 --- a/dbms/src/Storages/Transaction/applySnapshot.cpp +++ b/dbms/src/Storages/Transaction/applySnapshot.cpp @@ -19,7 +19,7 @@ extern const int LOGICAL_ERROR; static const std::string RegionSnapshotName = "RegionSnapshot"; -bool applySnapshot(const KVStorePtr & kvstore, RegionPtr new_region, Context * context) +bool applySnapshot(const KVStorePtr & kvstore, RegionPtr new_region, Context * context, bool try_flush_region) { Logger * log = &Logger::get(RegionSnapshotName); @@ -42,7 +42,7 @@ bool applySnapshot(const KVStorePtr & kvstore, RegionPtr new_region, Context * c auto & tmt = context->getTMTContext(); Timestamp safe_point = PDClientHelper::getGCSafePointWithRetry(tmt.getPDClient()); - std::unordered_map handle_maps; + HandleMap handle_map; { std::stringstream ss; @@ -67,34 +67,32 @@ bool applySnapshot(const KVStorePtr & kvstore, RegionPtr new_region, Context * c // Get all handle with largest version in those regions. for (const auto & region_info : regions_to_check) - new_region->compareAndUpdateHandleMaps(*region_info.second.first, handle_maps); + new_region->compareAndUpdateHandleMaps(*region_info.second.first, handle_map); } // Traverse all table in ch and update handle_maps. - for (auto [table_id, merge_tree] : tmt.getStorages().getAllStorage()) + auto table_id = new_region->getFlashTableID(); + if (auto storage = tmt.getStorages().get(table_id); storage) { const auto handle_range = new_region->getHandleRangeByTable(table_id); - if (handle_range.first >= handle_range.second) - continue; { - auto table_lock = merge_tree->lockStructure(false, __PRETTY_FUNCTION__); + auto table_lock = storage->lockStructure(false, __PRETTY_FUNCTION__); - const bool pk_is_uint64 = getTMTPKType(*merge_tree->getData().primary_key_data_types[0]) == TMTPKType::UINT64; + const bool pk_is_uint64 = getTMTPKType(*storage->getData().primary_key_data_types[0]) == TMTPKType::UINT64; if (pk_is_uint64) { const auto [n, new_range] = CHTableHandle::splitForUInt64TableHandle(handle_range); - getHandleMapByRange(*context, *merge_tree, new_range[0], handle_maps[table_id]); + getHandleMapByRange(*context, *storage, new_range[0], handle_map); if (n > 1) - getHandleMapByRange(*context, *merge_tree, new_range[1], handle_maps[table_id]); + getHandleMapByRange(*context, *storage, new_range[1], handle_map); } else - getHandleMapByRange(*context, *merge_tree, handle_range, handle_maps[table_id]); + getHandleMapByRange(*context, *storage, handle_range, handle_map); } } - for (auto & [table_id, handle_map] : handle_maps) - new_region->compareAndCompleteSnapshot(handle_map, table_id, safe_point); + new_region->compareAndCompleteSnapshot(handle_map, safe_point); } if (old_region) @@ -111,7 +109,7 @@ bool applySnapshot(const KVStorePtr & kvstore, RegionPtr new_region, Context * c } } - return kvstore->onSnapshot(new_region, context, regions_to_check); + return kvstore->onSnapshot(new_region, context, regions_to_check, try_flush_region); } void applySnapshot(const KVStorePtr & kvstore, RequestReader read, Context * context) @@ -160,7 +158,7 @@ void applySnapshot(const KVStorePtr & kvstore, RequestReader read, Context * con if (new_region->isPeerRemoved()) throw Exception("[applySnapshot] region is removed, should not happen", ErrorCodes::LOGICAL_ERROR); - bool status = applySnapshot(kvstore, new_region, context); + bool status = applySnapshot(kvstore, new_region, context, true); LOG_INFO(log, new_region->toString(false) << " apply snapshot " << (status ? "success" : "fail")); } diff --git a/dbms/src/Storages/Transaction/applySnapshot.h b/dbms/src/Storages/Transaction/applySnapshot.h index f2f0a416e83..1b0b1667a96 100644 --- a/dbms/src/Storages/Transaction/applySnapshot.h +++ b/dbms/src/Storages/Transaction/applySnapshot.h @@ -18,7 +18,7 @@ using KVStorePtr = std::shared_ptr; // Simplify test. using RequestReader = std::function; -void applySnapshot(const KVStorePtr & kvstore, RequestReader read, Context * context = nullptr); -bool applySnapshot(const KVStorePtr & kvstore, RegionPtr new_region, Context * context); +void applySnapshot(const KVStorePtr & kvstore, RequestReader read, Context * context); +bool applySnapshot(const KVStorePtr & kvstore, RegionPtr new_region, Context * context, bool try_flush_region); } // namespace DB From 31f0d666c18041e7b81f38d8f457c7f700d5bfb4 Mon Sep 17 00:00:00 2001 From: Tong Zhigao Date: Mon, 11 Nov 2019 20:50:36 +0800 Subject: [PATCH 02/12] small fix --- dbms/src/Debug/ClusterManage.cpp | 2 +- dbms/src/Flash/Coprocessor/InterpreterDAG.cpp | 2 +- .../Storages/Transaction/PartitionStreams.cpp | 4 ++-- dbms/src/Storages/Transaction/Region.cpp | 2 +- .../Storages/Transaction/RegionDataMover.cpp | 20 ++++++++++++++----- dbms/src/Storages/Transaction/RegionMeta.cpp | 1 - dbms/src/Storages/Transaction/RegionTable.cpp | 9 ++++++--- 7 files changed, 26 insertions(+), 14 deletions(-) diff --git a/dbms/src/Debug/ClusterManage.cpp b/dbms/src/Debug/ClusterManage.cpp index 3b999f934b8..1583ef930b6 100644 --- a/dbms/src/Debug/ClusterManage.cpp +++ b/dbms/src/Debug/ClusterManage.cpp @@ -137,7 +137,7 @@ void ClusterManage::findRegionByRange(Context & context, const ASTs & args, Prin void ClusterManage::checkTableOptimize(DB::Context & context, const DB::ASTs & args, DB::Printer) { - if (args.size() < 3) + if (args.size() < 2) throw Exception("Args not matched, should be: table-id, threshold", ErrorCodes::BAD_ARGUMENTS); auto & tmt = context.getTMTContext(); diff --git a/dbms/src/Flash/Coprocessor/InterpreterDAG.cpp b/dbms/src/Flash/Coprocessor/InterpreterDAG.cpp index dd8b3424cf4..207b762bcdf 100644 --- a/dbms/src/Flash/Coprocessor/InterpreterDAG.cpp +++ b/dbms/src/Flash/Coprocessor/InterpreterDAG.cpp @@ -266,7 +266,7 @@ void InterpreterDAG::executeTS(const tipb::TableScan & ts, Pipeline & pipeline) { std::vector region_ids; region_ids.push_back(info.region_id); - throw RegionException(std::move(region_ids), RegionException::NOT_FOUND); + throw RegionException(std::move(region_ids), RegionException::RegionReadStatus::NOT_FOUND); } if (!checkKeyRanges(dag.getKeyRanges(), table_id, storage->pkIsUInt64(), current_region->getRange())) throw Exception("Cop request only support full range scan for given region", ErrorCodes::COP_BAD_DAG_REQUEST); diff --git a/dbms/src/Storages/Transaction/PartitionStreams.cpp b/dbms/src/Storages/Transaction/PartitionStreams.cpp index 0660540edef..d5fd206f95d 100644 --- a/dbms/src/Storages/Transaction/PartitionStreams.cpp +++ b/dbms/src/Storages/Transaction/PartitionStreams.cpp @@ -112,8 +112,8 @@ void RegionTable::writeBlockByRegion(Context & context, RegionPtr region, Region } LOG_TRACE(log, - __PRETTY_FUNCTION__ << ": table " << table_id << ", region " << region->id() << ", cost [region read " << region_read_cost - << ", region decode " << region_decode_cost << ", write part " << write_part_cost << "] ms"); + __FUNCTION__ << ": table " << table_id << ", region " << region->id() << ", cost [region read " << region_read_cost + << ", region decode " << region_decode_cost << ", write part " << write_part_cost << "] ms"); } std::tuple RegionTable::readBlockByRegion(const TiDB::TableInfo & table_info, diff --git a/dbms/src/Storages/Transaction/Region.cpp b/dbms/src/Storages/Transaction/Region.cpp index a700bf7b058..6e32deb8689 100644 --- a/dbms/src/Storages/Transaction/Region.cpp +++ b/dbms/src/Storages/Transaction/Region.cpp @@ -52,7 +52,7 @@ void Region::doCheckTable(const DB::DecodedTiKVKey & raw_key) const if (table_id != getFlashTableID()) { LOG_ERROR(log, __FUNCTION__ << ": table id not match, except " << getFlashTableID() << ", got " << table_id); - throw Exception(std::string(__PRETTY_FUNCTION__) + ": table id not match"); + throw Exception(std::string(__PRETTY_FUNCTION__) + ": table id not match", ErrorCodes::LOGICAL_ERROR); } } diff --git a/dbms/src/Storages/Transaction/RegionDataMover.cpp b/dbms/src/Storages/Transaction/RegionDataMover.cpp index 0551cca6fc1..0b05ba2f528 100644 --- a/dbms/src/Storages/Transaction/RegionDataMover.cpp +++ b/dbms/src/Storages/Transaction/RegionDataMover.cpp @@ -30,7 +30,7 @@ BlockInputStreamPtr createBlockInputStreamFromRange( std::string query = ss.str(); - LOG_DEBUG(&Logger::get(RegionDataMoverName), "[createBlockInputStreamFromRange] sql: " << query); + LOG_DEBUG(&Logger::get(RegionDataMoverName), __FUNCTION__ << ": sql " << query); return executeQuery(query, context, true, QueryProcessingStage::Complete).in; } @@ -41,7 +41,7 @@ void getHandleMapByRange( { SortDescription pk_columns = storage.getData().getPrimarySortDescription(); if (pk_columns.size() != 1) - throw Exception("RegionDataMover: primary key should be one column", ErrorCodes::LOGICAL_ERROR); + throw Exception(std::string(__PRETTY_FUNCTION__) + ": primary key should be one column", ErrorCodes::LOGICAL_ERROR); std::string pk_name = pk_columns[0].column_name; auto start_time = Clock::now(); @@ -86,8 +86,9 @@ void getHandleMapByRange( auto end_time = Clock::now(); LOG_DEBUG(&Logger::get(RegionDataMoverName), - "[getHandleMapByRange] execute sql and handle data, cost " - << std::chrono::duration_cast(end_time - start_time).count() << " ms, read " << tol_rows << " rows"); + __FUNCTION__ << ": execute sql and handle data, cost " + << std::chrono::duration_cast(end_time - start_time).count() << " ms, read " << tol_rows + << " rows"); } template void getHandleMapByRange(Context &, StorageMergeTree &, const HandleRange &, HandleMap &); @@ -95,16 +96,25 @@ template void getHandleMapByRange(Context &, StorageMergeTree &, const H void tryOptimizeStorageFinal(Context & context, TableID table_id) { + auto log = &Logger::get(RegionDataMoverName); auto & tmt = context.getTMTContext(); auto storage = tmt.getStorages().get(table_id); if (!storage) + { + LOG_INFO(log, __FUNCTION__ << ": storage " << table_id << " is none"); return; + } auto table_lock = storage->lockStructure(true, __PRETTY_FUNCTION__); std::stringstream ss; ss << "OPTIMIZE TABLE `" << storage->getDatabaseName() << "`.`" << storage->getTableName() << "` PARTITION ID '0' FINAL"; - executeQuery(ss.str(), context, true, QueryProcessingStage::Complete); + + std::string query = ss.str(); + + LOG_WARNING(log, __FUNCTION__ << ": sql " << query); + + executeQuery(query, context, true, QueryProcessingStage::Complete); } } // namespace DB diff --git a/dbms/src/Storages/Transaction/RegionMeta.cpp b/dbms/src/Storages/Transaction/RegionMeta.cpp index 1d885ab03b7..e46be8e01f3 100644 --- a/dbms/src/Storages/Transaction/RegionMeta.cpp +++ b/dbms/src/Storages/Transaction/RegionMeta.cpp @@ -1,6 +1,5 @@ #include #include -#include #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wunused-parameter" diff --git a/dbms/src/Storages/Transaction/RegionTable.cpp b/dbms/src/Storages/Transaction/RegionTable.cpp index 54b2cc6cdc2..4f7d076218e 100644 --- a/dbms/src/Storages/Transaction/RegionTable.cpp +++ b/dbms/src/Storages/Transaction/RegionTable.cpp @@ -37,7 +37,7 @@ RegionTable::InternalRegion & RegionTable::insertRegion(Table & table, const Reg { auto & table_regions = table.regions; // Insert table mapping. - auto [it, ok] = table_regions.insert({region_id, InternalRegion(region_id, region_range_keys.getHandleRangeByTable(table.table_id))}); + auto [it, ok] = table_regions.emplace(region_id, InternalRegion(region_id, region_range_keys.getHandleRangeByTable(table.table_id))); if (!ok) throw Exception( std::string(__PRETTY_FUNCTION__) + ": insert duplicate internal region " + DB::toString(region_id), ErrorCodes::LOGICAL_ERROR); @@ -235,9 +235,9 @@ void RegionTable::tryFlushRegion(const RegionPtr & region, bool try_persist) const auto func_update_region = [&](std::function && callback) -> bool { std::lock_guard lock(mutex); - if (regions.find(region_id) != regions.end()) + if (auto it = regions.find(region_id); it != regions.end()) { - auto & internal_region = doGetInternalRegion(region->getFlashTableID(), region_id); + auto & internal_region = doGetInternalRegion(it->second, region_id); return callback(internal_region); } else @@ -332,7 +332,10 @@ bool RegionTable::tryFlushRegions() }); if (to_flush.useless_region != InvalidRegionID) + { + std::lock_guard lock(mutex); dirty_regions.erase(to_flush.useless_region); + } if (!to_flush.got) return false; From 0e8ac10b30a92d4d583577d6627c1f1ea1d0bb9c Mon Sep 17 00:00:00 2001 From: Tong Zhigao Date: Tue, 12 Nov 2019 15:38:16 +0800 Subject: [PATCH 03/12] optimize for cmd CompactLog Try to flush region data into ch while handling cmd CompactLog --- dbms/src/Raft/RaftService.cpp | 29 +++++++------------ dbms/src/Raft/RaftService.h | 12 ++++++-- dbms/src/Storages/Transaction/ExtraCFData.h | 29 +------------------ dbms/src/Storages/Transaction/KVStore.cpp | 25 +++++++++++----- .../Storages/Transaction/RaftCommandResult.h | 3 +- dbms/src/Storages/Transaction/Region.cpp | 12 ++++++-- dbms/src/Storages/Transaction/Region.h | 1 + .../Storages/Transaction/RegionCFDataBase.cpp | 3 +- .../Storages/Transaction/RegionDataMover.cpp | 2 +- dbms/src/Storages/Transaction/RegionTable.cpp | 26 ++++++++++------- dbms/src/Storages/Transaction/RegionTable.h | 6 ++-- 11 files changed, 72 insertions(+), 76 deletions(-) diff --git a/dbms/src/Raft/RaftService.cpp b/dbms/src/Raft/RaftService.cpp index 1affde0d36e..ac82e222267 100644 --- a/dbms/src/Raft/RaftService.cpp +++ b/dbms/src/Raft/RaftService.cpp @@ -39,22 +39,18 @@ RaftService::RaftService(DB::Context & db_context_) { LOG_INFO(log, "try to final optimize table " << table_id); tryOptimizeStorageFinal(db_context, table_id); + LOG_INFO(log, "finish final optimize table " << table_id); } return region_table.tryFlushRegions(); }); - region_flush_handle = background_pool.addTask([this] { - RegionID region_id; + data_reclaim_handle = background_pool.addTask([this] { + std::list tmp; { std::lock_guard lock(region_mutex); - if (regions_to_flush.empty()) - return false; - region_id = regions_to_flush.front(); - regions_to_flush.pop(); + tmp = std::move(data_to_reclaim); } - RegionTable & region_table = db_context.getTMTContext().getRegionTable(); - region_table.tryFlushRegion(region_id); - return true; + return false; }); region_decode_handle = background_pool.addTask([this] { @@ -83,13 +79,10 @@ RaftService::RaftService(DB::Context & db_context_) } } -void RaftService::addRegionToFlush(const Region & region) +void RaftService::dataMemReclaim(DB::RegionDataReadInfoList && data) { - { - std::lock_guard lock(region_mutex); - regions_to_flush.push(region.id()); - } - region_flush_handle->wake(); + std::lock_guard lock(reclaim_mutex); + data_to_reclaim.emplace_back(std::move(data)); } void RaftService::addRegionToDecode(const RegionPtr & region) @@ -114,10 +107,10 @@ RaftService::~RaftService() table_flush_handle = nullptr; } - if (region_flush_handle) + if (data_reclaim_handle) { - background_pool.removeTask(region_flush_handle); - region_flush_handle = nullptr; + background_pool.removeTask(data_reclaim_handle); + data_reclaim_handle = nullptr; } if (region_decode_handle) diff --git a/dbms/src/Raft/RaftService.h b/dbms/src/Raft/RaftService.h index 2f8041348d8..0e0f5c2001c 100644 --- a/dbms/src/Raft/RaftService.h +++ b/dbms/src/Raft/RaftService.h @@ -5,6 +5,7 @@ #include #include +#include #include #include #include @@ -27,7 +28,7 @@ class RaftService final : public enginepb::Engine::Service, public std::enable_s ~RaftService() final; - void addRegionToFlush(const Region & region); + void dataMemReclaim(RegionDataReadInfoList &&); void addRegionToDecode(const RegionPtr & region); private: @@ -45,12 +46,17 @@ class RaftService final : public enginepb::Engine::Service, public std::enable_s Logger * log; std::mutex region_mutex; - std::queue regions_to_flush; RegionMap regions_to_decode; + std::mutex reclaim_mutex; + std::list data_to_reclaim; + BackgroundProcessingPool::TaskHandle single_thread_task_handle; BackgroundProcessingPool::TaskHandle table_flush_handle; - BackgroundProcessingPool::TaskHandle region_flush_handle; + + // kvstore will try to flush data into ch when handling raft cmd CompactLog in order to reduce the size of region. + // use this task to reclaim data in another thread. + BackgroundProcessingPool::TaskHandle data_reclaim_handle; BackgroundProcessingPool::TaskHandle region_decode_handle; }; diff --git a/dbms/src/Storages/Transaction/ExtraCFData.h b/dbms/src/Storages/Transaction/ExtraCFData.h index 124bc9b9226..ef2d14df932 100644 --- a/dbms/src/Storages/Transaction/ExtraCFData.h +++ b/dbms/src/Storages/Transaction/ExtraCFData.h @@ -19,19 +19,12 @@ struct ExtraCFData template <> struct ExtraCFData { - mutable std::mutex default_cf_decode_mutex; - ExtraCFData() = default; - void add(const std::shared_ptr & e) - { - std::lock_guard lock(default_cf_decode_mutex); - queue.push_back(e); - } + void add(const std::shared_ptr & e) { queue.push_back(e); } std::optional popAll() { - std::lock_guard lock(default_cf_decode_mutex); if (queue.empty()) return {}; @@ -42,26 +35,6 @@ struct ExtraCFData ExtraCFData(const ExtraCFData & src) = delete; - ExtraCFData(ExtraCFData && src) { mergeFrom(src); } - - ExtraCFData & operator=(ExtraCFData && src) - { - mergeFrom(src); - return *this; - } - -private: - void mergeFrom(ExtraCFData & src) - { - auto res = src.popAll(); - if (res) - { - std::lock_guard lock(default_cf_decode_mutex); - for (auto && e : *res) - queue.emplace_back(std::move(e)); - } - } - private: ExtraCFDataQueue queue; }; diff --git a/dbms/src/Storages/Transaction/KVStore.cpp b/dbms/src/Storages/Transaction/KVStore.cpp index 644ed017b4c..3e23322f408 100644 --- a/dbms/src/Storages/Transaction/KVStore.cpp +++ b/dbms/src/Storages/Transaction/KVStore.cpp @@ -152,10 +152,7 @@ bool KVStore::onSnapshot(RegionPtr new_region, Context * context, const RegionsA region_range_index.add(new_region); if (context) - { - context->getRaftService().addRegionToDecode(new_region); context->getTMTContext().getRegionTable().shrinkRegionRange(*new_region); - } } return true; @@ -230,6 +227,21 @@ void KVStore::onServiceCommand(enginepb::CommandRequestBatch && cmds, RaftContex report_sync_log(); }; + const auto handle_compact_log = [&]() { + if (curr_region.writeCFCount() && curr_region.dataSize()) + { + try + { + auto tmp = region_table->tryFlushRegion(curr_region_ptr, false); + raft_service->dataMemReclaim(std::move(tmp)); + } + catch (...) + { + } + } + persist_and_sync(); + }; + const auto handle_batch_split = [&](Regions & split_regions) { { auto manage_lock = genRegionManageLock(); @@ -260,12 +272,8 @@ void KVStore::onServiceCommand(enginepb::CommandRequestBatch && cmds, RaftContex // update region_table first is safe, because the core rule is established: the range in RegionTable // is always >= range in KVStore. for (const auto & new_region : split_regions) - { region_table->updateRegion(*new_region); - raft_service->addRegionToFlush(*new_region); - } region_table->shrinkRegionRange(curr_region); - raft_service->addRegionToFlush(curr_region); } { @@ -317,6 +325,9 @@ void KVStore::onServiceCommand(enginepb::CommandRequestBatch && cmds, RaftContex case RaftCommandResult::Type::ChangePeer: handle_change_peer(); break; + case RaftCommandResult::Type::CompactLog: + handle_compact_log(); + break; default: throw Exception("Unsupported RaftCommandResult", ErrorCodes::LOGICAL_ERROR); } diff --git a/dbms/src/Storages/Transaction/RaftCommandResult.h b/dbms/src/Storages/Transaction/RaftCommandResult.h index 5d981ee1257..0ac0b0d88fd 100644 --- a/dbms/src/Storages/Transaction/RaftCommandResult.h +++ b/dbms/src/Storages/Transaction/RaftCommandResult.h @@ -21,7 +21,8 @@ struct RaftCommandResult : private boost::noncopyable IndexError, BatchSplit, UpdateTable, - ChangePeer + ChangePeer, + CompactLog, }; bool sync_log; diff --git a/dbms/src/Storages/Transaction/Region.cpp b/dbms/src/Storages/Transaction/Region.cpp index 6e32deb8689..d7989b83d28 100644 --- a/dbms/src/Storages/Transaction/Region.cpp +++ b/dbms/src/Storages/Transaction/Region.cpp @@ -242,6 +242,7 @@ void RegionRaftCommandDelegate::onCommand(enginepb::CommandRequest && cmd, const } case raft_cmdpb::AdminCmdType::CompactLog: execCompactLog(request, response, index, term); + result.type = RaftCommandResult::Type::CompactLog; break; case raft_cmdpb::AdminCmdType::ComputeHash: case raft_cmdpb::AdminCmdType::VerifyHash: @@ -260,6 +261,7 @@ void RegionRaftCommandDelegate::onCommand(enginepb::CommandRequest && cmd, const else { std::unique_lock lock(mutex); + std::lock_guard predecode_lock(predecode_mutex); for (auto && req : *cmd.mutable_requests()) { @@ -616,8 +618,14 @@ std::tuple Region::dumpVersion void Region::tryPreDecodeTiKVValue() { - DB::tryPreDecodeTiKVValue(data.defaultCF().getExtra().popAll()); - DB::tryPreDecodeTiKVValue(data.writeCF().getExtra().popAll()); + std::optional default_val, write_val; + { + std::lock_guard predecode_lock(predecode_mutex); + default_val = data.defaultCF().getExtra().popAll(); + write_val = data.writeCF().getExtra().popAll(); + } + DB::tryPreDecodeTiKVValue(std::move(default_val)); + DB::tryPreDecodeTiKVValue(std::move(write_val)); } Region::Region(RegionMeta && meta_) : Region(std::move(meta_), [](pingcap::kv::RegionVerID) { return nullptr; }) {} diff --git a/dbms/src/Storages/Transaction/Region.h b/dbms/src/Storages/Transaction/Region.h index 9ab63cf7058..53dec56412a 100644 --- a/dbms/src/Storages/Transaction/Region.h +++ b/dbms/src/Storages/Transaction/Region.h @@ -178,6 +178,7 @@ class Region : public std::enable_shared_from_this private: RegionData data; mutable std::shared_mutex mutex; + mutable std::mutex predecode_mutex; RegionMeta meta; diff --git a/dbms/src/Storages/Transaction/RegionCFDataBase.cpp b/dbms/src/Storages/Transaction/RegionCFDataBase.cpp index 01aeac076ab..6be400ef469 100644 --- a/dbms/src/Storages/Transaction/RegionCFDataBase.cpp +++ b/dbms/src/Storages/Transaction/RegionCFDataBase.cpp @@ -154,14 +154,13 @@ size_t RegionCFDataBase::getSize() const } template -RegionCFDataBase::RegionCFDataBase(RegionCFDataBase && region) : data(std::move(region.data)), extra(std::move(region.extra)) +RegionCFDataBase::RegionCFDataBase(RegionCFDataBase && region) : data(std::move(region.data)) {} template RegionCFDataBase & RegionCFDataBase::operator=(RegionCFDataBase && region) { data = std::move(region.data); - extra = std::move(region.extra); return *this; } diff --git a/dbms/src/Storages/Transaction/RegionDataMover.cpp b/dbms/src/Storages/Transaction/RegionDataMover.cpp index 0b05ba2f528..06032e67436 100644 --- a/dbms/src/Storages/Transaction/RegionDataMover.cpp +++ b/dbms/src/Storages/Transaction/RegionDataMover.cpp @@ -112,7 +112,7 @@ void tryOptimizeStorageFinal(Context & context, TableID table_id) std::string query = ss.str(); - LOG_WARNING(log, __FUNCTION__ << ": sql " << query); + LOG_WARNING(log, __FUNCTION__ << ": execute sql " << query); executeQuery(query, context, true, QueryProcessingStage::Complete); } diff --git a/dbms/src/Storages/Transaction/RegionTable.cpp b/dbms/src/Storages/Transaction/RegionTable.cpp index 4f7d076218e..77b39009ec7 100644 --- a/dbms/src/Storages/Transaction/RegionTable.cpp +++ b/dbms/src/Storages/Transaction/RegionTable.cpp @@ -86,11 +86,11 @@ bool RegionTable::shouldFlush(const InternalRegion & region) const return false; } -void RegionTable::flushRegion(const RegionPtr & region, bool try_persist) const +RegionDataReadInfoList RegionTable::flushRegion(const RegionPtr & region, bool try_persist) const { const auto & tmt = context->getTMTContext(); - LOG_DEBUG(log, + LOG_INFO(log, __FUNCTION__ << ": table " << region->getFlashTableID() << ", " << region->toString(false) << " original " << region->dataSize() << " bytes"); @@ -123,10 +123,12 @@ void RegionTable::flushRegion(const RegionPtr & region, bool try_persist) const region->incDirtyFlag(); } - LOG_DEBUG(log, + LOG_INFO(log, __FUNCTION__ << ": table " << region->getFlashTableID() << ", " << region->toString(false) << " after flush " << cache_size << " bytes"); } + + return data_list_to_remove; } static const Int64 FTH_BYTES_1 = 1; // 1 B @@ -217,19 +219,19 @@ void RegionTable::removeRegion(const RegionID region_id) } } -void RegionTable::tryFlushRegion(RegionID region_id, bool try_persist) +RegionDataReadInfoList RegionTable::tryFlushRegion(RegionID region_id, bool try_persist) { auto region = context->getTMTContext().getKVStore()->getRegion(region_id); if (!region) { LOG_WARNING(log, __FUNCTION__ << ": region " << region_id << " not found"); - return; + return {}; } - tryFlushRegion(region, try_persist); + return tryFlushRegion(region, try_persist); } -void RegionTable::tryFlushRegion(const RegionPtr & region, bool try_persist) +RegionDataReadInfoList RegionTable::tryFlushRegion(const RegionPtr & region, bool try_persist) { RegionID region_id = region->id(); @@ -242,7 +244,7 @@ void RegionTable::tryFlushRegion(const RegionPtr & region, bool try_persist) } else { - LOG_DEBUG(log, __FUNCTION__ << ": internal region " << region_id << " might be removed"); + LOG_WARNING(log, __FUNCTION__ << ": internal region " << region_id << " might be removed"); return false; } }; @@ -258,13 +260,13 @@ void RegionTable::tryFlushRegion(const RegionPtr & region, bool try_persist) }); if (!status) - return; + return {}; std::exception_ptr first_exception; - + RegionDataReadInfoList data_list_to_remove; try { - flushRegion(region, try_persist); + data_list_to_remove = flushRegion(region, try_persist); } catch (...) { @@ -285,6 +287,8 @@ void RegionTable::tryFlushRegion(const RegionPtr & region, bool try_persist) if (first_exception) std::rethrow_exception(first_exception); + + return data_list_to_remove; } bool RegionTable::tryFlushRegions() diff --git a/dbms/src/Storages/Transaction/RegionTable.h b/dbms/src/Storages/Transaction/RegionTable.h index 656c0b9b1e1..5f8017f4b0b 100644 --- a/dbms/src/Storages/Transaction/RegionTable.h +++ b/dbms/src/Storages/Transaction/RegionTable.h @@ -112,8 +112,8 @@ class RegionTable : private boost::noncopyable TableID popOneTableToOptimize(); bool tryFlushRegions(); - void tryFlushRegion(RegionID region_id, bool try_persist = false); - void tryFlushRegion(const RegionPtr & region, bool try_persist); + RegionDataReadInfoList tryFlushRegion(RegionID region_id, bool try_persist = false); + RegionDataReadInfoList tryFlushRegion(const RegionPtr & region, bool try_persist); void handleInternalRegionsByTable(const TableID table_id, std::function && callback) const; std::vector> getRegionsByTable(const TableID table_id) const; @@ -156,7 +156,7 @@ class RegionTable : private boost::noncopyable bool shouldFlush(const InternalRegion & region) const; - void flushRegion(const RegionPtr & region, bool try_persist) const; + RegionDataReadInfoList flushRegion(const RegionPtr & region, bool try_persist) const; private: TableMap tables; From b22bbfbb45edba0352e9932929ba65dd9315eadb Mon Sep 17 00:00:00 2001 From: Tong Zhigao Date: Tue, 12 Nov 2019 23:56:56 +0800 Subject: [PATCH 04/12] rollback: StorageMergeTree::shutdown can remove table in RegionTable. --- dbms/src/Storages/StorageMergeTree.cpp | 1 + dbms/src/Storages/Transaction/RegionTable.cpp | 3 +++ dbms/src/Storages/Transaction/RegionTable.h | 1 + 3 files changed, 5 insertions(+) diff --git a/dbms/src/Storages/StorageMergeTree.cpp b/dbms/src/Storages/StorageMergeTree.cpp index e8662a2e392..386f5dee76e 100644 --- a/dbms/src/Storages/StorageMergeTree.cpp +++ b/dbms/src/Storages/StorageMergeTree.cpp @@ -115,6 +115,7 @@ void StorageMergeTree::shutdown() { TMTContext & tmt_context = context.getTMTContext(); tmt_context.getStorages().remove(data.table_info->id); + tmt_context.getRegionTable().removeTable(data.table_info->id); } } diff --git a/dbms/src/Storages/Transaction/RegionTable.cpp b/dbms/src/Storages/Transaction/RegionTable.cpp index 77b39009ec7..9d9fdfb75d7 100644 --- a/dbms/src/Storages/Transaction/RegionTable.cpp +++ b/dbms/src/Storages/Transaction/RegionTable.cpp @@ -215,7 +215,10 @@ void RegionTable::removeRegion(const RegionID region_id) auto & table = tables.find(table_id)->second; table.regions.erase(region_id); if (table.regions.empty()) + { table_to_optimize.insert(table_id); + tables.erase(table_id); + } } } diff --git a/dbms/src/Storages/Transaction/RegionTable.h b/dbms/src/Storages/Transaction/RegionTable.h index 5f8017f4b0b..1ed70a5ee8e 100644 --- a/dbms/src/Storages/Transaction/RegionTable.h +++ b/dbms/src/Storages/Transaction/RegionTable.h @@ -146,6 +146,7 @@ class RegionTable : private boost::noncopyable private: friend class MockTiDB; + friend class StorageMergeTree; Table & getOrCreateTable(const TableID table_id); void removeTable(TableID table_id); From 265428523f4d3b394ed9135eb7f00795210bf5d5 Mon Sep 17 00:00:00 2001 From: Tong Zhigao Date: Wed, 13 Nov 2019 10:25:24 +0800 Subject: [PATCH 05/12] remove mock test cases with error. --- tests/mutable-test/txn_schema/drop_on_read.test | 2 ++ tests/mutable-test/txn_schema/drop_on_restart.test | 2 ++ tests/mutable-test/txn_schema/drop_on_write.test | 2 ++ tests/mutable-test/txn_schema/truncate_on_read.test | 2 ++ tests/mutable-test/txn_schema/truncate_on_write.test | 2 ++ 5 files changed, 10 insertions(+) diff --git a/tests/mutable-test/txn_schema/drop_on_read.test b/tests/mutable-test/txn_schema/drop_on_read.test index 2e2f8e09a24..743d53528fe 100644 --- a/tests/mutable-test/txn_schema/drop_on_read.test +++ b/tests/mutable-test/txn_schema/drop_on_read.test @@ -1,3 +1,5 @@ +#RETURN + => DBGInvoke __enable_schema_sync_service('false') => DBGInvoke __drop_tidb_table(default, test) diff --git a/tests/mutable-test/txn_schema/drop_on_restart.test b/tests/mutable-test/txn_schema/drop_on_restart.test index 63df70c6e41..e972f86920b 100644 --- a/tests/mutable-test/txn_schema/drop_on_restart.test +++ b/tests/mutable-test/txn_schema/drop_on_restart.test @@ -1,3 +1,5 @@ +#RETURN + => DBGInvoke __enable_schema_sync_service('false') => DBGInvoke __drop_tidb_table(default, test) diff --git a/tests/mutable-test/txn_schema/drop_on_write.test b/tests/mutable-test/txn_schema/drop_on_write.test index 21c754fdf78..312d196637b 100644 --- a/tests/mutable-test/txn_schema/drop_on_write.test +++ b/tests/mutable-test/txn_schema/drop_on_write.test @@ -1,3 +1,5 @@ +#RETURN + => DBGInvoke __enable_schema_sync_service('false') => DBGInvoke __drop_tidb_table(default, test) diff --git a/tests/mutable-test/txn_schema/truncate_on_read.test b/tests/mutable-test/txn_schema/truncate_on_read.test index aa784c15813..b94baeed5e2 100644 --- a/tests/mutable-test/txn_schema/truncate_on_read.test +++ b/tests/mutable-test/txn_schema/truncate_on_read.test @@ -1,3 +1,5 @@ +#RETURN + => DBGInvoke __enable_schema_sync_service('false') => DBGInvoke __drop_tidb_table(default, test) diff --git a/tests/mutable-test/txn_schema/truncate_on_write.test b/tests/mutable-test/txn_schema/truncate_on_write.test index f1e9fb0c32c..5e82e8b8457 100644 --- a/tests/mutable-test/txn_schema/truncate_on_write.test +++ b/tests/mutable-test/txn_schema/truncate_on_write.test @@ -1,3 +1,5 @@ +#RETURN + => DBGInvoke __enable_schema_sync_service('false') => DBGInvoke __drop_tidb_table(default, test) From 0620ba57dfdbf810f215a151056731535b06e175 Mon Sep 17 00:00:00 2001 From: Tong Zhigao Date: Wed, 13 Nov 2019 10:44:09 +0800 Subject: [PATCH 06/12] fix log output --- dbms/src/Storages/Transaction/RegionTable.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dbms/src/Storages/Transaction/RegionTable.cpp b/dbms/src/Storages/Transaction/RegionTable.cpp index 9d9fdfb75d7..fdac4f26a56 100644 --- a/dbms/src/Storages/Transaction/RegionTable.cpp +++ b/dbms/src/Storages/Transaction/RegionTable.cpp @@ -247,7 +247,7 @@ RegionDataReadInfoList RegionTable::tryFlushRegion(const RegionPtr & region, boo } else { - LOG_WARNING(log, __FUNCTION__ << ": internal region " << region_id << " might be removed"); + LOG_WARNING(log, "Internal region " << region_id << " might be removed"); return false; } }; @@ -255,7 +255,7 @@ RegionDataReadInfoList RegionTable::tryFlushRegion(const RegionPtr & region, boo bool status = func_update_region([&](InternalRegion & internal_region) -> bool { if (internal_region.pause_flush) { - LOG_INFO(log, __FUNCTION__ << ": internal region " << region_id << " pause flush, may be being flushed"); + LOG_INFO(log, "Internal region " << region_id << " pause flush, may be being flushed"); return false; } internal_region.pause_flush = true; From d81cbcede0b0351332b10394e963266439667cfd Mon Sep 17 00:00:00 2001 From: Tong Zhigao Date: Wed, 13 Nov 2019 12:15:54 +0800 Subject: [PATCH 07/12] fix mock tests --- dbms/src/Debug/DBGInvoker.cpp | 1 + dbms/src/Debug/dbgFuncMockTiDBTable.cpp | 20 ++++++++++++++++++- dbms/src/Debug/dbgFuncMockTiDBTable.h | 5 +++++ .../MergeTree/MergeTreeDataSelectExecutor.cpp | 2 -- dbms/src/Storages/Transaction/RegionTable.cpp | 3 ++- .../txn_mock/partition_table.test | 2 ++ .../mutable-test/txn_schema/drop_on_read.test | 4 ++-- .../txn_schema/drop_on_restart.test | 3 +-- .../txn_schema/drop_on_write.test | 8 ++++++-- .../txn_schema/truncate_on_read.test | 4 ++-- .../txn_schema/truncate_on_write.test | 4 ++-- 11 files changed, 42 insertions(+), 14 deletions(-) diff --git a/dbms/src/Debug/DBGInvoker.cpp b/dbms/src/Debug/DBGInvoker.cpp index 0de4f5d26f4..e2ed1251a86 100644 --- a/dbms/src/Debug/DBGInvoker.cpp +++ b/dbms/src/Debug/DBGInvoker.cpp @@ -35,6 +35,7 @@ DBGInvoker::DBGInvoker() // TODO: remove this, use sleep in bash script regSchemalessFunc("sleep", dbgFuncSleep); + regSchemalessFunc("clean_up_region", MockTiDBTable::dbgFuncCleanUpRegions); regSchemalessFunc("mock_tidb_table", MockTiDBTable::dbgFuncMockTiDBTable); regSchemalessFunc("mock_tidb_db", MockTiDBTable::dbgFuncMockTiDBDB); regSchemalessFunc("mock_tidb_partition", MockTiDBTable::dbgFuncMockTiDBPartition); diff --git a/dbms/src/Debug/dbgFuncMockTiDBTable.cpp b/dbms/src/Debug/dbgFuncMockTiDBTable.cpp index 41badcadb75..85fe3c339ef 100644 --- a/dbms/src/Debug/dbgFuncMockTiDBTable.cpp +++ b/dbms/src/Debug/dbgFuncMockTiDBTable.cpp @@ -26,7 +26,8 @@ extern const int LOGICAL_ERROR; void MockTiDBTable::dbgFuncMockTiDBTable(Context & context, const ASTs & args, DBGInvoker::Printer output) { if (args.size() != 3 && args.size() != 4) - throw Exception("Args not matched, should be: database-name, table-name, schema-string [, handle_pk_name]", ErrorCodes::BAD_ARGUMENTS); + throw Exception( + "Args not matched, should be: database-name, table-name, schema-string [, handle_pk_name]", ErrorCodes::BAD_ARGUMENTS); const String & database_name = typeid_cast(*args[0]).name; const String & table_name = typeid_cast(*args[1]).name; @@ -287,4 +288,21 @@ void MockTiDBTable::dbgFuncTruncateTiDBTable(Context & /*context*/, const ASTs & output(ss.str()); } +void MockTiDBTable::dbgFuncCleanUpRegions(DB::Context & context, const DB::ASTs &, DB::DBGInvoker::Printer output) +{ + std::vector regions; + auto & kvstore = context.getTMTContext().getKVStore(); + auto & region_table = context.getTMTContext().getRegionTable(); + { + auto lock = kvstore->genTaskLock(); + + for (const auto & e : kvstore->regions()) + regions.emplace_back(e.first); + + for (const auto & region_id : regions) + kvstore->removeRegion(region_id, ®ion_table, lock); + } + output("all regions have been cleaned"); +} + } // namespace DB diff --git a/dbms/src/Debug/dbgFuncMockTiDBTable.h b/dbms/src/Debug/dbgFuncMockTiDBTable.h index 16da3030792..1f32f2744e7 100644 --- a/dbms/src/Debug/dbgFuncMockTiDBTable.h +++ b/dbms/src/Debug/dbgFuncMockTiDBTable.h @@ -77,6 +77,11 @@ struct MockTiDBTable // ./storages-client.sh "DBGInvoke reset_syncer()" static void dbgFuncResetSyncer(Context & context, const ASTs & args, DBGInvoker::Printer output); + // Remove All Region. + // Usage: + // ./storages-client.sh "DBGInvoke clean_up_region()" + static void dbgFuncCleanUpRegions(Context & context, const ASTs & args, DBGInvoker::Printer output); + private: static void dbgFuncDropTiDBTableImpl( Context & context, String database_name, String table_name, bool drop_regions, bool is_drop_db, DBGInvoker::Printer output); diff --git a/dbms/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp b/dbms/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp index 9bfc2c3fc14..5efd3a8fcb5 100644 --- a/dbms/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp +++ b/dbms/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp @@ -302,8 +302,6 @@ BlockInputStreams MergeTreeDataSelectExecutor::read(const Names & column_names_t regions_executor_data.reserve(regions.size()); for (const auto & [id, region] : regions) { - if (region == nullptr) - continue; regions_executor_data.emplace_back(RegionQueryInfo{id, region->version(), region->confVer(), {0, 0}}); } } diff --git a/dbms/src/Storages/Transaction/RegionTable.cpp b/dbms/src/Storages/Transaction/RegionTable.cpp index fdac4f26a56..3059cdb80b9 100644 --- a/dbms/src/Storages/Transaction/RegionTable.cpp +++ b/dbms/src/Storages/Transaction/RegionTable.cpp @@ -369,7 +369,8 @@ std::vector> RegionTable::getRegionsByTable(const for (const auto & region_info : internal_regions) { auto region = kvstore->getRegion(region_info.first); - regions.emplace_back(region_info.first, std::move(region)); + if (region) + regions.emplace_back(region_info.first, std::move(region)); } }); return regions; diff --git a/tests/mutable-test/txn_mock/partition_table.test b/tests/mutable-test/txn_mock/partition_table.test index d00ff4e98a9..2069fdfe986 100644 --- a/tests/mutable-test/txn_mock/partition_table.test +++ b/tests/mutable-test/txn_mock/partition_table.test @@ -1,3 +1,4 @@ +=> DBGInvoke __clean_up_region() => DBGInvoke __enable_schema_sync_service('false') => DBGInvoke __drop_tidb_table(default, test) @@ -84,3 +85,4 @@ Code: 60. DB::Exception: Received from {#WORD} DB::Exception: Table default.test Received exception from server (version {#WORD}): Code: 60. DB::Exception: Received from {#WORD} DB::Exception: Table default.test1_9999 doesn't exist.. => DBGInvoke __enable_schema_sync_service('true') +=> DBGInvoke __clean_up_region() diff --git a/tests/mutable-test/txn_schema/drop_on_read.test b/tests/mutable-test/txn_schema/drop_on_read.test index 743d53528fe..fbeebae48a6 100644 --- a/tests/mutable-test/txn_schema/drop_on_read.test +++ b/tests/mutable-test/txn_schema/drop_on_read.test @@ -1,5 +1,4 @@ -#RETURN - +=> DBGInvoke __clean_up_region() => DBGInvoke __enable_schema_sync_service('false') => DBGInvoke __drop_tidb_table(default, test) @@ -15,6 +14,7 @@ => select * from default.test " --schema_version "10000000 Received exception from server (version {#WORD}): Code: 60. DB::Exception: Received from {#WORD} DB::Exception: Table default.test doesn't exist.. +=> DBGInvoke __clean_up_region() => DBGInvoke __mock_tidb_table(default, test, 'col_1 String, col_2 Nullable(Int8)') => select * from default.test diff --git a/tests/mutable-test/txn_schema/drop_on_restart.test b/tests/mutable-test/txn_schema/drop_on_restart.test index e972f86920b..37d92695b6e 100644 --- a/tests/mutable-test/txn_schema/drop_on_restart.test +++ b/tests/mutable-test/txn_schema/drop_on_restart.test @@ -1,5 +1,4 @@ -#RETURN - +=> DBGInvoke __clean_up_region() => DBGInvoke __enable_schema_sync_service('false') => DBGInvoke __drop_tidb_table(default, test) diff --git a/tests/mutable-test/txn_schema/drop_on_write.test b/tests/mutable-test/txn_schema/drop_on_write.test index 312d196637b..a7824356790 100644 --- a/tests/mutable-test/txn_schema/drop_on_write.test +++ b/tests/mutable-test/txn_schema/drop_on_write.test @@ -1,5 +1,4 @@ -#RETURN - +=> DBGInvoke __clean_up_region() => DBGInvoke __enable_schema_sync_service('false') => DBGInvoke __drop_tidb_table(default, test) @@ -19,7 +18,9 @@ Received exception from server (version {#WORD}): Code: 60. DB::Exception: Received from {#WORD} DB::Exception: Table default.test doesn't exist.. +=> DBGInvoke __clean_up_region() => DBGInvoke __mock_tidb_table(default, test, 'col_1 String, col_2 Nullable(Int8)') +=> DBGInvoke __put_region(4, 0, 100, default, test) => DBGInvoke __refresh_schemas() => select col_1, col_2 from default.test => DBGInvoke __drop_column_from_tidb_table(default, test, col_2) @@ -30,7 +31,9 @@ Code: 60. DB::Exception: Received from {#WORD} DB::Exception: Table default.test Received exception from server (version {#WORD}): Code: 60. DB::Exception: Received from {#WORD} DB::Exception: Table default.test doesn't exist.. +=> DBGInvoke __clean_up_region() => DBGInvoke __mock_tidb_table(default, test, 'col_1 String, col_2 Nullable(Int8)') +=> DBGInvoke __put_region(4, 0, 100, default, test) => DBGInvoke __refresh_schemas() => select col_1, col_2 from default.test => DBGInvoke __modify_column_in_tidb_table(default, test, 'col_2 Nullable(Int64)') @@ -44,3 +47,4 @@ Code: 60. DB::Exception: Received from {#WORD} DB::Exception: Table default.test => DBGInvoke __drop_tidb_table(default, test) => drop table if exists default.test => DBGInvoke __enable_schema_sync_service('true') +=> DBGInvoke __clean_up_region() diff --git a/tests/mutable-test/txn_schema/truncate_on_read.test b/tests/mutable-test/txn_schema/truncate_on_read.test index b94baeed5e2..d6825c2299a 100644 --- a/tests/mutable-test/txn_schema/truncate_on_read.test +++ b/tests/mutable-test/txn_schema/truncate_on_read.test @@ -1,5 +1,4 @@ -#RETURN - +=> DBGInvoke __clean_up_region() => DBGInvoke __enable_schema_sync_service('false') => DBGInvoke __drop_tidb_table(default, test) @@ -25,3 +24,4 @@ => DBGInvoke __drop_tidb_table(default, test) => drop table if exists default.test => DBGInvoke __enable_schema_sync_service('true') +=> DBGInvoke __clean_up_region() diff --git a/tests/mutable-test/txn_schema/truncate_on_write.test b/tests/mutable-test/txn_schema/truncate_on_write.test index 5e82e8b8457..15650139de6 100644 --- a/tests/mutable-test/txn_schema/truncate_on_write.test +++ b/tests/mutable-test/txn_schema/truncate_on_write.test @@ -1,5 +1,4 @@ -#RETURN - +=> DBGInvoke __clean_up_region() => DBGInvoke __enable_schema_sync_service('false') => DBGInvoke __drop_tidb_table(default, test) @@ -24,3 +23,4 @@ => DBGInvoke __drop_tidb_table(default, test) => drop table if exists default.test => DBGInvoke __enable_schema_sync_service('true') +=> DBGInvoke __clean_up_region() From 9ea111e42e0868585be351db5bb3f9521e91c257 Mon Sep 17 00:00:00 2001 From: Tong Zhigao Date: Wed, 13 Nov 2019 19:15:13 +0800 Subject: [PATCH 08/12] address comment --- .../Storages/Transaction/PartitionStreams.cpp | 4 ++-- dbms/src/Storages/Transaction/Region.cpp | 10 ++++---- dbms/src/Storages/Transaction/Region.h | 4 ++-- .../Storages/Transaction/RegionRangeKeys.h | 6 ++--- dbms/src/Storages/Transaction/RegionState.cpp | 12 +++++----- dbms/src/Storages/Transaction/RegionTable.cpp | 23 ++++++++----------- .../Storages/Transaction/applySnapshot.cpp | 2 +- 7 files changed, 28 insertions(+), 33 deletions(-) diff --git a/dbms/src/Storages/Transaction/PartitionStreams.cpp b/dbms/src/Storages/Transaction/PartitionStreams.cpp index d5fd206f95d..add7aeb27c0 100644 --- a/dbms/src/Storages/Transaction/PartitionStreams.cpp +++ b/dbms/src/Storages/Transaction/PartitionStreams.cpp @@ -23,7 +23,7 @@ extern const int LOGICAL_ERROR; void RegionTable::writeBlockByRegion(Context & context, RegionPtr region, RegionDataReadInfoList & data_list_to_remove, Logger * log) { const auto & tmt = context.getTMTContext(); - TableID table_id = region->getFlashTableID(); + TableID table_id = region->getMappedTableID(); UInt64 region_read_cost = -1, region_decode_cost = -1, write_part_cost = -1; RegionDataReadInfoList data_list_read; @@ -129,7 +129,7 @@ std::tuple RegionTable::readBlockByReg if (!region) throw Exception(std::string(__PRETTY_FUNCTION__) + ": region is null", ErrorCodes::LOGICAL_ERROR); - if (region->getFlashTableID() != table_info.id) + if (region->getMappedTableID() != table_info.id) throw Exception(std::string(__PRETTY_FUNCTION__) + ": table id not match", ErrorCodes::LOGICAL_ERROR); RegionDataReadInfoList data_list_read; diff --git a/dbms/src/Storages/Transaction/Region.cpp b/dbms/src/Storages/Transaction/Region.cpp index d7989b83d28..7340408366f 100644 --- a/dbms/src/Storages/Transaction/Region.cpp +++ b/dbms/src/Storages/Transaction/Region.cpp @@ -49,9 +49,9 @@ void Region::doInsert(const std::string & cf, TiKVKey && key, TiKVValue && value void Region::doCheckTable(const DB::DecodedTiKVKey & raw_key) const { auto table_id = RecordKVFormat::getTableId(raw_key); - if (table_id != getFlashTableID()) + if (table_id != getMappedTableID()) { - LOG_ERROR(log, __FUNCTION__ << ": table id not match, except " << getFlashTableID() << ", got " << table_id); + LOG_ERROR(log, __FUNCTION__ << ": table id not match, except " << getMappedTableID() << ", got " << table_id); throw Exception(std::string(__PRETTY_FUNCTION__) + ": table id not match", ErrorCodes::LOGICAL_ERROR); } } @@ -507,7 +507,7 @@ void Region::compareAndCompleteSnapshot(HandleMap & handle_map, const Timestamp if (handle_map.empty()) return; - auto table_id = getFlashTableID(); + auto table_id = getMappedTableID(); auto & write_map = data.writeCF().getDataMut(); size_t deleted_gc_cnt = 0, ori_write_map_size = write_map.size(); @@ -634,10 +634,10 @@ Region::Region(DB::RegionMeta && meta_, const DB::IndexReaderCreateFunc & index_ : meta(std::move(meta_)), index_reader(index_reader_create(meta.getRegionVerID())), log(&Logger::get(log_name)), - flash_table_id(meta.getRange()->getFlashTableID()) + mapped_table_id(meta.getRange()->getMappedTableID()) {} -TableID Region::getFlashTableID() const { return flash_table_id; } +TableID Region::getMappedTableID() const { return mapped_table_id; } const RegionRangeKeys & RegionRaftCommandDelegate::getRange() { return *meta.makeRaftCommandDelegate().regionState().getRange(); } UInt64 RegionRaftCommandDelegate::appliedIndex() { return meta.makeRaftCommandDelegate().applyState().applied_index(); } diff --git a/dbms/src/Storages/Transaction/Region.h b/dbms/src/Storages/Transaction/Region.h index 53dec56412a..61d56060b2f 100644 --- a/dbms/src/Storages/Transaction/Region.h +++ b/dbms/src/Storages/Transaction/Region.h @@ -155,7 +155,7 @@ class Region : public std::enable_shared_from_this void tryPreDecodeTiKVValue(); - TableID getFlashTableID() const; + TableID getMappedTableID() const; private: Region() = delete; @@ -191,7 +191,7 @@ class Region : public std::enable_shared_from_this Logger * log; - const TableID flash_table_id; + const TableID mapped_table_id; }; class RegionRaftCommandDelegate : public Region, private boost::noncopyable diff --git a/dbms/src/Storages/Transaction/RegionRangeKeys.h b/dbms/src/Storages/Transaction/RegionRangeKeys.h index dfabdea8ef2..be01a1d5ece 100644 --- a/dbms/src/Storages/Transaction/RegionRangeKeys.h +++ b/dbms/src/Storages/Transaction/RegionRangeKeys.h @@ -41,13 +41,13 @@ class RegionRangeKeys : boost::noncopyable const std::pair & rawKeys() const; HandleRange getHandleRangeByTable(const TableID table_id) const; explicit RegionRangeKeys(TiKVKey && start_key, TiKVKey && end_key); - TableID getFlashTableID() const; + TableID getMappedTableID() const; private: RegionRange ori; std::pair raw; - TableID flash_table_id; - HandleRange flash_handle_range; + TableID mapped_table_id; + HandleRange mapped_handle_range; }; } // namespace DB diff --git a/dbms/src/Storages/Transaction/RegionState.cpp b/dbms/src/Storages/Transaction/RegionState.cpp index b5e686795a0..4df21797b24 100644 --- a/dbms/src/Storages/Transaction/RegionState.cpp +++ b/dbms/src/Storages/Transaction/RegionState.cpp @@ -72,21 +72,21 @@ RegionRangeKeys::RegionRangeKeys(TiKVKey && start_key, TiKVKey && end_key) : ori(RegionRangeKeys::makeComparableKeys(std::move(start_key), std::move(end_key))), raw(ori.first.key.empty() ? DecodedTiKVKey() : RecordKVFormat::decodeTiKVKey(ori.first.key), ori.second.key.empty() ? DecodedTiKVKey() : RecordKVFormat::decodeTiKVKey(ori.second.key)), - flash_table_id(computeFlashTableID(raw.first)), - flash_handle_range(TiKVRange::getHandleRangeByTable(rawKeys().first, rawKeys().second, flash_table_id)) + mapped_table_id(computeFlashTableID(raw.first)), + mapped_handle_range(TiKVRange::getHandleRangeByTable(rawKeys().first, rawKeys().second, mapped_table_id)) { - if (flash_handle_range.first == flash_handle_range.second) + if (mapped_handle_range.first == mapped_handle_range.second) throw Exception(std::string(__PRETTY_FUNCTION__) + " got empty handle range", ErrorCodes::LOGICAL_ERROR); } -TableID RegionRangeKeys::getFlashTableID() const { return flash_table_id; } +TableID RegionRangeKeys::getMappedTableID() const { return mapped_table_id; } const std::pair & RegionRangeKeys::rawKeys() const { return raw; } HandleRange RegionRangeKeys::getHandleRangeByTable(const TableID table_id) const { - if (table_id == flash_table_id) - return flash_handle_range; + if (table_id == mapped_table_id) + return mapped_handle_range; return TiKVRange::getHandleRangeByTable(rawKeys().first, rawKeys().second, table_id); } diff --git a/dbms/src/Storages/Transaction/RegionTable.cpp b/dbms/src/Storages/Transaction/RegionTable.cpp index 3059cdb80b9..fca296d82d8 100644 --- a/dbms/src/Storages/Transaction/RegionTable.cpp +++ b/dbms/src/Storages/Transaction/RegionTable.cpp @@ -55,7 +55,7 @@ RegionTable::InternalRegion & RegionTable::doGetInternalRegion(DB::TableID table RegionTable::InternalRegion & RegionTable::getOrInsertRegion(const Region & region) { - auto table_id = region.getFlashTableID(); + auto table_id = region.getMappedTableID(); auto & table = getOrCreateTable(table_id); auto & table_regions = table.regions; if (auto it = table_regions.find(region.id()); it != table_regions.end()) @@ -68,7 +68,7 @@ void RegionTable::shrinkRegionRange(const Region & region) { std::lock_guard lock(mutex); auto & internal_region = getOrInsertRegion(region); - internal_region.range_in_table = region.getHandleRangeByTable(region.getFlashTableID()); + internal_region.range_in_table = region.getHandleRangeByTable(region.getMappedTableID()); } bool RegionTable::shouldFlush(const InternalRegion & region) const @@ -91,7 +91,7 @@ RegionDataReadInfoList RegionTable::flushRegion(const RegionPtr & region, bool t const auto & tmt = context->getTMTContext(); LOG_INFO(log, - __FUNCTION__ << ": table " << region->getFlashTableID() << ", " << region->toString(false) << " original " << region->dataSize() + __FUNCTION__ << ": table " << region->getMappedTableID() << ", " << region->toString(false) << " original " << region->dataSize() << " bytes"); /// Write region data into corresponding storage. @@ -124,7 +124,7 @@ RegionDataReadInfoList RegionTable::flushRegion(const RegionPtr & region, bool t } LOG_INFO(log, - __FUNCTION__ << ": table " << region->getFlashTableID() << ", " << region->toString(false) << " after flush " << cache_size + __FUNCTION__ << ": table " << region->getMappedTableID() << ", " << region->toString(false) << " after flush " << cache_size << " bytes"); } @@ -306,7 +306,6 @@ bool RegionTable::tryFlushRegions() { RegionID region_id; bool got = false; - RegionID useless_region = InvalidRegionID; }; DataToFlush to_flush; @@ -315,16 +314,18 @@ bool RegionTable::tryFlushRegions() const auto traverse_dirty_regions = [&](std::function && callback) { std::lock_guard lock(mutex); - for (const auto & region_id : dirty_regions) + for (auto dirty_it = dirty_regions.begin(); dirty_it != dirty_regions.end();) { + auto region_id = *dirty_it; if (auto it = regions.find(region_id); it != regions.end()) { auto table_id = it->second; if (callback(doGetInternalRegion(table_id, region_id))) return; + dirty_it++; } else - to_flush.useless_region = region_id; + dirty_it = dirty_regions.erase(dirty_it); } }; @@ -338,12 +339,6 @@ bool RegionTable::tryFlushRegions() return false; }); - if (to_flush.useless_region != InvalidRegionID) - { - std::lock_guard lock(mutex); - dirty_regions.erase(to_flush.useless_region); - } - if (!to_flush.got) return false; } @@ -385,7 +380,7 @@ void RegionTable::extendRegionRange(const RegionID region_id, const RegionRangeK { std::lock_guard lock(mutex); - auto table_id = region_range_keys.getFlashTableID(); + auto table_id = region_range_keys.getMappedTableID(); auto new_handle_range = region_range_keys.getHandleRangeByTable(table_id); if (auto it = regions.find(region_id); it != regions.end()) diff --git a/dbms/src/Storages/Transaction/applySnapshot.cpp b/dbms/src/Storages/Transaction/applySnapshot.cpp index afd3eb435c7..6b87e59172a 100644 --- a/dbms/src/Storages/Transaction/applySnapshot.cpp +++ b/dbms/src/Storages/Transaction/applySnapshot.cpp @@ -71,7 +71,7 @@ bool applySnapshot(const KVStorePtr & kvstore, RegionPtr new_region, Context * c } // Traverse all table in ch and update handle_maps. - auto table_id = new_region->getFlashTableID(); + auto table_id = new_region->getMappedTableID(); if (auto storage = tmt.getStorages().get(table_id); storage) { const auto handle_range = new_region->getHandleRangeByTable(table_id); From 12f1d0928d8f36b0463c4719cfb3e94c21fb5855 Mon Sep 17 00:00:00 2001 From: Tong Zhigao Date: Wed, 13 Nov 2019 19:39:07 +0800 Subject: [PATCH 09/12] move checking table id while reading to MergeTreeDataSelectExecutor::read --- dbms/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp | 5 +++++ dbms/src/Storages/Transaction/PartitionStreams.cpp | 3 --- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/dbms/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp b/dbms/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp index 5efd3a8fcb5..1f30ed64d85 100644 --- a/dbms/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp +++ b/dbms/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp @@ -343,6 +343,11 @@ BlockInputStreams MergeTreeDataSelectExecutor::read(const Names & column_names_t // wait learner read index auto region = kvstore_region[region_query_info.region_id]; + if (region->getMappedTableID() != data.table_info->id) + throw Exception(std::string(__PRETTY_FUNCTION__) + ": table id not match, except " + + std::to_string(region->getMappedTableID()) + ", got " + std::to_string(data.table_info->id), + ErrorCodes::LOGICAL_ERROR); + /// Blocking learner read. Note that learner read must be performed ahead of data read, /// otherwise the desired index will be blocked by the lock of data read. region->waitIndex(region->learnerRead()); diff --git a/dbms/src/Storages/Transaction/PartitionStreams.cpp b/dbms/src/Storages/Transaction/PartitionStreams.cpp index add7aeb27c0..4436d64e041 100644 --- a/dbms/src/Storages/Transaction/PartitionStreams.cpp +++ b/dbms/src/Storages/Transaction/PartitionStreams.cpp @@ -129,9 +129,6 @@ std::tuple RegionTable::readBlockByReg if (!region) throw Exception(std::string(__PRETTY_FUNCTION__) + ": region is null", ErrorCodes::LOGICAL_ERROR); - if (region->getMappedTableID() != table_info.id) - throw Exception(std::string(__PRETTY_FUNCTION__) + ": table id not match", ErrorCodes::LOGICAL_ERROR); - RegionDataReadInfoList data_list_read; { auto scanner = region->createCommittedScanner(); From 3c39fb57f48d27cae7f39337dd0dd666c9ef6f2e Mon Sep 17 00:00:00 2001 From: Tong Zhigao Date: Wed, 13 Nov 2019 19:50:23 +0800 Subject: [PATCH 10/12] rename computeFlashTableID to computeMappedTableID. --- dbms/src/Storages/Transaction/RegionState.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dbms/src/Storages/Transaction/RegionState.cpp b/dbms/src/Storages/Transaction/RegionState.cpp index 4df21797b24..521bd888f77 100644 --- a/dbms/src/Storages/Transaction/RegionState.cpp +++ b/dbms/src/Storages/Transaction/RegionState.cpp @@ -58,7 +58,7 @@ const RegionState::Base & RegionState::getBase() const { return *this; } const raft_serverpb::MergeState & RegionState::getMergeState() const { return merge_state(); } raft_serverpb::MergeState & RegionState::getMutMergeState() { return *mutable_merge_state(); } -TableID computeFlashTableID(const DecodedTiKVKey & key) +TableID computeMappedTableID(const DecodedTiKVKey & key) { // t table_id _r if (key.size() >= (1 + 8 + 2) && key[0] == RecordKVFormat::TABLE_PREFIX @@ -72,7 +72,7 @@ RegionRangeKeys::RegionRangeKeys(TiKVKey && start_key, TiKVKey && end_key) : ori(RegionRangeKeys::makeComparableKeys(std::move(start_key), std::move(end_key))), raw(ori.first.key.empty() ? DecodedTiKVKey() : RecordKVFormat::decodeTiKVKey(ori.first.key), ori.second.key.empty() ? DecodedTiKVKey() : RecordKVFormat::decodeTiKVKey(ori.second.key)), - mapped_table_id(computeFlashTableID(raw.first)), + mapped_table_id(computeMappedTableID(raw.first)), mapped_handle_range(TiKVRange::getHandleRangeByTable(rawKeys().first, rawKeys().second, mapped_table_id)) { if (mapped_handle_range.first == mapped_handle_range.second) From 905b5bb1ba011ceb3a71f3ee71dc26d2a64f8442 Mon Sep 17 00:00:00 2001 From: Tong Zhigao Date: Wed, 13 Nov 2019 20:22:02 +0800 Subject: [PATCH 11/12] fix typo --- dbms/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp | 2 +- dbms/src/Storages/Transaction/Region.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dbms/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp b/dbms/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp index 1f30ed64d85..b57ccf3a469 100644 --- a/dbms/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp +++ b/dbms/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp @@ -344,7 +344,7 @@ BlockInputStreams MergeTreeDataSelectExecutor::read(const Names & column_names_t auto region = kvstore_region[region_query_info.region_id]; if (region->getMappedTableID() != data.table_info->id) - throw Exception(std::string(__PRETTY_FUNCTION__) + ": table id not match, except " + throw Exception(std::string(__PRETTY_FUNCTION__) + ": table id not match, expect " + std::to_string(region->getMappedTableID()) + ", got " + std::to_string(data.table_info->id), ErrorCodes::LOGICAL_ERROR); diff --git a/dbms/src/Storages/Transaction/Region.cpp b/dbms/src/Storages/Transaction/Region.cpp index 7340408366f..562695b193b 100644 --- a/dbms/src/Storages/Transaction/Region.cpp +++ b/dbms/src/Storages/Transaction/Region.cpp @@ -51,7 +51,7 @@ void Region::doCheckTable(const DB::DecodedTiKVKey & raw_key) const auto table_id = RecordKVFormat::getTableId(raw_key); if (table_id != getMappedTableID()) { - LOG_ERROR(log, __FUNCTION__ << ": table id not match, except " << getMappedTableID() << ", got " << table_id); + LOG_ERROR(log, __FUNCTION__ << ": table id not match, expect " << getMappedTableID() << ", got " << table_id); throw Exception(std::string(__PRETTY_FUNCTION__) + ": table id not match", ErrorCodes::LOGICAL_ERROR); } } From a26646a5b891ef24b0015bcdf232a906ce899a5e Mon Sep 17 00:00:00 2001 From: Tong Zhigao Date: Wed, 13 Nov 2019 20:30:33 +0800 Subject: [PATCH 12/12] refactor RegionTable::tryFlushRegions --- dbms/src/Storages/Transaction/RegionTable.cpp | 66 +++++++------------ dbms/src/Storages/Transaction/RegionTable.h | 2 +- 2 files changed, 25 insertions(+), 43 deletions(-) diff --git a/dbms/src/Storages/Transaction/RegionTable.cpp b/dbms/src/Storages/Transaction/RegionTable.cpp index fca296d82d8..7fefebac026 100644 --- a/dbms/src/Storages/Transaction/RegionTable.cpp +++ b/dbms/src/Storages/Transaction/RegionTable.cpp @@ -294,58 +294,40 @@ RegionDataReadInfoList RegionTable::tryFlushRegion(const RegionPtr & region, boo return data_list_to_remove; } -bool RegionTable::tryFlushRegions() +RegionID RegionTable::pickRegionToFlush() { - { - std::lock_guard lock(mutex); - if (dirty_regions.empty()) - return false; - } - - struct DataToFlush - { - RegionID region_id; - bool got = false; - }; + std::lock_guard lock(mutex); - DataToFlush to_flush; + for (auto dirty_it = dirty_regions.begin(); dirty_it != dirty_regions.end();) { - // judge choose region to flush - const auto traverse_dirty_regions = [&](std::function && callback) { - std::lock_guard lock(mutex); - - for (auto dirty_it = dirty_regions.begin(); dirty_it != dirty_regions.end();) - { - auto region_id = *dirty_it; - if (auto it = regions.find(region_id); it != regions.end()) - { - auto table_id = it->second; - if (callback(doGetInternalRegion(table_id, region_id))) - return; - dirty_it++; - } - else - dirty_it = dirty_regions.erase(dirty_it); - } - }; + auto region_id = *dirty_it; + if (auto it = regions.find(region_id); it != regions.end()) + { + auto table_id = it->second; - traverse_dirty_regions([&](InternalRegion & region) { - if (shouldFlush(region)) + if (shouldFlush(doGetInternalRegion(table_id, region_id))) { - to_flush = DataToFlush{region.region_id, true}; - dirty_regions.erase(region.region_id); - return true; + dirty_regions.erase(dirty_it); + return region_id; } - return false; - }); - if (!to_flush.got) - return false; + dirty_it++; + } + else + dirty_it = dirty_regions.erase(dirty_it); } + return InvalidRegionID; +} - tryFlushRegion(to_flush.region_id, true); +bool RegionTable::tryFlushRegions() +{ + if (RegionID region_to_flush = pickRegionToFlush(); region_to_flush != InvalidRegionID) + { + tryFlushRegion(region_to_flush, true); + return true; + } - return true; + return false; } void RegionTable::handleInternalRegionsByTable(const TableID table_id, std::function && callback) const diff --git a/dbms/src/Storages/Transaction/RegionTable.h b/dbms/src/Storages/Transaction/RegionTable.h index 1ed70a5ee8e..b557b4f59db 100644 --- a/dbms/src/Storages/Transaction/RegionTable.h +++ b/dbms/src/Storages/Transaction/RegionTable.h @@ -156,7 +156,7 @@ class RegionTable : private boost::noncopyable InternalRegion & doGetInternalRegion(TableID table_id, RegionID region_id); bool shouldFlush(const InternalRegion & region) const; - + RegionID pickRegionToFlush(); RegionDataReadInfoList flushRegion(const RegionPtr & region, bool try_persist) const; private: