From 26b6dd7925d3725f401abd093197c651cc148967 Mon Sep 17 00:00:00 2001
From: zanmato1984 <zanmato1984@gmail.com>
Date: Tue, 16 Jul 2019 17:20:42 +0800
Subject: [PATCH 01/10] Add sync schema on read

---
 dbms/src/Debug/MockSchemaSyncer.h             |  2 +-
 .../Interpreters/InterpreterSelectQuery.cpp   | 57 +++++++++++++------
 .../src/Interpreters/InterpreterSelectQuery.h |  2 +-
 3 files changed, 43 insertions(+), 18 deletions(-)

diff --git a/dbms/src/Debug/MockSchemaSyncer.h b/dbms/src/Debug/MockSchemaSyncer.h
index 5c2dd58b592..52f7d22ac5a 100644
--- a/dbms/src/Debug/MockSchemaSyncer.h
+++ b/dbms/src/Debug/MockSchemaSyncer.h
@@ -12,7 +12,7 @@ class MockSchemaSyncer : public SchemaSyncer
 
     bool syncSchemas(Context & context) override;
 
-    void syncSchema(Context & context, TableID table_id, bool) override;
+    void syncSchema(Context & context, TableID table_id, bool lock) override;
 
     TableID getTableIdByName(const std::string & database_name, const std::string & table_name)
     {
diff --git a/dbms/src/Interpreters/InterpreterSelectQuery.cpp b/dbms/src/Interpreters/InterpreterSelectQuery.cpp
index c312e00422a..2446554e3df 100644
--- a/dbms/src/Interpreters/InterpreterSelectQuery.cpp
+++ b/dbms/src/Interpreters/InterpreterSelectQuery.cpp
@@ -78,6 +78,7 @@ namespace ErrorCodes
     extern const int LOGICAL_ERROR;
     extern const int NOT_IMPLEMENTED;
     extern const int SCHEMA_VERSION_ERROR;
+    extern const int UNKNOWN_EXCEPTION;
 }
 
 InterpreterSelectQuery::InterpreterSelectQuery(
@@ -162,10 +163,7 @@ void InterpreterSelectQuery::init(const Names & required_result_column_names)
     }
 
     if (storage)
-        table_lock = storage->lockStructure(false, __PRETTY_FUNCTION__);
-
-    /// Make sure TMT storage schema qualifies the version specified by upper (TiDB or TiSpark).
-    alignStorageSchema(settings.schema_version);
+        table_lock = alignStorageSchemaAndLock(settings.schema_version);
 
     query_analyzer = std::make_unique<ExpressionAnalyzer>(
         query_ptr, context, storage, source_columns, required_result_column_names, subquery_depth, !only_analyze);
@@ -189,24 +187,51 @@ void InterpreterSelectQuery::init(const Names & required_result_column_names)
 }
 
 
-void InterpreterSelectQuery::alignStorageSchema(Int64 schema_version)
+TableStructureReadLockPtr InterpreterSelectQuery::alignStorageSchemaAndLock(Int64 schema_version)
 {
-    if (schema_version == DEFAULT_SCHEMA_VERSION || !storage)
-        return;
-
+    /// Regular read lock for non-TMT or DEFAULT_SCHEMA_VERSION specified.
     const auto merge_tree = dynamic_cast<const StorageMergeTree *>(storage.get());
-    if (!merge_tree || merge_tree->getData().merging_params.mode != MergeTreeData::MergingParams::Txn)
-        return;
+    if (schema_version == DEFAULT_SCHEMA_VERSION || !merge_tree || merge_tree->getData().merging_params.mode != MergeTreeData::MergingParams::Txn)
+        return storage->lockStructure(false, __PRETTY_FUNCTION__);
+
+    /// Lambda for schema version check under the read lock.
+    auto checkSchemaVersionAndLock = [&](bool schema_synced) -> std::tuple<TableStructureReadLockPtr, Int64> {
+        auto lock = storage->lockStructure(false, __PRETTY_FUNCTION__);
+
+        auto storage_schema_version = merge_tree->getTableInfo().schema_version;
+        if (storage_schema_version > schema_version)
+            throw Exception("Storage schema version " + std::to_string(storage_schema_version) + " newer than query schema version " + std::to_string(schema_version), ErrorCodes::SCHEMA_VERSION_ERROR);
+
+        if ((schema_synced && storage_schema_version <= schema_version) || (!schema_synced && storage_schema_version == schema_version))
+            return std::make_tuple(lock, storage_schema_version);
+
+        return std::make_tuple(nullptr, storage_schema_version);
+    };
 
-    auto storage_schema_version = merge_tree->getTableInfo().schema_version;
-    if (storage_schema_version < schema_version)
+    /// Try check and lock once.
     {
-        LOG_TRACE(log, __PRETTY_FUNCTION__ << " storage schema version: " << storage_schema_version << ", query schema version: " << schema_version << ", syncing schema.");
-        context.getTMTContext().getSchemaSyncer()->syncSchema(context, merge_tree->getTableInfo().id, false);
+        auto [lock, storage_schema_version] = checkSchemaVersionAndLock(false);
+        if (lock)
+        {
+            LOG_DEBUG(log, __PRETTY_FUNCTION__ << " storage schema version: " << storage_schema_version << ", query schema version: " << schema_version << ", schema check OK, no syncing required.");
+            return lock;
+        }
+        LOG_DEBUG(log, __PRETTY_FUNCTION__ << " storage schema version: " << storage_schema_version << ", query schema version: " << schema_version << ", schema check not OK.");
     }
 
-    if (storage_schema_version > schema_version)
-        throw Exception("Storage schema version " + std::to_string(storage_schema_version) + " newer than query schema version " + std::to_string(schema_version), ErrorCodes::SCHEMA_VERSION_ERROR);
+    /// If first try failed, sync schema and check again.
+    {
+        context.getTMTContext().getSchemaSyncer()->syncSchemas(context);
+
+        auto [lock, storage_schema_version] = checkSchemaVersionAndLock(true);
+        if (lock)
+        {
+            LOG_DEBUG(log, __PRETTY_FUNCTION__ << " storage schema version: " << storage_schema_version << ", query schema version: " << schema_version << ", schema check OK after syncing.");
+            return lock;
+        }
+
+        throw Exception("Shouldn't reach here", ErrorCodes::UNKNOWN_EXCEPTION);
+    }
 }
 
 
diff --git a/dbms/src/Interpreters/InterpreterSelectQuery.h b/dbms/src/Interpreters/InterpreterSelectQuery.h
index 24ca7ce350a..c0b007690d0 100644
--- a/dbms/src/Interpreters/InterpreterSelectQuery.h
+++ b/dbms/src/Interpreters/InterpreterSelectQuery.h
@@ -111,7 +111,7 @@ class InterpreterSelectQuery : public IInterpreter
 
     void init(const Names & required_result_column_names);
 
-    void alignStorageSchema(Int64 schema_version);
+    TableStructureReadLockPtr alignStorageSchemaAndLock(Int64 schema_version);
 
     void executeImpl(Pipeline & pipeline, const BlockInputStreamPtr & input, bool dry_run);
 

From d1ac4f42143680cbba7376bb9f03a0b57d6095b7 Mon Sep 17 00:00:00 2001
From: zanmato1984 <zanmato1984@gmail.com>
Date: Tue, 16 Jul 2019 19:52:35 +0800
Subject: [PATCH 02/10] Simplify schema syncer interface and adjust mock stuff

---
 dbms/src/Debug/DBGInvoker.cpp                 |  3 +-
 dbms/src/Debug/MockSchemaSyncer.cpp           | 55 ++++++-----
 dbms/src/Debug/MockSchemaSyncer.h             | 13 +--
 dbms/src/Debug/MockTiDB.cpp                   | 34 ++-----
 dbms/src/Debug/MockTiDB.h                     |  6 +-
 dbms/src/Debug/dbgFuncSchema.cpp              | 92 +++++--------------
 dbms/src/Debug/dbgFuncSchema.h                | 11 ++-
 dbms/src/Interpreters/Context.cpp             |  4 +-
 dbms/src/Interpreters/Context.h               |  2 +-
 .../Storages/Transaction/SchemaSyncService.h  |  2 +
 dbms/src/Storages/Transaction/SchemaSyncer.h  |  9 --
 .../Storages/Transaction/TiDBSchemaSyncer.h   |  2 -
 12 files changed, 78 insertions(+), 155 deletions(-)

diff --git a/dbms/src/Debug/DBGInvoker.cpp b/dbms/src/Debug/DBGInvoker.cpp
index 0f7b97c11bb..ce80ba28ab1 100644
--- a/dbms/src/Debug/DBGInvoker.cpp
+++ b/dbms/src/Debug/DBGInvoker.cpp
@@ -59,8 +59,9 @@ DBGInvoker::DBGInvoker()
 
     regFunc("dump_all_region", dbgFuncDumpAllRegion);
 
+    regFunc("enable_schema_sync_service", dbgFuncEnableSchemaSyncService);
     regFunc("mock_schema_syncer", dbgFuncMockSchemaSyncer);
-    regFunc("refresh_schema", dbgFuncRefreshSchema);
+    regFunc("refresh_schemas", dbgFuncRefreshSchemas);
 }
 
 void replaceSubstr(std::string & str, const std::string & target, const std::string & replacement)
diff --git a/dbms/src/Debug/MockSchemaSyncer.cpp b/dbms/src/Debug/MockSchemaSyncer.cpp
index df41fd98270..116d6295127 100644
--- a/dbms/src/Debug/MockSchemaSyncer.cpp
+++ b/dbms/src/Debug/MockSchemaSyncer.cpp
@@ -235,31 +235,36 @@ AlterCommands detectSchemaChanges(const TableInfo & table_info, const TableInfo
 
 MockSchemaSyncer::MockSchemaSyncer() : log(&Logger::get("MockSchemaSyncer")) {}
 
-bool MockSchemaSyncer::syncSchemas(Context & /*context*/)
+bool MockSchemaSyncer::syncSchemas(Context & context)
 {
-    // Don't do full schema sync, we want to test schema sync timing in a fine-grained fashion.
-    return false;
+    std::unordered_map<TableID, MockTiDB::TablePtr> new_tables;
+    MockTiDB::instance().traverseTables([&](const auto & table) { new_tables.emplace(table->id(), table); });
+
+    for (auto [id, table] : tables)
+    {
+        if (new_tables.find(id) == new_tables.end())
+            dropTable(table->table_info.db_name, table->table_info.name, context);
+    }
+
+    for (auto [id, table] : new_tables)
+    {
+        std::ignore = id;
+        syncTable(context, table);
+    }
+
+    tables.swap(new_tables);
+
+    return true;
 }
 
-void MockSchemaSyncer::syncSchema(Context & context, TableID table_id, bool /*lock*/)
+void MockSchemaSyncer::syncTable(Context & context, MockTiDB::TablePtr table)
 {
     auto & tmt_context = context.getTMTContext();
 
-    /// Get table schema json from TiDB/TiKV.
-    String table_info_json = getSchemaJson(table_id, context);
-    if (table_info_json.empty())
-    {
-        /// Table dropped.
-        auto storage = tmt_context.getStorages().get(table_id);
-        if (storage == nullptr)
-        {
-            LOG_DEBUG(log, __PRETTY_FUNCTION__ << ": Table " << table_id << "doesn't exist in TiDB and doesn't exist in TMT, do nothing.");
-            return;
-        }
-        LOG_DEBUG(log, __PRETTY_FUNCTION__ << ": Table " << table_id << "doesn't exist in TiDB, dropping.");
-        dropTable(storage->getDatabaseName(), storage->getTableName(), context);
-        return;
-    }
+    /// Get table schema json.
+    auto table_id = table->id();
+
+    String table_info_json = table->table_info.serialize(false);
 
     LOG_DEBUG(log, __PRETTY_FUNCTION__ << ": Table " << table_id << " info json: " << table_info_json);
 
@@ -326,8 +331,7 @@ void MockSchemaSyncer::syncSchema(Context & context, TableID table_id, bool /*lo
     }
 
     /// Table existing, detect schema changes and apply.
-    auto merge_tree = std::dynamic_pointer_cast<StorageMergeTree>(storage);
-    const TableInfo & orig_table_info = merge_tree->getTableInfo();
+    const TableInfo & orig_table_info = storage->getTableInfo();
     AlterCommands alter_commands = detectSchemaChanges(table_info, orig_table_info);
 
     std::stringstream ss;
@@ -345,15 +349,8 @@ void MockSchemaSyncer::syncSchema(Context & context, TableID table_id, bool /*lo
 
     LOG_DEBUG(log, __PRETTY_FUNCTION__ << ": " << ss.str());
 
-    {
-        // Change internal TableInfo in TMT first.
-        // TODO: Ideally this should be done within alter function, however we are limited by the narrow alter interface, thus not truly atomic.
-        auto table_hard_lock = storage->lockStructureForAlter(__PRETTY_FUNCTION__);
-        merge_tree->setTableInfo(table_info);
-    }
-
     // Call storage alter to apply schema changes.
-    storage->alter(alter_commands, table_info.db_name, table_info.name, context);
+    storage->alterForTMT(alter_commands, table_info, context);
 
     LOG_DEBUG(log, __PRETTY_FUNCTION__ << ": Schema changes apply done.");
 
diff --git a/dbms/src/Debug/MockSchemaSyncer.h b/dbms/src/Debug/MockSchemaSyncer.h
index 52f7d22ac5a..4139f02c425 100644
--- a/dbms/src/Debug/MockSchemaSyncer.h
+++ b/dbms/src/Debug/MockSchemaSyncer.h
@@ -2,6 +2,8 @@
 
 #include <Debug/MockTiDB.h>
 
+#include <unordered_map>
+
 namespace DB
 {
 
@@ -12,17 +14,12 @@ class MockSchemaSyncer : public SchemaSyncer
 
     bool syncSchemas(Context & context) override;
 
-    void syncSchema(Context & context, TableID table_id, bool lock) override;
-
-    TableID getTableIdByName(const std::string & database_name, const std::string & table_name)
-    {
-        return MockTiDB::instance().getTableIDByName(database_name, table_name);
-    }
-
 protected:
-    String getSchemaJson(TableID table_id, Context & /*context*/) { return MockTiDB::instance().getSchemaJson(table_id); }
+    void syncTable(Context & context, MockTiDB::TablePtr table);
 
     Logger * log;
+
+    std::unordered_map<TableID, MockTiDB::TablePtr> tables;
 };
 
 } // namespace DB
diff --git a/dbms/src/Debug/MockTiDB.cpp b/dbms/src/Debug/MockTiDB.cpp
index 9b937fc1fdc..5ec28ba2598 100644
--- a/dbms/src/Debug/MockTiDB.cpp
+++ b/dbms/src/Debug/MockTiDB.cpp
@@ -26,33 +26,6 @@ Table::Table(const String & database_name_, const String & table_name_, TableInf
     : table_info(std::move(table_info_)), database_name(database_name_), table_name(table_name_)
 {}
 
-String MockTiDB::getSchemaJson(TableID table_id)
-{
-    std::lock_guard lock(tables_mutex);
-
-    auto it = tables_by_id.find(table_id);
-    if (it == tables_by_id.end())
-    {
-        return "";
-    }
-
-    return it->second->table_info.serialize(false);
-}
-
-TableID MockTiDB::getTableIDByName(const std::string & database_name, const std::string & table_name)
-{
-    std::lock_guard lock(tables_mutex);
-
-    String qualified_name = database_name + "." + table_name;
-    auto it = tables_by_name.find(qualified_name);
-    if (it == tables_by_name.end())
-    {
-        return InvalidTableID;
-    }
-
-    return it->second->table_info.id;
-}
-
 void MockTiDB::dropTable(const String & database_name, const String & table_name)
 {
     std::lock_guard lock(tables_mutex);
@@ -251,6 +224,13 @@ TablePtr MockTiDB::getTableByName(const String & database_name, const String & t
     return getTableByNameInternal(database_name, table_name);
 }
 
+void MockTiDB::traverseTables(std::function<void(TablePtr)> f)
+{
+    std::lock_guard lock(tables_mutex);
+
+    std::for_each(tables_by_id.begin(), tables_by_id.end(), [&](const auto & pair) { f(pair.second); });
+}
+
 TablePtr MockTiDB::getTableByNameInternal(const String & database_name, const String & table_name)
 {
     String qualified_name = database_name + "." + table_name;
diff --git a/dbms/src/Debug/MockTiDB.h b/dbms/src/Debug/MockTiDB.h
index 50bd28d7a15..5adc0dce5eb 100644
--- a/dbms/src/Debug/MockTiDB.h
+++ b/dbms/src/Debug/MockTiDB.h
@@ -62,10 +62,6 @@ class MockTiDB : public ext::singleton<MockTiDB>
     using TablePtr = std::shared_ptr<Table>;
 
 public:
-    String getSchemaJson(TableID table_id);
-
-    TableID getTableIDByName(const std::string & database_name, const std::string & table_name);
-
     TableID newTable(const String & database_name, const String & table_name, const ColumnsDescription & columns);
 
     TableID newPartition(const String & database_name, const String & table_name, const String & partition_name);
@@ -80,6 +76,8 @@ class MockTiDB : public ext::singleton<MockTiDB>
 
     TablePtr getTableByName(const String & database_name, const String & table_name);
 
+    void traverseTables(std::function<void(TablePtr)> f);
+
 private:
     TablePtr getTableByNameInternal(const String & database_name, const String & table_name);
 
diff --git a/dbms/src/Debug/dbgFuncSchema.cpp b/dbms/src/Debug/dbgFuncSchema.cpp
index 731ac90899c..dc45d067e76 100644
--- a/dbms/src/Debug/dbgFuncSchema.cpp
+++ b/dbms/src/Debug/dbgFuncSchema.cpp
@@ -3,6 +3,7 @@
 #include <Parsers/ASTIdentifier.h>
 #include <Parsers/ASTLiteral.h>
 #include <Storages/StorageMergeTree.h>
+#include <Storages/Transaction/SchemaSyncService.h>
 #include <Storages/Transaction/SchemaSyncer.h>
 #include <Storages/Transaction/TMTContext.h>
 #include <Storages/Transaction/TiDB.h>
@@ -15,6 +16,23 @@ namespace ErrorCodes
 extern const int UNKNOWN_TABLE;
 } // namespace ErrorCodes
 
+void dbgFuncEnableSchemaSyncService(Context & context, const ASTs & args, DBGInvoker::Printer output)
+{
+    if (args.size() != 1)
+        throw Exception("Args not matched, should be: enable (true/false)", ErrorCodes::BAD_ARGUMENTS);
+
+    bool enable = safeGet<String>(typeid_cast<const ASTLiteral &>(*args[0]).value) == "true";
+
+    if (enable)
+        context.initializeSchemaSyncService();
+    else
+        context.getSchemaSyncService().reset();
+
+    std::stringstream ss;
+    ss << "schema sync service " << (enable ? "enabled" : "disabled");
+    output(ss.str());
+}
+
 void dbgFuncMockSchemaSyncer(Context & context, const ASTs & args, DBGInvoker::Printer output)
 {
     if (args.size() != 1)
@@ -39,79 +57,15 @@ void dbgFuncMockSchemaSyncer(Context & context, const ASTs & args, DBGInvoker::P
     output(ss.str());
 }
 
-void dbgFuncRefreshSchema(Context & context, const ASTs & args, DBGInvoker::Printer output)
+void dbgFuncRefreshSchemas(Context & context, const ASTs &, DBGInvoker::Printer output)
 {
-    if (args.size() != 2)
-        throw Exception("Args not matched, should be: database-name, table-name", ErrorCodes::BAD_ARGUMENTS);
-
-    std::string database_name = typeid_cast<const ASTIdentifier &>(*args[0]).name;
-    std::transform(database_name.begin(), database_name.end(), database_name.begin(), ::tolower);
-    std::string table_name = typeid_cast<const ASTIdentifier &>(*args[1]).name;
-    std::transform(table_name.begin(), table_name.end(), table_name.begin(), ::tolower);
-
-    auto log = [&](TableID table_id) {
-        std::stringstream ss;
-        ss << "refreshed schema for table #" << table_id;
-        output(ss.str());
-    };
-
     TMTContext & tmt = context.getTMTContext();
     auto schema_syncer = tmt.getSchemaSyncer();
-    auto mock_schema_syncer = std::dynamic_pointer_cast<MockSchemaSyncer>(schema_syncer);
-    if (!mock_schema_syncer)
-        throw Exception("Debug function refresh_schema can only be used under mock schema syncer.");
-
-    TableID table_id = mock_schema_syncer->getTableIdByName(database_name, table_name);
-    auto storage = tmt.getStorages().getByName(database_name, table_name);
-
-    if (storage == nullptr && table_id == InvalidTableID)
-        // Table does not exist in CH nor TiDB, error out.
-        throw Exception("Table " + database_name + "." + table_name + " doesn't exist in tidb", ErrorCodes::UNKNOWN_TABLE);
-
-    if (storage == nullptr && table_id != InvalidTableID)
-    {
-        // Table does not exist in CH, but exists in TiDB.
-        // Might be renamed or never synced.
-        // Note there will be a dangling table in CH for the following scenario:
-        // Table t was synced to CH already, then t was renamed (name changed) and truncated (ID changed).
-        // Then this function was called with the new name given, the table will be synced to a new table.
-        // User must manually call this function with the old name to remove the dangling table in CH.
-        mock_schema_syncer->syncSchema(context, table_id, true);
-
-        log(table_id);
-
-        return;
-    }
-
-    if (table_id == InvalidTableID)
-    {
-        // Table exists in CH, but does not exist in TiDB.
-        // Just sync it using the storage's ID, syncer will then remove it.
-        mock_schema_syncer->syncSchema(context, storage->getTableInfo().id, true);
-
-        log(table_id);
-
-        return;
-    }
+    schema_syncer->syncSchemas(context);
 
-    // Table exists in both CH and TiDB.
-    if (table_id != storage->getTableInfo().id)
-    {
-        // Table in TiDB is not the old one, i.e. dropped/renamed then recreated.
-        // Sync the old one in CH first, then sync the new one.
-        mock_schema_syncer->syncSchema(context, storage->getTableInfo().id, true);
-        mock_schema_syncer->syncSchema(context, table_id, true);
-
-        log(table_id);
-
-        return;
-    }
-
-    // Table in TiDB is the same one as in CH.
-    // Just sync it.
-    mock_schema_syncer->syncSchema(context, table_id, true);
-
-    log(table_id);
+    std::stringstream ss;
+    ss << "schemas refreshed";
+    output(ss.str());
 }
 
 } // namespace DB
diff --git a/dbms/src/Debug/dbgFuncSchema.h b/dbms/src/Debug/dbgFuncSchema.h
index 10c56bcf570..9e15ad88ef3 100644
--- a/dbms/src/Debug/dbgFuncSchema.h
+++ b/dbms/src/Debug/dbgFuncSchema.h
@@ -8,14 +8,19 @@ namespace DB
 
 class Context;
 
+// Enable/disable schema sync service.
+// Usage:
+//   ./storages-client.sh "DBGInvoke enable_schema_sync_service(enable)"
+void dbgFuncEnableSchemaSyncService(Context & context, const ASTs & args, DBGInvoker::Printer output);
+
 // Change whether to mock schema syncer.
 // Usage:
 //   ./storages-client.sh "DBGInvoke mock_schema_syncer(enabled)"
 void dbgFuncMockSchemaSyncer(Context & context, const ASTs & args, DBGInvoker::Printer output);
 
-// Refresh schema of the given table.
+// Refresh schemas for all tables.
 // Usage:
-//   ./storage-client.sh "DBGInvoke refresh_schema(database_name, table_name)"
-void dbgFuncRefreshSchema(Context & context, const ASTs & args, DBGInvoker::Printer output);
+//   ./storage-client.sh "DBGInvoke refresh_schemas()"
+void dbgFuncRefreshSchemas(Context & context, const ASTs & args, DBGInvoker::Printer output);
 
 } // namespace DB
diff --git a/dbms/src/Interpreters/Context.cpp b/dbms/src/Interpreters/Context.cpp
index d4ed73d947e..141d611e7bf 100644
--- a/dbms/src/Interpreters/Context.cpp
+++ b/dbms/src/Interpreters/Context.cpp
@@ -1450,12 +1450,12 @@ void Context::initializeSchemaSyncService()
     shared->schema_sync_service = std::make_shared<SchemaSyncService>(*this);
 }
 
-SchemaSyncService & Context::getSchemaSyncService()
+SchemaSyncServicePtr Context::getSchemaSyncService()
 {
     auto lock = getLock();
     if (!shared->schema_sync_service)
         throw Exception("Schema Sync Service is not initialized.", ErrorCodes::LOGICAL_ERROR);
-    return *shared->schema_sync_service;
+    return shared->schema_sync_service;
 }
 
 zkutil::ZooKeeperPtr Context::getZooKeeper() const
diff --git a/dbms/src/Interpreters/Context.h b/dbms/src/Interpreters/Context.h
index c4c53b43d41..4b7402c1a6e 100644
--- a/dbms/src/Interpreters/Context.h
+++ b/dbms/src/Interpreters/Context.h
@@ -369,7 +369,7 @@ class Context
     TiDBService & getTiDBService();
 
     void initializeSchemaSyncService();
-    SchemaSyncService & getSchemaSyncService();
+    SchemaSyncServicePtr getSchemaSyncService();
 
     Clusters & getClusters() const;
     std::shared_ptr<Cluster> getCluster(const std::string & cluster_name) const;
diff --git a/dbms/src/Storages/Transaction/SchemaSyncService.h b/dbms/src/Storages/Transaction/SchemaSyncService.h
index a846f0bf6c5..6a731a98cfb 100644
--- a/dbms/src/Storages/Transaction/SchemaSyncService.h
+++ b/dbms/src/Storages/Transaction/SchemaSyncService.h
@@ -28,4 +28,6 @@ class SchemaSyncService : public std::enable_shared_from_this<SchemaSyncService>
     Logger * log;
 };
 
+using SchemaSyncServicePtr = std::shared_ptr<SchemaSyncService>;
+
 } // namespace DB
diff --git a/dbms/src/Storages/Transaction/SchemaSyncer.h b/dbms/src/Storages/Transaction/SchemaSyncer.h
index 892a5a85839..461c5813853 100644
--- a/dbms/src/Storages/Transaction/SchemaSyncer.h
+++ b/dbms/src/Storages/Transaction/SchemaSyncer.h
@@ -20,15 +20,6 @@ class SchemaSyncer
      * @param context
      */
     virtual bool syncSchemas(Context & context) = 0;
-
-    /**
-     * Synchronize schema between TiDB and CH, to make sure the CH table is new enough to accept data from raft.
-     * Should be stateless.
-     * Nevertheless, the implementations may assume that the storage is appropriately locked, thus still not thread-safe.
-     * @param context
-     * @param table_id
-     */
-    virtual void syncSchema(Context & context, TableID table_id, bool lock) = 0;
 };
 
 using SchemaSyncerPtr = std::shared_ptr<SchemaSyncer>;
diff --git a/dbms/src/Storages/Transaction/TiDBSchemaSyncer.h b/dbms/src/Storages/Transaction/TiDBSchemaSyncer.h
index 79a663b9683..cf6fbb1304c 100644
--- a/dbms/src/Storages/Transaction/TiDBSchemaSyncer.h
+++ b/dbms/src/Storages/Transaction/TiDBSchemaSyncer.h
@@ -52,8 +52,6 @@ struct TiDBSchemaSyncer : public SchemaSyncer
         return true;
     }
 
-    void syncSchema(Context & context, TableID, bool) override { syncSchemas(context); }
-
     bool tryLoadSchemaDiffs(SchemaGetter & getter, Int64 version, Context & context)
     {
         if (isTooOldSchema(cur_version, version))

From d3e2298dd8b98e054545f4fcafcf7aa712cc33b0 Mon Sep 17 00:00:00 2001
From: zanmato1984 <zanmato1984@gmail.com>
Date: Thu, 18 Jul 2019 13:08:55 +0800
Subject: [PATCH 03/10] Rename default schema version setting

---
 dbms/src/Core/Defines.h          | 2 +-
 dbms/src/Interpreters/Settings.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/dbms/src/Core/Defines.h b/dbms/src/Core/Defines.h
index 48fe68de6b3..85eb7d499d1 100644
--- a/dbms/src/Core/Defines.h
+++ b/dbms/src/Core/Defines.h
@@ -27,7 +27,7 @@
 #define DEFAULT_MAX_COMPRESS_BLOCK_SIZE 1048576
 
 #define DEFAULT_MAX_READ_TSO 0xFFFFFFFFFFFFFFFF
-#define DEFAULT_SCHEMA_VERSION -1
+#define DEFAULT_UNSPECIFIED_SCHEMA_VERSION -1
 
 /** Which blocks by default read the data (by number of rows).
   * Smaller values give better cache locality, less consumption of RAM, but more overhead to process the query.
diff --git a/dbms/src/Interpreters/Settings.h b/dbms/src/Interpreters/Settings.h
index b6077e0879e..c67999e6916 100644
--- a/dbms/src/Interpreters/Settings.h
+++ b/dbms/src/Interpreters/Settings.h
@@ -29,7 +29,7 @@ struct Settings
     M(SettingString, regions, "", "the region need to be read.") \
     M(SettingBool, resolve_locks, false, "tmt read tso.") \
     M(SettingUInt64, read_tso, DEFAULT_MAX_READ_TSO, "tmt read tso.") \
-    M(SettingInt64, schema_version, DEFAULT_SCHEMA_VERSION, "tmt schema version.") \
+    M(SettingInt64, schema_version, DEFAULT_UNSPECIFIED_SCHEMA_VERSION, "tmt schema version.") \
     M(SettingUInt64, min_compress_block_size, DEFAULT_MIN_COMPRESS_BLOCK_SIZE, "The actual size of the block to compress, if the uncompressed data less than max_compress_block_size is no less than this value and no less than the volume of data for one mark.") \
     M(SettingUInt64, max_compress_block_size, DEFAULT_MAX_COMPRESS_BLOCK_SIZE, "The maximum size of blocks of uncompressed data before compressing for writing to a table.") \
     M(SettingUInt64, max_block_size, DEFAULT_BLOCK_SIZE, "Maximum block size for reading") \

From e690046f1ba4ad7c2f457c061193c7a08f8bfcf8 Mon Sep 17 00:00:00 2001
From: zanmato1984 <zanmato1984@gmail.com>
Date: Thu, 18 Jul 2019 13:15:45 +0800
Subject: [PATCH 04/10] Compensate last commit

---
 dbms/src/Interpreters/InterpreterSelectQuery.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/dbms/src/Interpreters/InterpreterSelectQuery.cpp b/dbms/src/Interpreters/InterpreterSelectQuery.cpp
index 2446554e3df..b63950e5887 100644
--- a/dbms/src/Interpreters/InterpreterSelectQuery.cpp
+++ b/dbms/src/Interpreters/InterpreterSelectQuery.cpp
@@ -189,9 +189,9 @@ void InterpreterSelectQuery::init(const Names & required_result_column_names)
 
 TableStructureReadLockPtr InterpreterSelectQuery::alignStorageSchemaAndLock(Int64 schema_version)
 {
-    /// Regular read lock for non-TMT or DEFAULT_SCHEMA_VERSION specified.
+    /// Regular read lock for non-TMT or schema version unspecified.
     const auto merge_tree = dynamic_cast<const StorageMergeTree *>(storage.get());
-    if (schema_version == DEFAULT_SCHEMA_VERSION || !merge_tree || merge_tree->getData().merging_params.mode != MergeTreeData::MergingParams::Txn)
+    if (schema_version == DEFAULT_UNSPECIFIED_SCHEMA_VERSION || !merge_tree || merge_tree->getData().merging_params.mode != MergeTreeData::MergingParams::Txn)
         return storage->lockStructure(false, __PRETTY_FUNCTION__);
 
     /// Lambda for schema version check under the read lock.

From 49b3fdf31db1192ea688b84a5d45e28855eb4e8a Mon Sep 17 00:00:00 2001
From: zanmato1984 <zanmato1984@gmail.com>
Date: Thu, 18 Jul 2019 19:08:01 +0800
Subject: [PATCH 05/10] Remove curl library

---
 CMakeLists.txt        | 1 -
 cmake/find_curl.cmake | 7 -------
 2 files changed, 8 deletions(-)
 delete mode 100644 cmake/find_curl.cmake

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 5628975d5d6..49971669148 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -265,7 +265,6 @@ include (cmake/find_capnp.cmake)
 include (cmake/find_llvm.cmake)
 include (cmake/find_grpc.cmake)
 include (cmake/find_kvproto.cmake)
-include (cmake/find_curl.cmake)
 
 
 include (cmake/find_contrib_lib.cmake)
diff --git a/cmake/find_curl.cmake b/cmake/find_curl.cmake
deleted file mode 100644
index 74217cbc7cb..00000000000
--- a/cmake/find_curl.cmake
+++ /dev/null
@@ -1,7 +0,0 @@
-find_package (CURL REQUIRED)
-
-if (NOT CURL_FOUND)
-  message (FATAL_ERROR "Curl Not Found!")
-endif (NOT CURL_FOUND)
-
-message (STATUS "Using CURL: ${CURL_INCLUDE_DIRS} : ${CURL_LIBRARIES}")

From b5c2a855707d087c79a5b818b3629343b98aaa7d Mon Sep 17 00:00:00 2001
From: zanmato1984 <zanmato1984@gmail.com>
Date: Thu, 18 Jul 2019 19:31:11 +0800
Subject: [PATCH 06/10] Remove curl from builder image

---
 docker/builder/Dockerfile | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/docker/builder/Dockerfile b/docker/builder/Dockerfile
index c615e43e934..614d4a777ab 100644
--- a/docker/builder/Dockerfile
+++ b/docker/builder/Dockerfile
@@ -13,9 +13,6 @@ RUN apt update -y \
         # For tests: # bash expect python python-lxml python-termcolor curl perl sudo tzdata
  && rm -rf /var/lib/apt/lists/*
 
-RUN git clone https://github.com/curl/curl.git \
- && cd /curl && mkdir .build && cd .build && cmake .. && make && make install
-
 RUN git clone https://github.com/grpc/grpc.git && cd grpc && git checkout v1.14.2 && git submodule update --init \
  && cd /grpc && mkdir .build && cd .build && cmake .. -DgRPC_BUILD_TESTS=OFF -DCMAKE_BUILD_TYPE=Release && make install -j $(nproc || grep -c ^processor /proc/cpuinfo) \
  && rm -rf /grpc/.build \

From e515c3005daf4656bfd7149660e57850ff1570a7 Mon Sep 17 00:00:00 2001
From: zanmato1984 <zanmato1984@gmail.com>
Date: Thu, 18 Jul 2019 19:55:25 +0800
Subject: [PATCH 07/10] Remove useless codes, init schema syncer based on pd
 config

---
 dbms/src/Debug/MockSchemaSyncer.cpp          |  1 -
 dbms/src/Interpreters/Context.cpp            | 21 ++-----------
 dbms/src/Interpreters/Context.h              |  5 ++-
 dbms/src/Server/Server.cpp                   | 32 ++++++++------------
 dbms/src/Storages/Transaction/TMTContext.cpp |  9 ++++--
 dbms/src/Storages/Transaction/TMTContext.h   |  6 +++-
 dbms/src/TiDB/TiDBService.cpp                | 17 -----------
 dbms/src/TiDB/TiDBService.h                  | 29 ------------------
 tests/docker/config/config.xml               |  5 ---
 9 files changed, 29 insertions(+), 96 deletions(-)
 delete mode 100644 dbms/src/TiDB/TiDBService.cpp
 delete mode 100644 dbms/src/TiDB/TiDBService.h

diff --git a/dbms/src/Debug/MockSchemaSyncer.cpp b/dbms/src/Debug/MockSchemaSyncer.cpp
index 9c5f6e78872..9addcfcc148 100644
--- a/dbms/src/Debug/MockSchemaSyncer.cpp
+++ b/dbms/src/Debug/MockSchemaSyncer.cpp
@@ -17,7 +17,6 @@
 #include <Storages/Transaction/TMTContext.h>
 #include <Storages/Transaction/TiDB.h>
 #include <Storages/Transaction/TypeMapping.h>
-#include <TiDB/TiDBService.h>
 #include <common/JSON.h>
 
 
diff --git a/dbms/src/Interpreters/Context.cpp b/dbms/src/Interpreters/Context.cpp
index 141d611e7bf..d5e37309145 100644
--- a/dbms/src/Interpreters/Context.cpp
+++ b/dbms/src/Interpreters/Context.cpp
@@ -51,7 +51,6 @@
 #include <Parsers/ParserCreateQuery.h>
 #include <Parsers/parseQuery.h>
 #include <Raft/RaftService.h>
-#include <TiDB/TiDBService.h>
 
 #include <Common/Config/ConfigProcessor.h>
 #include <Common/ZooKeeper/ZooKeeper.h>
@@ -152,7 +151,6 @@ struct ContextShared
 
     SharedQueriesPtr shared_queries;                        /// The cache of shared queries.
     RaftServicePtr raft_service;                            /// Raft service instance.
-    TiDBServicePtr tidb_service;                            /// TiDB service instance.
     SchemaSyncServicePtr schema_sync_service;               /// Schema sync service instance.
 
     /// Named sessions. The user could specify session identifier to reuse settings and temporary tables in subsequent requests.
@@ -1409,13 +1407,14 @@ void Context::shutdownRaftService()
 void Context::createTMTContext(const std::vector<std::string> & pd_addrs,
                                const std::string & learner_key,
                                const std::string & learner_value,
+                               const std::unordered_set<std::string> & ignore_databases,
                                const std::string & kvstore_path,
                                const std::string & region_mapping_path)
 {
     auto lock = getLock();
     if (shared->tmt_context)
         throw Exception("TMTContext has already existed", ErrorCodes::LOGICAL_ERROR);
-    shared->tmt_context = std::make_shared<TMTContext>(*this, pd_addrs, learner_key, learner_value, kvstore_path, region_mapping_path);
+    shared->tmt_context = std::make_shared<TMTContext>(*this, pd_addrs, learner_key, learner_value, ignore_databases, kvstore_path, region_mapping_path);
 }
 
 RaftService & Context::getRaftService()
@@ -1426,22 +1425,6 @@ RaftService & Context::getRaftService()
     return *shared->raft_service;
 }
 
-void Context::initializeTiDBService(const std::string & service_ip, const std::string & status_port, const std::unordered_set<std::string> & ignore_databases)
-{
-    auto lock = getLock();
-    if (shared->tidb_service)
-        throw Exception("TiDB Service has already been initialized.", ErrorCodes::LOGICAL_ERROR);
-    shared->tidb_service = std::make_shared<TiDBService>(service_ip, status_port, ignore_databases);
-}
-
-TiDBService & Context::getTiDBService()
-{
-    auto lock = getLock();
-    if (!shared->tidb_service)
-        throw Exception("TiDB Service is not initialized.", ErrorCodes::LOGICAL_ERROR);
-    return *shared->tidb_service;
-}
-
 void Context::initializeSchemaSyncService()
 {
     auto lock = getLock();
diff --git a/dbms/src/Interpreters/Context.h b/dbms/src/Interpreters/Context.h
index 4b7402c1a6e..b5ec91baebe 100644
--- a/dbms/src/Interpreters/Context.h
+++ b/dbms/src/Interpreters/Context.h
@@ -6,6 +6,7 @@
 #include <memory>
 #include <mutex>
 #include <thread>
+#include <unordered_set>
 
 #include <common/MultiVersion.h>
 #include <Core/Types.h>
@@ -361,13 +362,11 @@ class Context
     void createTMTContext(const std::vector<std::string> & pd_addrs,
                           const std::string & learner_key,
                           const std::string & learner_value,
+                          const std::unordered_set<std::string> & ignore_databases,
                           const std::string & kvstore_path,
                           const std::string & region_mapping_path);
     RaftService & getRaftService();
 
-    void initializeTiDBService(const std::string & service_ip, const std::string & status_port, const std::unordered_set<std::string> & ignore_databases);
-    TiDBService & getTiDBService();
-
     void initializeSchemaSyncService();
     SchemaSyncServicePtr getSchemaSyncService();
 
diff --git a/dbms/src/Server/Server.cpp b/dbms/src/Server/Server.cpp
index b8ca7b8cc13..db7437ebe0e 100644
--- a/dbms/src/Server/Server.cpp
+++ b/dbms/src/Server/Server.cpp
@@ -314,6 +314,7 @@ int Server::main(const std::vector<std::string> & /*args*/)
     std::vector<std::string> pd_addrs;
     std::string learner_key;
     std::string learner_value;
+    std::unordered_set<std::string> ignore_databases;
     std::string kvstore_path = path + "kvstore/";
     std::string region_mapping_path = path + "regmap/";
 
@@ -354,23 +355,7 @@ int Server::main(const std::vector<std::string> & /*args*/)
             learner_value = "engine";
         }
 
-        if (config().has("raft.kvstore_path"))
-        {
-            kvstore_path = config().getString("raft.kvstore_path");
-        }
-
-        if (config().has("raft.regmap"))
-        {
-            region_mapping_path = config().getString("raft.regmap");
-        }
-    }
-    // TODO: Remove this config once decent schema syncer is done.
-    if (config().has("tidb"))
-    {
-        String service_ip = config().getString("tidb.service_ip");
-        String status_port = config().getString("tidb.status_port");
-        std::unordered_set<std::string> ignore_databases;
-        if (config().has("tidb.ignore_databases"))
+        if (config().has("raft.ignore_databases"))
         {
             String ignore_dbs = config().getString("tidb.ignore_databases");
             Poco::StringTokenizer string_tokens(ignore_dbs, ",");
@@ -382,12 +367,21 @@ int Server::main(const std::vector<std::string> & /*args*/)
             }
             LOG_INFO(log, "Found ignore databases:\n" << ss.str());
         }
-        global_context->initializeTiDBService(service_ip, status_port, ignore_databases);
+
+        if (config().has("raft.kvstore_path"))
+        {
+            kvstore_path = config().getString("raft.kvstore_path");
+        }
+
+        if (config().has("raft.regmap"))
+        {
+            region_mapping_path = config().getString("raft.regmap");
+        }
     }
 
     {
         /// create TMTContext
-        global_context->createTMTContext(pd_addrs, learner_key, learner_value, kvstore_path, region_mapping_path);
+        global_context->createTMTContext(pd_addrs, learner_key, learner_value, ignore_databases, kvstore_path, region_mapping_path);
     }
 
     /// Then, load remaining databases
diff --git a/dbms/src/Storages/Transaction/TMTContext.cpp b/dbms/src/Storages/Transaction/TMTContext.cpp
index e68e3c2e457..ed443f6ce04 100644
--- a/dbms/src/Storages/Transaction/TMTContext.cpp
+++ b/dbms/src/Storages/Transaction/TMTContext.cpp
@@ -1,3 +1,4 @@
+#include <Debug/MockSchemaSyncer.h>
 #include <Interpreters/Context.h>
 #include <Storages/Transaction/KVStore.h>
 #include <Storages/Transaction/SchemaSyncer.h>
@@ -9,14 +10,18 @@ namespace DB
 {
 
 TMTContext::TMTContext(Context & context, const std::vector<std::string> & addrs, const std::string & learner_key,
-    const std::string & learner_value, const std::string & kvstore_path, const std::string & region_mapping_path)
+    const std::string & learner_value, const std::unordered_set<std::string> & ignore_databases_, const std::string & kvstore_path,
+    const std::string & region_mapping_path)
     : kvstore(std::make_shared<KVStore>(kvstore_path)),
       region_table(context, region_mapping_path),
       pd_client(addrs.size() == 0 ? static_cast<pingcap::pd::IClient *>(new pingcap::pd::MockPDClient())
                                   : static_cast<pingcap::pd::IClient *>(new pingcap::pd::Client(addrs))),
       region_cache(std::make_shared<pingcap::kv::RegionCache>(pd_client, learner_key, learner_value)),
       rpc_client(std::make_shared<pingcap::kv::RpcClient>()),
-      schema_syncer(std::make_shared<TiDBSchemaSyncer>(pd_client, region_cache, rpc_client))
+      ignore_databases(ignore_databases_),
+      schema_syncer(addrs.size() == 0
+              ? std::static_pointer_cast<SchemaSyncer>(std::make_shared<MockSchemaSyncer>())
+              : std::static_pointer_cast<SchemaSyncer>(std::make_shared<TiDBSchemaSyncer>(pd_client, region_cache, rpc_client)))
 {}
 
 void TMTContext::restore()
diff --git a/dbms/src/Storages/Transaction/TMTContext.h b/dbms/src/Storages/Transaction/TMTContext.h
index 8b37eda5a0f..8595085d30c 100644
--- a/dbms/src/Storages/Transaction/TMTContext.h
+++ b/dbms/src/Storages/Transaction/TMTContext.h
@@ -8,6 +8,8 @@
 #include <tikv/RegionClient.h>
 #pragma GCC diagnostic pop
 
+#include <unordered_set>
+
 namespace DB
 {
 
@@ -35,7 +37,8 @@ class TMTContext : private boost::noncopyable
 
     // TODO: get flusher args from config file
     explicit TMTContext(Context & context, const std::vector<std::string> & addrs, const std::string & learner_key,
-        const std::string & learner_value, const std::string & kv_store_path, const std::string & region_mapping_path);
+        const std::string & learner_value, const std::unordered_set<std::string> & ignore_databases_, const std::string & kv_store_path,
+        const std::string & region_mapping_path);
 
     SchemaSyncerPtr getSchemaSyncer() const;
     void setSchemaSyncer(SchemaSyncerPtr);
@@ -64,6 +67,7 @@ class TMTContext : private boost::noncopyable
     mutable std::mutex mutex;
     std::atomic_bool initialized = false;
 
+    const std::unordered_set<std::string> ignore_databases;
     SchemaSyncerPtr schema_syncer;
 };
 
diff --git a/dbms/src/TiDB/TiDBService.cpp b/dbms/src/TiDB/TiDBService.cpp
deleted file mode 100644
index 5e49d4a04c0..00000000000
--- a/dbms/src/TiDB/TiDBService.cpp
+++ /dev/null
@@ -1,17 +0,0 @@
-#include <TiDB/TiDBService.h>
-
-namespace DB
-{
-
-TiDBService::TiDBService(
-    const std::string & service_ip_, const std::string & status_port_, const std::unordered_set<std::string> & ignore_databases_)
-    : service_ip(service_ip_), status_port(status_port_), ignore_databases(ignore_databases_)
-{}
-
-const std::string & TiDBService::serviceIp() const { return service_ip; }
-
-const std::string & TiDBService::statusPort() const { return status_port; }
-
-const std::unordered_set<std::string> & TiDBService::ignoreDatabases() const { return ignore_databases; }
-
-} // namespace DB
diff --git a/dbms/src/TiDB/TiDBService.h b/dbms/src/TiDB/TiDBService.h
deleted file mode 100644
index 32967ad65a9..00000000000
--- a/dbms/src/TiDB/TiDBService.h
+++ /dev/null
@@ -1,29 +0,0 @@
-#pragma once
-
-#include <memory>
-#include <string>
-#include <unordered_set>
-
-#include <boost/noncopyable.hpp>
-
-namespace DB
-{
-
-class TiDBService final : public std::enable_shared_from_this<TiDBService>, private boost::noncopyable
-{
-public:
-    TiDBService(
-        const std::string & service_ip_, const std::string & status_port_, const std::unordered_set<std::string> & ignore_databases_);
-    const std::string & serviceIp() const;
-    const std::string & statusPort() const;
-    const std::unordered_set<std::string> & ignoreDatabases() const;
-
-private:
-    const std::string service_ip;
-
-    const std::string status_port;
-
-    const std::unordered_set<std::string> ignore_databases;
-};
-
-} // namespace DB
diff --git a/tests/docker/config/config.xml b/tests/docker/config/config.xml
index a4ea5f68d53..e6911f1227e 100644
--- a/tests/docker/config/config.xml
+++ b/tests/docker/config/config.xml
@@ -21,11 +21,6 @@
         <!--pd_addr>127.0.0.1:2379</pd_addr-->
     </raft>
 
-    <tidb>
-        <service_ip>127.0.0.1</service_ip>
-        <status_port>10080</status_port>
-    </tidb>
-
     <http_port>8123</http_port>
     <tcp_port>9000</tcp_port>
     <interserver_http_port>9009</interserver_http_port>

From e084771576653878e0d09d5b6cf3bcf35baa9124 Mon Sep 17 00:00:00 2001
From: zanmato1984 <zanmato1984@gmail.com>
Date: Thu, 18 Jul 2019 22:41:31 +0800
Subject: [PATCH 08/10] Minor fix to schema debug

---
 dbms/src/Debug/MockSchemaSyncer.cpp |  9 ++-------
 dbms/src/Debug/dbgFuncSchema.cpp    | 10 ++++++++--
 dbms/src/Interpreters/Context.cpp   |  4 +---
 dbms/src/Interpreters/Context.h     |  2 +-
 4 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/dbms/src/Debug/MockSchemaSyncer.cpp b/dbms/src/Debug/MockSchemaSyncer.cpp
index 9addcfcc148..ef0f2966da4 100644
--- a/dbms/src/Debug/MockSchemaSyncer.cpp
+++ b/dbms/src/Debug/MockSchemaSyncer.cpp
@@ -261,13 +261,8 @@ void MockSchemaSyncer::syncTable(Context & context, MockTiDB::TablePtr table)
     auto & tmt_context = context.getTMTContext();
 
     /// Get table schema json.
-    auto table_id = table->id();
-
-    String table_info_json = table->table_info.serialize(false);
-
-    LOG_DEBUG(log, __PRETTY_FUNCTION__ << ": Table " << table_id << " info json: " << table_info_json);
-
-    TableInfo table_info(table_info_json, false);
+    TableInfo table_info = table->table_info;
+    auto table_id = table_info.id;
 
     auto storage = tmt_context.getStorages().get(table_id);
 
diff --git a/dbms/src/Debug/dbgFuncSchema.cpp b/dbms/src/Debug/dbgFuncSchema.cpp
index dc45d067e76..5431f9baaad 100644
--- a/dbms/src/Debug/dbgFuncSchema.cpp
+++ b/dbms/src/Debug/dbgFuncSchema.cpp
@@ -24,9 +24,15 @@ void dbgFuncEnableSchemaSyncService(Context & context, const ASTs & args, DBGInv
     bool enable = safeGet<String>(typeid_cast<const ASTLiteral &>(*args[0]).value) == "true";
 
     if (enable)
-        context.initializeSchemaSyncService();
+    {
+        if (!context.getSchemaSyncService())
+            context.initializeSchemaSyncService();
+    }
     else
-        context.getSchemaSyncService().reset();
+    {
+        if (context.getSchemaSyncService())
+            context.getSchemaSyncService().reset();
+    }
 
     std::stringstream ss;
     ss << "schema sync service " << (enable ? "enabled" : "disabled");
diff --git a/dbms/src/Interpreters/Context.cpp b/dbms/src/Interpreters/Context.cpp
index d5e37309145..41f7c3078d9 100644
--- a/dbms/src/Interpreters/Context.cpp
+++ b/dbms/src/Interpreters/Context.cpp
@@ -1433,11 +1433,9 @@ void Context::initializeSchemaSyncService()
     shared->schema_sync_service = std::make_shared<SchemaSyncService>(*this);
 }
 
-SchemaSyncServicePtr Context::getSchemaSyncService()
+SchemaSyncServicePtr & Context::getSchemaSyncService()
 {
     auto lock = getLock();
-    if (!shared->schema_sync_service)
-        throw Exception("Schema Sync Service is not initialized.", ErrorCodes::LOGICAL_ERROR);
     return shared->schema_sync_service;
 }
 
diff --git a/dbms/src/Interpreters/Context.h b/dbms/src/Interpreters/Context.h
index b5ec91baebe..1c1f9b8c041 100644
--- a/dbms/src/Interpreters/Context.h
+++ b/dbms/src/Interpreters/Context.h
@@ -368,7 +368,7 @@ class Context
     RaftService & getRaftService();
 
     void initializeSchemaSyncService();
-    SchemaSyncServicePtr getSchemaSyncService();
+    SchemaSyncServicePtr & getSchemaSyncService();
 
     Clusters & getClusters() const;
     std::shared_ptr<Cluster> getCluster(const std::string & cluster_name) const;

From e89c697e419aad7a737f5fa9aeee929368137d26 Mon Sep 17 00:00:00 2001
From: zanmato1984 <zanmato1984@gmail.com>
Date: Fri, 19 Jul 2019 01:44:33 +0800
Subject: [PATCH 09/10] Fix alter tmt and pass tests

---
 dbms/src/Storages/StorageMergeTree.cpp   | 55 +++++++++---------------
 dbms/src/Storages/StorageMergeTree.h     |  2 +
 tests/mutable-test/txn_schema/alter.test |  8 ++--
 tests/mutable-test/txn_schema/drop.test  |  6 +--
 4 files changed, 31 insertions(+), 40 deletions(-)

diff --git a/dbms/src/Storages/StorageMergeTree.cpp b/dbms/src/Storages/StorageMergeTree.cpp
index c8f90d96c74..f57e31e3f43 100644
--- a/dbms/src/Storages/StorageMergeTree.cpp
+++ b/dbms/src/Storages/StorageMergeTree.cpp
@@ -303,6 +303,25 @@ void StorageMergeTree::alter(
     const String & database_name,
     const String & table_name,
     const Context & context)
+{
+    alterInternal(params, database_name, table_name, std::nullopt, context);
+}
+
+void StorageMergeTree::alterForTMT(
+    const AlterCommands & params,
+    const TiDB::TableInfo & table_info,
+    const String & database_name,
+    const Context & context)
+{
+    alterInternal(params, database_name, table_info.name, std::optional<std::reference_wrapper<const TableInfo>>(table_info), context);
+}
+
+void StorageMergeTree::alterInternal(
+    const AlterCommands & params,
+    const String & database_name,
+    const String & table_name,
+    const std::optional<std::reference_wrapper<const TiDB::TableInfo>> table_info,
+    const Context & context)
 {
     /// NOTE: Here, as in ReplicatedMergeTree, you can do ALTER which does not block the writing of data for a long time.
     auto merge_blocker = merger.merges_blocker.cancel();
@@ -364,6 +383,8 @@ void StorageMergeTree::alter(
 
     context.getDatabase(database_name)->alterTable(context, table_name, new_columns, storage_modifier);
     setColumns(std::move(new_columns));
+    if (table_info)
+        setTableInfo(table_info->get());
 
     if (primary_key_is_modified)
     {
@@ -382,40 +403,6 @@ void StorageMergeTree::alter(
         data.loadDataParts(false);
 }
 
-void StorageMergeTree::alterForTMT(
-    const AlterCommands & params,
-    const TiDB::TableInfo & table_info,
-    const String & database_name,
-    const Context & context)
-{
-    const String & table_name = table_info.name;
-    /// NOTE: Here, as in ReplicatedMergeTree, you can do ALTER which does not block the writing of data for a long time.
-    auto merge_blocker = merger.merges_blocker.cancel();
-
-    auto table_soft_lock = lockDataForAlter(__PRETTY_FUNCTION__);
-
-    data.checkAlter(params);
-
-    auto new_columns = data.getColumns();
-    params.apply(new_columns);
-
-//    std::vector<MergeTreeData::AlterDataPartTransactionPtr> transactions;
-
-    auto table_hard_lock = lockStructureForAlter(__PRETTY_FUNCTION__);
-
-    IDatabase::ASTModifier storage_modifier = [this] (IAST & ast)
-    {
-        auto & storage_ast = typeid_cast<ASTStorage &>(ast);
-
-        auto literal = std::make_shared<ASTLiteral>(Field(data.table_info->serialize(true)));
-        typeid_cast<ASTExpressionList &>(*storage_ast.engine->arguments).children.back() = literal;
-    };
-
-    context.getDatabase(database_name)->alterTable(context, table_name, new_columns, storage_modifier);
-    setTableInfo(table_info);
-    setColumns(std::move(new_columns));
-}
-
 /// While exists, marks parts as 'currently_merging' and reserves free space on filesystem.
 /// It's possible to mark parts before.
 struct CurrentlyMergingPartsTagger
diff --git a/dbms/src/Storages/StorageMergeTree.h b/dbms/src/Storages/StorageMergeTree.h
index 8582e8232d5..6cdfe9ea03d 100644
--- a/dbms/src/Storages/StorageMergeTree.h
+++ b/dbms/src/Storages/StorageMergeTree.h
@@ -81,6 +81,8 @@ class StorageMergeTree : public ext::shared_ptr_helper<StorageMergeTree>, public
 
     void alterForTMT(const AlterCommands & params, const TiDB::TableInfo & table_info, const String & database_name, const Context & context);
 
+    void alterInternal(const AlterCommands & params, const String & database_name, const String & table_name, const std::optional<std::reference_wrapper<const TiDB::TableInfo>> table_info, const Context & context);
+
     bool checkTableCanBeDropped() const override;
 
     const TableInfo & getTableInfo() const;
diff --git a/tests/mutable-test/txn_schema/alter.test b/tests/mutable-test/txn_schema/alter.test
index c5500e9f68d..9fa67026043 100644
--- a/tests/mutable-test/txn_schema/alter.test
+++ b/tests/mutable-test/txn_schema/alter.test
@@ -5,9 +5,11 @@
 => DBGInvoke __set_flush_threshold(1000000, 1000000)
 => DBGInvoke __mock_schema_syncer('true')
 
+=> DBGInvoke __enable_schema_sync_service('false')
+
 # Sync add column by checking missing column in CH when flushing.
 => DBGInvoke __mock_tidb_table(default, test, 'col_1 String')
-=> DBGInvoke __refresh_schema(default, test)
+=> DBGInvoke __refresh_schemas()
 => DBGInvoke __put_region(4, 0, 100, default, test)
 => select col_1 from default.test
 => DBGInvoke __add_column_to_tidb_table(default, test, 'col_2 Nullable(Int8)')
@@ -71,7 +73,7 @@ Code: 47. DB::Exception: Received from {#WORD} DB::Exception: Unknown identifier
 
 # Sync add column and type change together by checking value overflow in CH when flushing.
 => DBGInvoke __add_column_to_tidb_table(default, test, 'col_3 UInt8')
-=> DBGInvoke __refresh_schema(default, test)
+=> DBGInvoke __refresh_schemas()
 => DBGInvoke __modify_column_in_tidb_table(default, test, 'col_3 UInt64')
 => DBGInvoke __raft_insert_row(default, test, 4, 55, 0, 256)
 => DBGInvoke __add_column_to_tidb_table(default, test, 'col_4 Nullable(UInt8)')
@@ -109,7 +111,7 @@ Code: 47. DB::Exception: Received from {#WORD} DB::Exception: Unknown identifier
 │ -9223372036854775807 │ 18446744073709551615 │     1 │
 │  9223372036854775807 │ 18446744073709551615 │     1 │
 └──────────────────────┴──────────────────────┴───────┘
-=> DBGInvoke __refresh_schema(default, test)
+=> DBGInvoke __refresh_schemas()
 => selraw nokvstore col_3 from default.test
 Received exception from server (version {#WORD}):
 Code: 47. DB::Exception: Received from {#WORD} DB::Exception: Unknown identifier: col_3.
diff --git a/tests/mutable-test/txn_schema/drop.test b/tests/mutable-test/txn_schema/drop.test
index f1126511591..bb71443b30f 100644
--- a/tests/mutable-test/txn_schema/drop.test
+++ b/tests/mutable-test/txn_schema/drop.test
@@ -5,7 +5,7 @@
 => DBGInvoke __mock_schema_syncer('true')
 
 => DBGInvoke __mock_tidb_table(default, test, 'col_1 String')
-=> DBGInvoke __refresh_schema(default, test)
+=> DBGInvoke __refresh_schemas()
 => DBGInvoke __put_region(4, 0, 100, default, test)
 => select col_1 from default.test
 => DBGInvoke __add_column_to_tidb_table(default, test, 'col_2 Nullable(Int8)')
@@ -17,7 +17,7 @@ Received exception from server (version {#WORD}):
 Code: 60. DB::Exception: Received from {#WORD} DB::Exception: Table default.test doesn't exist..
 
 => DBGInvoke __mock_tidb_table(default, test, 'col_1 String, col_2 Nullable(Int8)')
-=> DBGInvoke __refresh_schema(default, test)
+=> DBGInvoke __refresh_schemas()
 => select col_1, col_2 from default.test
 => DBGInvoke __drop_column_from_tidb_table(default, test, col_2)
 => DBGInvoke __raft_insert_row(default, test, 4, 51, 'test2')
@@ -28,7 +28,7 @@ Received exception from server (version {#WORD}):
 Code: 60. DB::Exception: Received from {#WORD} DB::Exception: Table default.test doesn't exist..
 
 => DBGInvoke __mock_tidb_table(default, test, 'col_1 String, col_2 Nullable(Int8)')
-=> DBGInvoke __refresh_schema(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)')
 => DBGInvoke __raft_insert_row(default, test, 4, 52, 'test2', 256)

From 074f521c29a99af41c25413892fc8196f60212de Mon Sep 17 00:00:00 2001
From: zanmato1984 <zanmato1984@gmail.com>
Date: Fri, 19 Jul 2019 01:55:15 +0800
Subject: [PATCH 10/10] Fix build fail

---
 dbms/src/Interpreters/Context.cpp | 1 -
 1 file changed, 1 deletion(-)

diff --git a/dbms/src/Interpreters/Context.cpp b/dbms/src/Interpreters/Context.cpp
index feee3162f22..afe6faf59c6 100644
--- a/dbms/src/Interpreters/Context.cpp
+++ b/dbms/src/Interpreters/Context.cpp
@@ -27,7 +27,6 @@
 #include <Storages/Transaction/SchemaSyncService.h>
 #include <Storages/Transaction/TMTContext.h>
 #include <Storages/PartPathSelector.h>
-#include <Storages/CompressionSettingsSelector.h>
 #include <TableFunctions/TableFunctionFactory.h>
 #include <Interpreters/Settings.h>
 #include <Interpreters/RuntimeComponentsFactory.h>