From 47153fcc88c0513639f96132fb99801f209c6c4b Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Fri, 13 May 2022 22:43:43 +0800 Subject: [PATCH 1/3] Fix fail for --gtest_filter='*StorageDeltaMergeTest*:RegionKVStoreTest.KVStore' --- .../tests/gtest_dm_storage_delta_merge.cpp | 10 ++++---- .../Transaction/tests/gtest_kvstore.cpp | 23 ++++++++++++++++--- 2 files changed, 26 insertions(+), 7 deletions(-) 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 index a26471cfe01..d03f2b60a87 100644 --- a/dbms/src/Storages/DeltaMerge/tests/gtest_dm_storage_delta_merge.cpp +++ b/dbms/src/Storages/DeltaMerge/tests/gtest_dm_storage_delta_merge.cpp @@ -40,6 +40,7 @@ #include +#include "Storages/Transaction/TiDB.h" #include "dm_basic_include.h" namespace DB @@ -609,6 +610,8 @@ try sample.insert(DB::tests::createColumn( Strings(100, "a"), "col2")); + constexpr TiDB::TableID table_id = 1232; + const String table_name = fmt::format("t_{}", table_id); Context ctx = DMTestEnv::getContext(); std::shared_ptr storage; @@ -631,12 +634,11 @@ try path.remove(true); // primary_expr_ast - const String table_name = "t_1233"; ASTPtr astptr(new ASTIdentifier(table_name, ASTIdentifier::Kind::Table)); astptr->children.emplace_back(new ASTIdentifier("col1")); TiDB::TableInfo tidb_table_info; - tidb_table_info.id = 1; + tidb_table_info.id = table_id; storage = StorageDeltaMerge::create("TiFlash", /* db_name= */ "default", @@ -692,8 +694,8 @@ try { Field res; c->get(i, res); - ASSERT(!res.isNull()); - ASSERT(res.get() == 1); + ASSERT_TRUE(!res.isNull()); + ASSERT_EQ(res.get(), table_id); } } } diff --git a/dbms/src/Storages/Transaction/tests/gtest_kvstore.cpp b/dbms/src/Storages/Transaction/tests/gtest_kvstore.cpp index e93a117cc1c..2378871f71f 100644 --- a/dbms/src/Storages/Transaction/tests/gtest_kvstore.cpp +++ b/dbms/src/Storages/Transaction/tests/gtest_kvstore.cpp @@ -47,8 +47,7 @@ RegionPtr makeRegion(UInt64 id, const std::string start_key, const std::string e class RegionKVStoreTest : public ::testing::Test { public: - RegionKVStoreTest() - = default; + RegionKVStoreTest() = default; static void SetUpTestCase() {} static void testBasic(); @@ -1372,12 +1371,30 @@ void RegionKVStoreTest::testBasic() } } -TEST_F(RegionKVStoreTest, run) +TEST_F(RegionKVStoreTest, Basic) try { testBasic(); +} +CATCH + +TEST_F(RegionKVStoreTest, KVStore) +try +{ testKVStore(); +} +CATCH + +TEST_F(RegionKVStoreTest, Region) +try +{ testRegion(); +} +CATCH + +TEST_F(RegionKVStoreTest, ReadIndex) +try +{ testReadIndex(); } CATCH From 270514aba27a9dc2c8bc83d7d1064c84de01034a Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Mon, 16 May 2022 14:32:14 +0800 Subject: [PATCH 2/3] enable tests & clenup config for mock test --- .../tests/gtest_dm_storage_delta_merge.cpp | 2 +- tests/docker/config/tics_dt.toml | 13 +--- .../docker/config/tiflash_dt_async_grpc.toml | 69 ------------------- .../tiflash_dt_disable_local_tunnel.toml | 69 ------------------- 4 files changed, 3 insertions(+), 150 deletions(-) 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 index d03f2b60a87..c3573f3d8ca 100644 --- a/dbms/src/Storages/DeltaMerge/tests/gtest_dm_storage_delta_merge.cpp +++ b/dbms/src/Storages/DeltaMerge/tests/gtest_dm_storage_delta_merge.cpp @@ -34,13 +34,13 @@ #include #include #include +#include #include #include #include #include -#include "Storages/Transaction/TiDB.h" #include "dm_basic_include.h" namespace DB diff --git a/tests/docker/config/tics_dt.toml b/tests/docker/config/tics_dt.toml index 89147f80c7d..56bef659cb7 100644 --- a/tests/docker/config/tics_dt.toml +++ b/tests/docker/config/tics_dt.toml @@ -13,25 +13,16 @@ # limitations under the License. tmp_path = "/tmp/tiflash/data/tmp" -display_name = "TiFlash" -# specify paths used for store data, multiple path should be seperated by comma path = "/tmp/tiflash/data/db" -capacity = "107374182400" -# multi-paths example -# path = "/tmp/tiflash/data/db,/tmp/tiflash1,/tmp/tiflash2" -# capacity = "0,0,0" +capacity = "107374182400" # 100GB mark_cache_size = 5368709120 minmax_index_cache_size = 5368709120 tcp_port = 9000 http_port = 8123 + [logger] count = 10 errorlog = "/tmp/tiflash/log/error.log" size = "1000M" log = "/tmp/tiflash/log/server.log" level = "trace" -[application] -runAsDaemon = true -[raft] -# specify which storage engine we use. tmt or dt -storage_engine = "dt" diff --git a/tests/docker/config/tiflash_dt_async_grpc.toml b/tests/docker/config/tiflash_dt_async_grpc.toml index 3c67c37db33..bf31c61cfa8 100644 --- a/tests/docker/config/tiflash_dt_async_grpc.toml +++ b/tests/docker/config/tiflash_dt_async_grpc.toml @@ -13,71 +13,15 @@ # limitations under the License. tmp_path = "/tmp/tiflash/data/tmp" -display_name = "TiFlash" -## Deprecated storage path setting style. Check [storage] section for new style. path = "/tmp/tiflash/data/db" capacity = "10737418240" -## Deprecated storage path setting style of multi-disks. Check [storage] section for new style. -# path = "/tmp/tiflash/data/db,/tmp/tiflash1,/tmp/tiflash2" -# capacity = "0" mark_cache_size = 5368709120 minmax_index_cache_size = 5368709120 tcp_port = 9000 http_port = 8123 -## Storage paths settings. -# [storage] -## The storage format version in storage engine. Valid values: 1, 2 (experimental). -## format_version = 1 - -## If there are multiple SSD disks on the machine, -## specify the path list on `storage.main.dir` can improve TiFlash performance. - -## If there are multiple disks with different IO metrics (e.g. one SSD and some HDDs) -## on the machine, -## set `storage.latest.dir` to store the latest data on SSD (disks with higher IOPS metrics) -## set `storage.main.dir` to store the main data on HDD (disks with lower IOPS metrics) -## can improve TiFlash performance. - -# [storage.main] -## The path to store main data. -# e.g. -# dir = [ "/data0/tiflash" ] -# or -# dir = [ "/data0/tiflash", "/data1/tiflash" ] - -## Store capacity of each path, i.e. max data size allowed. -## If it is not set, or is set to 0s, the actual disk capacity is used. -## Note that we don't support human-readable big numbers(like "10GB") yet. -## Please set in the specified number of bytes. -# e.g. -# capacity = [ 10737418240, 10737418240 ] - -# [storage.latest] -## The path(s) to store latest data. -## If not set, it will be the same with `storage.main.dir`. -# dir = [ ] - -## Store capacity of each path, i.e. max data size allowed. -## If it is not set, or is set to 0s, the actual disk capacity is used. -# e.g. -# capacity = [ 10737418240, 10737418240 ] - -# [storage.raft] -## The path(s) to store Raft data. -## If not set, it will be the paths in `storage.latest.dir` appended with "/kvstore". -# dir = [ ] - -# [storage.io_rate_limit] -## The max I/O bandwith. Default value is 0 and I/O rate limit is disabled. -# max_bytes_per_sec = 268435456 -## max_read_bytes_per_sec and max_write_bytes_per_sec are the same meaning as max_bytes_per_sec, -## but for disk that read bandwidth and write bandwith are calculated separatly, such as GCP's persistent disks. -# max_read_bytes_per_sec = 0 -# max_write_bytes_per_sec = 0 - [flash] tidb_status_addr = "tidb0:10080" service_addr = "0.0.0.0:3930" @@ -100,22 +44,9 @@ size = "1000M" log = "/tmp/tiflash/log/server.log" level = "trace" -[application] -runAsDaemon = true - [raft] pd_addr = "pd0:2379" ignore_databases = "system,default" -# specify which storage engine we use. tmt or dt -storage_engine = "dt" -# Deprecated Raft data storage path setting style. Check [storage.raft] section for new style. -# If it is not set, it will be the first path of "path" appended with "/kvstore". -# kvstore_path = "" - -[raft.snapshot] -# The way to apply snapshot data -# The value is one of "block" / "file1" / "file2". -# method = "file1" [profiles] [profiles.default] diff --git a/tests/docker/config/tiflash_dt_disable_local_tunnel.toml b/tests/docker/config/tiflash_dt_disable_local_tunnel.toml index 23b82909776..1fb166a9a19 100644 --- a/tests/docker/config/tiflash_dt_disable_local_tunnel.toml +++ b/tests/docker/config/tiflash_dt_disable_local_tunnel.toml @@ -13,71 +13,15 @@ # limitations under the License. tmp_path = "/tmp/tiflash/data/tmp" -display_name = "TiFlash" -## Deprecated storage path setting style. Check [storage] section for new style. path = "/tmp/tiflash/data/db" capacity = "10737418240" -## Deprecated storage path setting style of multi-disks. Check [storage] section for new style. -# path = "/tmp/tiflash/data/db,/tmp/tiflash1,/tmp/tiflash2" -# capacity = "0" mark_cache_size = 5368709120 minmax_index_cache_size = 5368709120 tcp_port = 9000 http_port = 8123 -## Storage paths settings. -# [storage] -## The storage format version in storage engine. Valid values: 1, 2 (experimental). -## format_version = 1 - -## If there are multiple SSD disks on the machine, -## specify the path list on `storage.main.dir` can improve TiFlash performance. - -## If there are multiple disks with different IO metrics (e.g. one SSD and some HDDs) -## on the machine, -## set `storage.latest.dir` to store the latest data on SSD (disks with higher IOPS metrics) -## set `storage.main.dir` to store the main data on HDD (disks with lower IOPS metrics) -## can improve TiFlash performance. - -# [storage.main] -## The path to store main data. -# e.g. -# dir = [ "/data0/tiflash" ] -# or -# dir = [ "/data0/tiflash", "/data1/tiflash" ] - -## Store capacity of each path, i.e. max data size allowed. -## If it is not set, or is set to 0s, the actual disk capacity is used. -## Note that we don't support human-readable big numbers(like "10GB") yet. -## Please set in the specified number of bytes. -# e.g. -# capacity = [ 10737418240, 10737418240 ] - -# [storage.latest] -## The path(s) to store latest data. -## If not set, it will be the same with `storage.main.dir`. -# dir = [ ] - -## Store capacity of each path, i.e. max data size allowed. -## If it is not set, or is set to 0s, the actual disk capacity is used. -# e.g. -# capacity = [ 10737418240, 10737418240 ] - -# [storage.raft] -## The path(s) to store Raft data. -## If not set, it will be the paths in `storage.latest.dir` appended with "/kvstore". -# dir = [ ] - -# [storage.io_rate_limit] -## The max I/O bandwith. Default value is 0 and I/O rate limit is disabled. -# max_bytes_per_sec = 268435456 -## max_read_bytes_per_sec and max_write_bytes_per_sec are the same meaning as max_bytes_per_sec, -## but for disk that read bandwidth and write bandwith are calculated separatly, such as GCP's persistent disks. -# max_read_bytes_per_sec = 0 -# max_write_bytes_per_sec = 0 - [flash] tidb_status_addr = "tidb0:10080" service_addr = "0.0.0.0:3930" @@ -100,22 +44,9 @@ size = "1000M" log = "/tmp/tiflash/log/server.log" level = "trace" -[application] -runAsDaemon = true - [raft] pd_addr = "pd0:2379" ignore_databases = "system,default" -# specify which storage engine we use. tmt or dt -storage_engine = "dt" -# Deprecated Raft data storage path setting style. Check [storage.raft] section for new style. -# If it is not set, it will be the first path of "path" appended with "/kvstore". -# kvstore_path = "" - -[raft.snapshot] -# The way to apply snapshot data -# The value is one of "block" / "file1" / "file2". -# method = "file1" [profiles] [profiles.default] From 4988be5e5b4ac3a8f3c0c41c699cda5174ff8538 Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Mon, 30 May 2022 15:12:27 +0800 Subject: [PATCH 3/3] Remove IStorage object from TiFlash context after dropped --- .../tests/gtest_dm_storage_delta_merge.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) 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 index c3573f3d8ca..1cb735a2b65 100644 --- a/dbms/src/Storages/DeltaMerge/tests/gtest_dm_storage_delta_merge.cpp +++ b/dbms/src/Storages/DeltaMerge/tests/gtest_dm_storage_delta_merge.cpp @@ -169,6 +169,8 @@ try } EXPECT_EQ(total_segment_rows, num_rows_read); storage->drop(); + // remove the storage from TiFlash context manually + storage->removeFromTMTContext(); } CATCH @@ -253,6 +255,8 @@ try ASSERT_EQ(storage->getDatabaseName(), new_db_name); storage->drop(); + // remove the storage from TiFlash context manually + storage->removeFromTMTContext(); } CATCH @@ -316,6 +320,8 @@ try ASSERT_EQ(sort_desc.front().nulls_direction, sort_desc2.front().nulls_direction); storage->drop(); + // remove the storage from TiFlash context manually + storage->removeFromTMTContext(); } CATCH @@ -610,7 +616,7 @@ try sample.insert(DB::tests::createColumn( Strings(100, "a"), "col2")); - constexpr TiDB::TableID table_id = 1232; + constexpr TiDB::TableID table_id = 1; const String table_name = fmt::format("t_{}", table_id); Context ctx = DMTestEnv::getContext(); @@ -703,6 +709,8 @@ try in->readSuffix(); ASSERT_EQ(num_rows_read, sample.rows()); storage->drop(); + // remove the storage from TiFlash context manually + storage->removeFromTMTContext(); } CATCH @@ -850,6 +858,8 @@ try ASSERT_LT(read_data(), num_rows_write); } storage->drop(); + // remove the storage from TiFlash context manually + storage->removeFromTMTContext(); } CATCH