Fix DeltaMerge CI (#350)

* fix read_tso while doing StorageDeltaMerge_test::ReadWriteCase * fix temporary path * fix bug: non-initlized variable Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
pingcap · Dec 12, 2019 · 621dc2d · 621dc2d
1 parent 69f490f
commit 621dc2d
Show file tree

Hide file tree

Showing 9 changed files with 33 additions and 19 deletions.
diff --git a/dbms/src/Parsers/ASTInsertQuery.h b/dbms/src/Parsers/ASTInsertQuery.h
@@ -33,9 +33,10 @@ class ASTInsertQuery : public IAST
     const char * data = nullptr;
     const char * end = nullptr;
 
-    bool is_import;
-    bool is_upsert;
-    bool is_delete;
+    // If insert block is synced from TiDB, set is_import = true
+    bool is_import = false;
+    bool is_upsert = false;
+    bool is_delete = false;
 
     /** Get the text that identifies this element. */
     String getID() const override { return "InsertQuery_" + database + "_" + table; };

diff --git a/dbms/src/Storages/DeltaMerge/tests/gtest_dm_delta_merge_store.cpp b/dbms/src/Storages/DeltaMerge/tests/gtest_dm_delta_merge_store.cpp
@@ -1,5 +1,6 @@
 #include <gtest/gtest.h>
 #include <memory>
+#include <test_utils/TiflashTestBasic.h>
 #include "dm_basic_include.h"
 
 #include <Poco/ConsoleChannel.h>
@@ -22,7 +23,7 @@ namespace tests
 class DeltaMergeStore_test : public ::testing::Test
 {
 public:
-    DeltaMergeStore_test() : name("tmp"), path("./" + name) {}
+    DeltaMergeStore_test() : name("tmp"), path(DB::tests::TiFlashTestEnv::getTemporaryPath() + name) {}
 
 protected:
     static void SetUpTestCase()

diff --git a/dbms/src/Storages/DeltaMerge/tests/gtest_dm_disk_value_space.cpp b/dbms/src/Storages/DeltaMerge/tests/gtest_dm_disk_value_space.cpp
@@ -19,7 +19,7 @@ namespace tests
 class DiskValueSpace_test : public ::testing::Test
 {
 public:
-    DiskValueSpace_test() : name("tmp"), path("./" + name), storage_pool() {}
+    DiskValueSpace_test() : name("tmp"), path(DB::tests::TiFlashTestEnv::getTemporaryPath() +name), storage_pool() {}
 
 private:
     void dropDataInDisk()

diff --git a/dbms/src/Storages/DeltaMerge/tests/gtest_dm_file.cpp b/dbms/src/Storages/DeltaMerge/tests/gtest_dm_file.cpp
@@ -20,7 +20,7 @@ using DMFileBlockInputStreamPtr  = std::shared_ptr<DMFileBlockInputStream>;
 class DMFile_Test : public ::testing::Test
 {
 public:
-    DMFile_Test() : path("tmp"), dm_file(nullptr) {}
+    DMFile_Test() : path(DB::tests::TiFlashTestEnv::getTemporaryPath() + "/dm_file_tests"), dm_file(nullptr) {}
 
     void SetUp() override
     {
@@ -55,7 +55,8 @@ class DMFile_Test : public ::testing::Test
                                                  db_context->getSettingsRef().dm_segment_limit_rows,
                                                  db_context->getSettingsRef().dm_segment_delta_limit_rows,
                                                  db_context->getSettingsRef().dm_segment_delta_cache_limit_rows,
-                                                 db_context->getSettingsRef().dm_segment_stable_chunk_rows);
+                                                 db_context->getSettingsRef().dm_segment_stable_chunk_rows,
+                                                 db_context->getSettingsRef().dm_enable_logical_split);
     }
 
 

diff --git a/dbms/src/Storages/DeltaMerge/tests/gtest_dm_segment.cpp b/dbms/src/Storages/DeltaMerge/tests/gtest_dm_segment.cpp
@@ -22,7 +22,7 @@ namespace tests
 class Segment_test : public ::testing::Test
 {
 public:
-    Segment_test() : name("tmp"), path("./" + name), storage_pool() {}
+    Segment_test() : name("tmp"), path(DB::tests::TiFlashTestEnv::getTemporaryPath() + name), storage_pool() {}
 
 private:
     void dropDataInDisk()

diff --git a/dbms/src/Storages/DeltaMerge/tests/gtest_dm_storage_delta_merge.cpp b/dbms/src/Storages/DeltaMerge/tests/gtest_dm_storage_delta_merge.cpp
@@ -1,4 +1,5 @@
 #include <gtest/gtest.h>
+#include <limits>
 #include "dm_basic_include.h"
 
 #include <Columns/IColumn.h>
@@ -85,7 +86,7 @@ try
             column_names.push_back(name_type.name);
         }
 
-        const String path_name = "tmp/";
+        const String path_name = DB::tests::TiFlashTestEnv::getTemporaryPath();
         Poco::File   path(path_name);
         if (path.exists())
             path.remove(true);
@@ -194,7 +195,7 @@ try
             column_names.push_back(name_type.name);
         }
 
-        const String path_name = "tmp/";
+        const String path_name = DB::tests::TiFlashTestEnv::getTemporaryPath();
         Poco::File   path(path_name);
         if (path.exists())
             path.remove(true);
@@ -231,7 +232,7 @@ try
     query_info.query                          = std::make_shared<ASTSelectQuery>();
     query_info.mvcc_query_info                = std::make_unique<MvccQueryInfo>();
     query_info.mvcc_query_info->resolve_locks = global_ctx.getSettingsRef().resolve_locks;
-    query_info.mvcc_query_info->read_tso      = global_ctx.getSettingsRef().read_tso;
+    query_info.mvcc_query_info->read_tso      = std::numeric_limits<UInt64>::max();
     BlockInputStreams ins                     = storage->read(column_names, query_info, global_ctx, stage2, 8192, 1);
     ASSERT_EQ(ins.size(), 1UL);
     BlockInputStreamPtr in = ins[0];

diff --git a/dbms/src/Storages/Page/tests/gtest_page_storage.cpp b/dbms/src/Storages/Page/tests/gtest_page_storage.cpp
@@ -1,4 +1,5 @@
 #include "gtest/gtest.h"
+#include <test_utils/TiflashTestBasic.h>
 
 #include <Common/CurrentMetrics.h>
 #include <IO/ReadBufferFromMemory.h>
@@ -27,7 +28,7 @@ namespace tests
 class PageStorage_test : public ::testing::Test
 {
 public:
-    PageStorage_test() : path("./t"), storage() {}
+    PageStorage_test() : path(DB::tests::TiFlashTestEnv::getTemporaryPath() + "/page_storage_test"), storage() {}
 
 protected:
     static void SetUpTestCase()

diff --git a/dbms/src/Storages/StorageDeltaMerge.cpp b/dbms/src/Storages/StorageDeltaMerge.cpp
@@ -502,12 +502,19 @@ BlockInputStreams StorageDeltaMerge::read( //
             throw Exception("TMTContext is not initialized", ErrorCodes::LOGICAL_ERROR);
 
         const auto & mvcc_query_info = *query_info.mvcc_query_info;
+
         // Read with specify tso, check if tso is smaller than TiDB GcSafePoint
-        const auto safe_point = tmt.getPDClient()->getGCSafePoint();
-        if (mvcc_query_info.read_tso < safe_point)
-            throw Exception("query id: " + context.getCurrentQueryId() + ", read tso: " + toString(mvcc_query_info.read_tso)
-                    + " is smaller than tidb gc safe point: " + toString(safe_point),
-                ErrorCodes::LOGICAL_ERROR);
+        auto pd_client = tmt.getPDClient();
+        if (likely(!pd_client->isMock()))
+        {
+            auto safe_point = PDClientHelper::getGCSafePointWithRetry(pd_client,
+                /* ignore_cache= */ false,
+                global_context.getSettingsRef().safe_point_update_interval_seconds);
+            if (mvcc_query_info.read_tso < safe_point)
+                throw Exception("query id: " + context.getCurrentQueryId() + ", read tso: " + toString(mvcc_query_info.read_tso)
+                        + " is smaller than tidb gc safe point: " + toString(safe_point),
+                    ErrorCodes::LOGICAL_ERROR);
+        }
 
         /// If request comes from TiDB/TiSpark, mvcc_query_info.concurrent is 0,
         /// and `concurrent_num` should be 1. Concurrency handled by TiDB/TiSpark.

diff --git a/dbms/src/test_utils/TiflashTestBasic.h b/dbms/src/test_utils/TiflashTestBasic.h
@@ -13,10 +13,12 @@ namespace tests
 class TiFlashTestEnv
 {
 public:
+    static String getTemporaryPath() { return "./tmp/"; }
+
     static Context & getContext(const DB::Settings & settings = DB::Settings())
     {
         static Context context = DB::Context::createGlobal();
-        context.setPath(".");
+        context.setPath(getTemporaryPath());
         context.setGlobalContext(context);
         try
         {
@@ -28,7 +30,7 @@ class TiFlashTestEnv
             context.setGlobalContext(context);
             context.setApplicationType(DB::Context::ApplicationType::SERVER);
 
-            context.createTMTContext({}, "", "", {"default"}, "./tmp/kvstore", "", TiDB::StorageEngine::TMT, false);
+            context.createTMTContext({}, "", "", {"default"}, getTemporaryPath() + "/kvstore", "", TiDB::StorageEngine::TMT, false);
             context.getTMTContext().restore();
         }
         context.getSettingsRef() = settings;