From 29e97a9c9b5b8946d38b17628b9e870a504d6b75 Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Tue, 28 Jun 2022 16:57:28 +0800 Subject: [PATCH 01/14] Schema: allow load empty schema diff when version grow up --- dbms/src/Debug/MockSchemaGetter.h | 8 +- dbms/src/Debug/MockTiDB.cpp | 2 +- dbms/src/Debug/MockTiDB.h | 2 +- .../Storages/Transaction/ReadIndexWorker.cpp | 2 +- dbms/src/TiDB/Schema/SchemaGetter.cpp | 6 +- dbms/src/TiDB/Schema/SchemaGetter.h | 4 +- dbms/src/TiDB/Schema/TiDBSchemaSyncer.h | 80 ++++++++++--------- 7 files changed, 57 insertions(+), 47 deletions(-) diff --git a/dbms/src/Debug/MockSchemaGetter.h b/dbms/src/Debug/MockSchemaGetter.h index f02699866ce..b4576d00cad 100644 --- a/dbms/src/Debug/MockSchemaGetter.h +++ b/dbms/src/Debug/MockSchemaGetter.h @@ -17,16 +17,20 @@ #include #include +#include + namespace DB { - struct MockSchemaGetter { TiDB::DBInfoPtr getDatabase(DatabaseID db_id) { return MockTiDB::instance().getDBInfoByID(db_id); } Int64 getVersion() { return MockTiDB::instance().getVersion(); } - SchemaDiff getSchemaDiff(Int64 version) { return MockTiDB::instance().getSchemaDiff(version); } + std::optional getSchemaDiff(Int64 version) + { + return MockTiDB::instance().getSchemaDiff(version); + } TiDB::TableInfoPtr getTableInfo(DatabaseID, TableID table_id) { return MockTiDB::instance().getTableInfoByID(table_id); } diff --git a/dbms/src/Debug/MockTiDB.cpp b/dbms/src/Debug/MockTiDB.cpp index 42ab56a97c1..d3d8c8c8712 100644 --- a/dbms/src/Debug/MockTiDB.cpp +++ b/dbms/src/Debug/MockTiDB.cpp @@ -669,7 +669,7 @@ std::pair MockTiDB::getDBIDByName(const String & database_name return std::make_pair(false, -1); } -SchemaDiff MockTiDB::getSchemaDiff(Int64 version_) +std::optional MockTiDB::getSchemaDiff(Int64 version_) { return version_diff[version_]; } diff --git a/dbms/src/Debug/MockTiDB.h b/dbms/src/Debug/MockTiDB.h index 36d2af90859..b0ec9276113 100644 --- a/dbms/src/Debug/MockTiDB.h +++ b/dbms/src/Debug/MockTiDB.h @@ -127,7 +127,7 @@ class MockTiDB : public ext::Singleton std::pair getDBIDByName(const String & database_name); - SchemaDiff getSchemaDiff(Int64 version); + std::optional getSchemaDiff(Int64 version); std::unordered_map getDatabases() { return databases; } diff --git a/dbms/src/Storages/Transaction/ReadIndexWorker.cpp b/dbms/src/Storages/Transaction/ReadIndexWorker.cpp index 3223c815989..7de79dd5c6d 100644 --- a/dbms/src/Storages/Transaction/ReadIndexWorker.cpp +++ b/dbms/src/Storages/Transaction/ReadIndexWorker.cpp @@ -880,7 +880,7 @@ BatchReadIndexRes ReadIndexWorkerManager::batchReadIndex( } } { // if meet timeout, which means part of regions can not get response from leader, try to poll rest tasks - TEST_LOG_FMT("rest {}, poll rest tasks onece", tasks.size()); + TEST_LOG_FMT("rest {}, poll rest tasks once", tasks.size()); while (!tasks.empty()) { diff --git a/dbms/src/TiDB/Schema/SchemaGetter.cpp b/dbms/src/TiDB/Schema/SchemaGetter.cpp index 7f52f9301b1..29ececa8895 100644 --- a/dbms/src/TiDB/Schema/SchemaGetter.cpp +++ b/dbms/src/TiDB/Schema/SchemaGetter.cpp @@ -19,7 +19,6 @@ namespace DB { - namespace ErrorCodes { extern const int SCHEMA_SYNC_ERROR; @@ -193,13 +192,14 @@ String SchemaGetter::getSchemaDiffKey(Int64 ver) return std::string(schemaDiffPrefix) + ":" + std::to_string(ver); } -SchemaDiff SchemaGetter::getSchemaDiff(Int64 ver) +std::optional SchemaGetter::getSchemaDiff(Int64 ver) { String key = getSchemaDiffKey(ver); String data = TxnStructure::get(snap, key); if (data.empty()) { - throw TiFlashException("cannot find schema diff for version: " + std::to_string(ver), Errors::Table::SyncError); + LOG_FMT_WARNING(log, "The schema diff for version {}, key {} is empty, waitting for next round to load schema diff", ver, key); + return std::nullopt; } SchemaDiff diff; diff.deserialize(data); diff --git a/dbms/src/TiDB/Schema/SchemaGetter.h b/dbms/src/TiDB/Schema/SchemaGetter.h index 02d2f7a7c88..075f8af4b26 100644 --- a/dbms/src/TiDB/Schema/SchemaGetter.h +++ b/dbms/src/TiDB/Schema/SchemaGetter.h @@ -26,6 +26,8 @@ #include +#include + namespace DB { // The enum results are completely the same as the DDL Action listed in the "parser/model/ddl.go" of TiDB codebase, which must be keeping in sync. @@ -138,7 +140,7 @@ struct SchemaGetter Int64 getVersion(); - SchemaDiff getSchemaDiff(Int64 ver); + std::optional getSchemaDiff(Int64 ver); static String getSchemaDiffKey(Int64 ver); diff --git a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h index 4fdba195acb..4870e912195 100644 --- a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h +++ b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h @@ -113,12 +113,13 @@ struct TiDBSchemaSyncer : public SchemaSyncer SCOPE_EXIT({ GET_METRIC(tiflash_schema_applying).Set(0.0); }); GET_METRIC(tiflash_schema_apply_count, type_diff).Increment(); - if (!tryLoadSchemaDiffs(getter, version, context)) + Int64 version_after_load_diff = 0; + if (version_after_load_diff = tryLoadSchemaDiffs(getter, version, context); version_after_load_diff == -1) { GET_METRIC(tiflash_schema_apply_count, type_full).Increment(); loadAllSchema(getter, version, context); } - cur_version = version; + cur_version = version_after_load_diff; GET_METRIC(tiflash_schema_version).Set(cur_version); LOG_FMT_INFO(log, "end sync schema, version has been updated to {}", cur_version); return true; @@ -144,7 +145,7 @@ struct TiDBSchemaSyncer : public SchemaSyncer return it->second; } - bool tryLoadSchemaDiffs(Getter & getter, Int64 version, Context & context) + Int64 tryLoadSchemaDiffs(Getter & getter, Int64 version, Context & context) { if (isTooOldSchema(cur_version, version)) { @@ -156,52 +157,55 @@ struct TiDBSchemaSyncer : public SchemaSyncer SchemaBuilder builder(getter, context, databases, version); Int64 used_version = cur_version; - std::vector diffs; while (used_version < version) { used_version++; - diffs.push_back(getter.getSchemaDiff(used_version)); - } - LOG_FMT_DEBUG(log, "end load schema diffs with total {} entries.", diffs.size()); - try - { - for (const auto & diff : diffs) + auto schema_diff = getter.getSchemaDiff(used_version); + if (!schema_diff) { - builder.applyDiff(diff); + LOG_FMT_DEBUG(log, "End load a part of schema diffs, current version is {} ", used_version); + return used_version; } - } - catch (TiFlashException & e) - { - if (!e.getError().is(Errors::DDL::StaleSchema)) + + try + { + builder.applyDiff(*schema_diff); + } + catch (TiFlashException & e) { + if (!e.getError().is(Errors::DDL::StaleSchema)) + { + GET_METRIC(tiflash_schema_apply_count, type_failed).Increment(); + } + LOG_FMT_WARNING(log, "apply diff meets exception : {} \n stack is {}", e.displayText(), e.getStackTrace().toString()); + return -1; + } + catch (Exception & e) + { + if (e.code() == ErrorCodes::FAIL_POINT_ERROR) + { + throw; + } GET_METRIC(tiflash_schema_apply_count, type_failed).Increment(); + LOG_FMT_WARNING(log, "apply diff meets exception : {} \n stack is {}", e.displayText(), e.getStackTrace().toString()); + return -1; } - LOG_FMT_WARNING(log, "apply diff meets exception : {} \n stack is {}", e.displayText(), e.getStackTrace().toString()); - return false; - } - catch (Exception & e) - { - if (e.code() == ErrorCodes::FAIL_POINT_ERROR) + catch (Poco::Exception & e) { - throw; + GET_METRIC(tiflash_schema_apply_count, type_failed).Increment(); + LOG_FMT_WARNING(log, "apply diff meets exception : {}", e.displayText()); + return -1; + } + catch (std::exception & e) + { + GET_METRIC(tiflash_schema_apply_count, type_failed).Increment(); + LOG_FMT_WARNING(log, "apply diff meets exception : {}", e.what()); + return -1; } - GET_METRIC(tiflash_schema_apply_count, type_failed).Increment(); - LOG_FMT_WARNING(log, "apply diff meets exception : {} \n stack is {}", e.displayText(), e.getStackTrace().toString()); - return false; - } - catch (Poco::Exception & e) - { - GET_METRIC(tiflash_schema_apply_count, type_failed).Increment(); - LOG_FMT_WARNING(log, "apply diff meets exception : {}", e.displayText()); - return false; - } - catch (std::exception & e) - { - GET_METRIC(tiflash_schema_apply_count, type_failed).Increment(); - LOG_FMT_WARNING(log, "apply diff meets exception : {}", e.what()); - return false; } - return true; + + LOG_FMT_DEBUG(log, "End load all of schema diffs, current version is {} ", used_version); + return used_version; } void loadAllSchema(Getter & getter, Int64 version, Context & context) From 6f9860033709197bf15ab526ea8bb949fe06d7a5 Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Tue, 28 Jun 2022 17:30:14 +0800 Subject: [PATCH 02/14] update --- dbms/src/TiDB/Schema/TiDBSchemaSyncer.h | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h index 4870e912195..197006de97c 100644 --- a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h +++ b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h @@ -145,26 +145,41 @@ struct TiDBSchemaSyncer : public SchemaSyncer return it->second; } - Int64 tryLoadSchemaDiffs(Getter & getter, Int64 version, Context & context) + Int64 tryLoadSchemaDiffs(Getter & getter, Int64 lastest_version, Context & context) { - if (isTooOldSchema(cur_version, version)) + if (isTooOldSchema(cur_version, lastest_version)) { return false; } LOG_FMT_DEBUG(log, "try load schema diffs."); - SchemaBuilder builder(getter, context, databases, version); + SchemaBuilder builder(getter, context, databases, lastest_version); Int64 used_version = cur_version; - while (used_version < version) + while (used_version < lastest_version) { used_version++; auto schema_diff = getter.getSchemaDiff(used_version); if (!schema_diff) { + // If `schema diff` from `lastest_version` got empty `schema diff` + // Then we won't apply to `lastest_version`, but we will apply to `lastest_version - 1` + // If `schema diff` from [`cur_version`, `lastest_version - 1`] got empty `schema diff` + // Then we should just skip it. + // + // example: + // - `cur_version` is 1, `lastest_version` is 10 + // - The schema diff of schema version [2,4,6] is empty, Then we just skip it. + // - The schema diff of schema version 10 is empty, Then we should just apply version into 9 + if (used_version != lastest_version) + { + LOG_FMT_WARNING(log, "Skip the schema diff from version {}. ", used_version); + continue; + } // else (used_version == lastest_version) + LOG_FMT_DEBUG(log, "End load a part of schema diffs, current version is {} ", used_version); - return used_version; + return used_version - 1; } try From a39e985c1fc80d6e022f951cab6f22ad64875aa2 Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Tue, 28 Jun 2022 17:37:29 +0800 Subject: [PATCH 03/14] update log --- dbms/src/TiDB/Schema/SchemaGetter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/TiDB/Schema/SchemaGetter.cpp b/dbms/src/TiDB/Schema/SchemaGetter.cpp index 29ececa8895..3bf4969e646 100644 --- a/dbms/src/TiDB/Schema/SchemaGetter.cpp +++ b/dbms/src/TiDB/Schema/SchemaGetter.cpp @@ -198,7 +198,7 @@ std::optional SchemaGetter::getSchemaDiff(Int64 ver) String data = TxnStructure::get(snap, key); if (data.empty()) { - LOG_FMT_WARNING(log, "The schema diff for version {}, key {} is empty, waitting for next round to load schema diff", ver, key); + LOG_FMT_WARNING(log, "The schema diff for version {}, key {} is empty.", ver, key); return std::nullopt; } SchemaDiff diff; From 3340367a38e06f09580b42a266b36c21cdffd62f Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Wed, 29 Jun 2022 10:17:45 +0800 Subject: [PATCH 04/14] fix build --- dbms/src/Debug/MockTiDB.cpp | 4 ++-- dbms/src/TiDB/Schema/TiDBSchemaSyncer.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dbms/src/Debug/MockTiDB.cpp b/dbms/src/Debug/MockTiDB.cpp index d3d8c8c8712..c0372911c5a 100644 --- a/dbms/src/Debug/MockTiDB.cpp +++ b/dbms/src/Debug/MockTiDB.cpp @@ -302,7 +302,7 @@ int MockTiDB::newTables( tables_by_id.emplace(table->table_info.id, table); tables_by_name.emplace(qualified_name, table); - AffectedOption opt; + AffectedOption opt{}; opt.schema_id = table->database_id; opt.table_id = table->id(); opt.old_schema_id = table->database_id; @@ -571,7 +571,7 @@ void MockTiDB::renameTables(const std::vectordatabase_id; opt.table_id = new_table->id(); opt.old_schema_id = table->database_id; diff --git a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h index 197006de97c..20dad1dfddd 100644 --- a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h +++ b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h @@ -149,7 +149,7 @@ struct TiDBSchemaSyncer : public SchemaSyncer { if (isTooOldSchema(cur_version, lastest_version)) { - return false; + return -1; } LOG_FMT_DEBUG(log, "try load schema diffs."); @@ -178,7 +178,7 @@ struct TiDBSchemaSyncer : public SchemaSyncer continue; } // else (used_version == lastest_version) - LOG_FMT_DEBUG(log, "End load a part of schema diffs, current version is {} ", used_version); + LOG_FMT_DEBUG(log, "End load a part of schema diffs, current version is {} ", used_version - 1); return used_version - 1; } From fd0fa6013c3a791615dd14fb7474e5640e2a43e9 Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Wed, 29 Jun 2022 13:46:25 +0800 Subject: [PATCH 05/14] support loadallschema --- dbms/src/Debug/MockSchemaGetter.h | 5 +++++ dbms/src/Debug/MockTiDB.cpp | 5 +++++ dbms/src/Debug/MockTiDB.h | 2 ++ dbms/src/TiDB/Schema/SchemaGetter.cpp | 7 +++++++ dbms/src/TiDB/Schema/SchemaGetter.h | 2 ++ dbms/src/TiDB/Schema/TiDBSchemaSyncer.h | 2 ++ 6 files changed, 23 insertions(+) diff --git a/dbms/src/Debug/MockSchemaGetter.h b/dbms/src/Debug/MockSchemaGetter.h index b4576d00cad..11c5d97f036 100644 --- a/dbms/src/Debug/MockSchemaGetter.h +++ b/dbms/src/Debug/MockSchemaGetter.h @@ -32,6 +32,11 @@ struct MockSchemaGetter return MockTiDB::instance().getSchemaDiff(version); } + bool checkSchemaDiffExists(Int64 version) + { + return MockTiDB::instance().checkSchemaDiffExists(version); + } + TiDB::TableInfoPtr getTableInfo(DatabaseID, TableID table_id) { return MockTiDB::instance().getTableInfoByID(table_id); } std::vector listDBs() diff --git a/dbms/src/Debug/MockTiDB.cpp b/dbms/src/Debug/MockTiDB.cpp index c0372911c5a..ed9a73c1e1f 100644 --- a/dbms/src/Debug/MockTiDB.cpp +++ b/dbms/src/Debug/MockTiDB.cpp @@ -674,4 +674,9 @@ std::optional MockTiDB::getSchemaDiff(Int64 version_) return version_diff[version_]; } +bool MockTiDB::checkSchemaDiffExists(Int64 version) +{ + return version_diff.find(version) != version_diff.end(); +} + } // namespace DB diff --git a/dbms/src/Debug/MockTiDB.h b/dbms/src/Debug/MockTiDB.h index b0ec9276113..261e547b13a 100644 --- a/dbms/src/Debug/MockTiDB.h +++ b/dbms/src/Debug/MockTiDB.h @@ -127,6 +127,8 @@ class MockTiDB : public ext::Singleton std::pair getDBIDByName(const String & database_name); + bool checkSchemaDiffExists(Int64 version); + std::optional getSchemaDiff(Int64 version); std::unordered_map getDatabases() { return databases; } diff --git a/dbms/src/TiDB/Schema/SchemaGetter.cpp b/dbms/src/TiDB/Schema/SchemaGetter.cpp index 3bf4969e646..6e333d6ba87 100644 --- a/dbms/src/TiDB/Schema/SchemaGetter.cpp +++ b/dbms/src/TiDB/Schema/SchemaGetter.cpp @@ -187,6 +187,13 @@ Int64 SchemaGetter::getVersion() return std::stoll(ver); } +bool SchemaGetter::checkSchemaDiffExists(Int64 ver) +{ + String key = getSchemaDiffKey(ver); + String data = TxnStructure::get(snap, key); + return !data.empty(); +} + String SchemaGetter::getSchemaDiffKey(Int64 ver) { return std::string(schemaDiffPrefix) + ":" + std::to_string(ver); diff --git a/dbms/src/TiDB/Schema/SchemaGetter.h b/dbms/src/TiDB/Schema/SchemaGetter.h index 075f8af4b26..fe0ecd59af0 100644 --- a/dbms/src/TiDB/Schema/SchemaGetter.h +++ b/dbms/src/TiDB/Schema/SchemaGetter.h @@ -140,6 +140,8 @@ struct SchemaGetter Int64 getVersion(); + bool checkSchemaDiffExists(Int64 ver); + std::optional getSchemaDiff(Int64 ver); static String getSchemaDiffKey(Int64 ver); diff --git a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h index 20dad1dfddd..e07c1a49d66 100644 --- a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h +++ b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h @@ -118,6 +118,8 @@ struct TiDBSchemaSyncer : public SchemaSyncer { GET_METRIC(tiflash_schema_apply_count, type_full).Increment(); loadAllSchema(getter, version, context); + // After loadAllSchema, we need update `version_after_load_diff` by last diff value exist or not + version_after_load_diff = getter.checkSchemaDiffExists(version) ? version : version - 1; } cur_version = version_after_load_diff; GET_METRIC(tiflash_schema_version).Set(cur_version); From fbf67ab51bdf1042845a3cfb786a5c2b6493558c Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Wed, 29 Jun 2022 14:29:40 +0800 Subject: [PATCH 06/14] address --- dbms/src/TiDB/Schema/TiDBSchemaSyncer.h | 114 +++++++++++++----------- 1 file changed, 63 insertions(+), 51 deletions(-) diff --git a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h index e07c1a49d66..844b58de2fc 100644 --- a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h +++ b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h @@ -147,78 +147,90 @@ struct TiDBSchemaSyncer : public SchemaSyncer return it->second; } - Int64 tryLoadSchemaDiffs(Getter & getter, Int64 lastest_version, Context & context) + // Return Values + // - if lastest schema diff is not empty, return the (latest_version) + // - if lastest schema diff is empty, return the (latest_version - 1) + // - if error happend, return (-1) + Int64 tryLoadSchemaDiffs(Getter & getter, Int64 latest_version, Context & context) { - if (isTooOldSchema(cur_version, lastest_version)) + if (isTooOldSchema(cur_version, latest_version)) { return -1; } LOG_FMT_DEBUG(log, "try load schema diffs."); - SchemaBuilder builder(getter, context, databases, lastest_version); + SchemaBuilder builder(getter, context, databases, latest_version); Int64 used_version = cur_version; - while (used_version < lastest_version) + std::vector> diffs; + while (used_version < latest_version) { used_version++; - auto schema_diff = getter.getSchemaDiff(used_version); - if (!schema_diff) - { - // If `schema diff` from `lastest_version` got empty `schema diff` - // Then we won't apply to `lastest_version`, but we will apply to `lastest_version - 1` - // If `schema diff` from [`cur_version`, `lastest_version - 1`] got empty `schema diff` - // Then we should just skip it. - // - // example: - // - `cur_version` is 1, `lastest_version` is 10 - // - The schema diff of schema version [2,4,6] is empty, Then we just skip it. - // - The schema diff of schema version 10 is empty, Then we should just apply version into 9 - if (used_version != lastest_version) - { - LOG_FMT_WARNING(log, "Skip the schema diff from version {}. ", used_version); - continue; - } // else (used_version == lastest_version) - - LOG_FMT_DEBUG(log, "End load a part of schema diffs, current version is {} ", used_version - 1); - return used_version - 1; - } + diffs.push_back(getter.getSchemaDiff(used_version)); + } + LOG_FMT_DEBUG(log, "end load schema diffs with total {} entries.", diffs.size()); - try - { - builder.applyDiff(*schema_diff); - } - catch (TiFlashException & e) - { - if (!e.getError().is(Errors::DDL::StaleSchema)) - { - GET_METRIC(tiflash_schema_apply_count, type_failed).Increment(); - } - LOG_FMT_WARNING(log, "apply diff meets exception : {} \n stack is {}", e.displayText(), e.getStackTrace().toString()); - return -1; - } - catch (Exception & e) + try + { + for (size_t diff_index = 0; diff_index < diffs.size(); diff_index++) { - if (e.code() == ErrorCodes::FAIL_POINT_ERROR) + auto schema_diff = diffs[diff_index]; + + if (!schema_diff) { - throw; + // If `schema diff` from `latest_version` got empty `schema diff` + // Then we won't apply to `latest_version`, but we will apply to `latest_version - 1` + // If `schema diff` from [`cur_version`, `latest_version - 1`] got empty `schema diff` + // Then we should just skip it. + // + // example: + // - `cur_version` is 1, `latest_version` is 10 + // - The schema diff of schema version [2,4,6] is empty, Then we just skip it. + // - The schema diff of schema version 10 is empty, Then we should just apply version into 9 + if (diff_index != diffs.size() - 1) + { + LOG_FMT_WARNING(log, "Skip the schema diff from version {}. ", used_version); + continue; + } // else (diff_index == diffs.size() - 1) + + LOG_FMT_DEBUG(log, "End load schema diffs without latest one, current version is {} ", used_version - 1); + return used_version - 1; } - GET_METRIC(tiflash_schema_apply_count, type_failed).Increment(); - LOG_FMT_WARNING(log, "apply diff meets exception : {} \n stack is {}", e.displayText(), e.getStackTrace().toString()); - return -1; + + builder.applyDiff(*schema_diff); } - catch (Poco::Exception & e) + } + catch (TiFlashException & e) + { + if (!e.getError().is(Errors::DDL::StaleSchema)) { GET_METRIC(tiflash_schema_apply_count, type_failed).Increment(); - LOG_FMT_WARNING(log, "apply diff meets exception : {}", e.displayText()); - return -1; } - catch (std::exception & e) + LOG_FMT_WARNING(log, "apply diff meets exception : {} \n stack is {}", e.displayText(), e.getStackTrace().toString()); + return -1; + } + catch (Exception & e) + { + if (e.code() == ErrorCodes::FAIL_POINT_ERROR) { - GET_METRIC(tiflash_schema_apply_count, type_failed).Increment(); - LOG_FMT_WARNING(log, "apply diff meets exception : {}", e.what()); - return -1; + throw; } + GET_METRIC(tiflash_schema_apply_count, type_failed).Increment(); + LOG_FMT_WARNING(log, "apply diff meets exception : {} \n stack is {}", e.displayText(), e.getStackTrace().toString()); + return -1; + } + catch (Poco::Exception & e) + { + GET_METRIC(tiflash_schema_apply_count, type_failed).Increment(); + LOG_FMT_WARNING(log, "apply diff meets exception : {}", e.displayText()); + return -1; + } + catch (std::exception & e) + { + GET_METRIC(tiflash_schema_apply_count, type_failed).Increment(); + LOG_FMT_WARNING(log, "apply diff meets exception : {}", e.what()); + return -1; } LOG_FMT_DEBUG(log, "End load all of schema diffs, current version is {} ", used_version); From 5ddcf333baecf288cb90d86017aa5706461b2c39 Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Thu, 30 Jun 2022 10:25:47 +0800 Subject: [PATCH 07/14] address --- dbms/src/TiDB/Schema/TiDBSchemaSyncer.h | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h index 844b58de2fc..2522a7d2d80 100644 --- a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h +++ b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h @@ -148,8 +148,8 @@ struct TiDBSchemaSyncer : public SchemaSyncer } // Return Values - // - if lastest schema diff is not empty, return the (latest_version) - // - if lastest schema diff is empty, return the (latest_version - 1) + // - if latest schema diff is not empty, return the (latest_version) + // - if latest schema diff is empty, return the (latest_version - 1) // - if error happend, return (-1) Int64 tryLoadSchemaDiffs(Getter & getter, Int64 latest_version, Context & context) { @@ -175,7 +175,7 @@ struct TiDBSchemaSyncer : public SchemaSyncer { for (size_t diff_index = 0; diff_index < diffs.size(); diff_index++) { - auto schema_diff = diffs[diff_index]; + const auto & schema_diff = diffs[diff_index]; if (!schema_diff) { @@ -190,7 +190,7 @@ struct TiDBSchemaSyncer : public SchemaSyncer // - The schema diff of schema version 10 is empty, Then we should just apply version into 9 if (diff_index != diffs.size() - 1) { - LOG_FMT_WARNING(log, "Skip the schema diff from version {}. ", used_version); + LOG_FMT_WARNING(log, "Skip the schema diff from version {}. ", cur_version + diff_index + 1); continue; } // else (diff_index == diffs.size() - 1) @@ -233,7 +233,6 @@ struct TiDBSchemaSyncer : public SchemaSyncer return -1; } - LOG_FMT_DEBUG(log, "End load all of schema diffs, current version is {} ", used_version); return used_version; } From ac39a1b256e0a82f3b28ce424ef317fe556ccf91 Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Thu, 30 Jun 2022 17:00:07 +0800 Subject: [PATCH 08/14] Update dbms/src/TiDB/Schema/TiDBSchemaSyncer.h Co-authored-by: JaySon --- dbms/src/TiDB/Schema/TiDBSchemaSyncer.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h index 2522a7d2d80..c36fbc1fc39 100644 --- a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h +++ b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h @@ -113,6 +113,13 @@ struct TiDBSchemaSyncer : public SchemaSyncer SCOPE_EXIT({ GET_METRIC(tiflash_schema_applying).Set(0.0); }); GET_METRIC(tiflash_schema_apply_count, type_diff).Increment(); + // After the feature concurrent DDL, TiDB does `update schema version` before `set schema diff`, and they are done in separate transactions. + // So TiFlash may see a schema version X but no schema diff X, meaning that the transaction of schema diff X has not been committed or has + // been aborted. + // However, TiDB makes sure that if we get a schema version X, then the schema diff X-1 must exist. Otherwise the transaction of schema diff + // X-1 is aborted and we can safely ignore it. + // Since TiDB can not make sure the schema diff of the latest schema version X is not empty, under this situation we should set the `cur_version` + // to X-1 and try to fetch the schema diff X next time. Int64 version_after_load_diff = 0; if (version_after_load_diff = tryLoadSchemaDiffs(getter, version, context); version_after_load_diff == -1) { From 3d4319bc0f63b564c090fc2fc32e70335f66f6cd Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Thu, 30 Jun 2022 17:02:32 +0800 Subject: [PATCH 09/14] address --- dbms/src/TiDB/Schema/TiDBSchemaSyncer.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h index c36fbc1fc39..4e9299a9899 100644 --- a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h +++ b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h @@ -130,7 +130,7 @@ struct TiDBSchemaSyncer : public SchemaSyncer } cur_version = version_after_load_diff; GET_METRIC(tiflash_schema_version).Set(cur_version); - LOG_FMT_INFO(log, "end sync schema, version has been updated to {}", cur_version); + LOG_FMT_INFO(log, "End sync schema, version has been updated to {}{}", cur_version, cur_version == version ? "" : "(latest diff is empty)"); return true; } @@ -201,7 +201,6 @@ struct TiDBSchemaSyncer : public SchemaSyncer continue; } // else (diff_index == diffs.size() - 1) - LOG_FMT_DEBUG(log, "End load schema diffs without latest one, current version is {} ", used_version - 1); return used_version - 1; } From 7f603a83730fe8f353c265fc10c1b2cdd9dbc617 Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Fri, 1 Jul 2022 13:15:27 +0800 Subject: [PATCH 10/14] address --- dbms/src/TiDB/Schema/TiDBSchemaSyncer.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h index 4e9299a9899..2d5bb565610 100644 --- a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h +++ b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h @@ -106,7 +106,7 @@ struct TiDBSchemaSyncer : public SchemaSyncer Stopwatch watch; SCOPE_EXIT({ GET_METRIC(tiflash_schema_apply_duration_seconds).Observe(watch.elapsedSeconds()); }); - LOG_FMT_INFO(log, "start to sync schemas. current version is: {} and try to sync schema version to: {}", cur_version, version); + LOG_FMT_INFO(log, "Start to sync schemas. current version is: {} and try to sync schema version to: {}", cur_version, version); // Show whether the schema mutex is held for a long time or not. GET_METRIC(tiflash_schema_applying).Set(1.0); @@ -165,7 +165,7 @@ struct TiDBSchemaSyncer : public SchemaSyncer return -1; } - LOG_FMT_DEBUG(log, "try load schema diffs."); + LOG_FMT_DEBUG(log, "Try load schema diffs."); SchemaBuilder builder(getter, context, databases, latest_version); @@ -176,7 +176,7 @@ struct TiDBSchemaSyncer : public SchemaSyncer used_version++; diffs.push_back(getter.getSchemaDiff(used_version)); } - LOG_FMT_DEBUG(log, "end load schema diffs with total {} entries.", diffs.size()); + LOG_FMT_DEBUG(log, "End load schema diffs with total {} entries.", diffs.size()); try { From 26629b5b32228c53488a8a7b607c7c82caab6116 Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Fri, 1 Jul 2022 13:15:52 +0800 Subject: [PATCH 11/14] Update dbms/src/TiDB/Schema/TiDBSchemaSyncer.h Co-authored-by: JaySon --- dbms/src/TiDB/Schema/TiDBSchemaSyncer.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h index 2d5bb565610..b90d0ac1afb 100644 --- a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h +++ b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h @@ -170,6 +170,8 @@ struct TiDBSchemaSyncer : public SchemaSyncer SchemaBuilder builder(getter, context, databases, latest_version); Int64 used_version = cur_version; + // First get all schema diff from `cur_version` to `latest_version`. Only apply the schema diff(s) if we fetch all + // schema diff without any exception. std::vector> diffs; while (used_version < latest_version) { From 5d3a442772241dc5812b3570521f84c97484308d Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Mon, 4 Jul 2022 10:38:57 +0800 Subject: [PATCH 12/14] Update dbms/src/TiDB/Schema/TiDBSchemaSyncer.h Co-authored-by: Lloyd-Pottiger <60744015+Lloyd-Pottiger@users.noreply.github.com> --- dbms/src/TiDB/Schema/TiDBSchemaSyncer.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h index b90d0ac1afb..cadf463268b 100644 --- a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h +++ b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h @@ -201,8 +201,9 @@ struct TiDBSchemaSyncer : public SchemaSyncer { LOG_FMT_WARNING(log, "Skip the schema diff from version {}. ", cur_version + diff_index + 1); continue; - } // else (diff_index == diffs.size() - 1) - + } + + // if diff_index == diffs.size() - 1, return used_version - 1; return used_version - 1; } From de5238379271bb0e1531c6644f438ce79df78adb Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Mon, 4 Jul 2022 11:11:21 +0800 Subject: [PATCH 13/14] format --- dbms/src/TiDB/Schema/TiDBSchemaSyncer.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h index cadf463268b..2b30dda7742 100644 --- a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h +++ b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h @@ -202,7 +202,7 @@ struct TiDBSchemaSyncer : public SchemaSyncer LOG_FMT_WARNING(log, "Skip the schema diff from version {}. ", cur_version + diff_index + 1); continue; } - + // if diff_index == diffs.size() - 1, return used_version - 1; return used_version - 1; } From e73782385cc81a71129724c4e4632d956a3b9d6b Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Mon, 4 Jul 2022 11:58:49 +0800 Subject: [PATCH 14/14] Update dbms/src/TiDB/Schema/TiDBSchemaSyncer.h Co-authored-by: Lloyd-Pottiger <60744015+Lloyd-Pottiger@users.noreply.github.com> --- dbms/src/TiDB/Schema/TiDBSchemaSyncer.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h index 2b30dda7742..a23aeab139f 100644 --- a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h +++ b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h @@ -182,7 +182,7 @@ struct TiDBSchemaSyncer : public SchemaSyncer try { - for (size_t diff_index = 0; diff_index < diffs.size(); diff_index++) + for (size_t diff_index = 0; diff_index < diffs.size(); ++diff_index) { const auto & schema_diff = diffs[diff_index];