Skip to content

Commit

Permalink
fix RegionKVStoreTest (pingcap#6768)
Browse files Browse the repository at this point in the history
  • Loading branch information
flowbehappy authored and guo-shaoge committed Feb 10, 2023
1 parent 9f8724e commit 3e8ce03
Show file tree
Hide file tree
Showing 9 changed files with 43 additions and 12 deletions.
2 changes: 1 addition & 1 deletion dbms/src/Debug/MockRaftStoreProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -657,7 +657,7 @@ TableID MockRaftStoreProxy::bootstrap_table(
columns.ordinary = NamesAndTypesList({NameAndTypePair{"a", data_type_factory.get("Int64")}});
auto tso = tmt.getPDClient()->getTS();
MockTiDB::instance().newDataBase("d");
UInt64 table_id = MockTiDB::instance().newTable("d", "t", columns, tso, "", "dt");
UInt64 table_id = MockTiDB::instance().newTable("d", "t" + toString(random()), columns, tso, "", "dt");

auto schema_syncer = tmt.getSchemaSyncer();
schema_syncer->syncSchemas(ctx);
Expand Down
9 changes: 6 additions & 3 deletions dbms/src/Debug/MockStorage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,16 @@ MockColumnInfoVec MockStorage::getTableSchema(const String & name)
}

/// for delta merge
void MockStorage::addTableSchemaForDeltaMerge(const String & name, const MockColumnInfoVec & columnInfos)
Int64 MockStorage::addTableSchemaForDeltaMerge(const String & name, const MockColumnInfoVec & columnInfos)
{
name_to_id_map_for_delta_merge[name] = MockTableIdGenerator::instance().nextTableId();
auto table_id = MockTableIdGenerator::instance().nextTableId();
name_to_id_map_for_delta_merge[name] = table_id;
table_schema_for_delta_merge[getTableIdForDeltaMerge(name)] = columnInfos;
addTableInfoForDeltaMerge(name, columnInfos);
return table_id;
}

void MockStorage::addTableDataForDeltaMerge(Context & context, const String & name, ColumnsWithTypeAndName & columns)
Int64 MockStorage::addTableDataForDeltaMerge(Context & context, const String & name, ColumnsWithTypeAndName & columns)
{
auto table_id = getTableIdForDeltaMerge(name);
addNamesAndTypesForDeltaMerge(table_id, columns);
Expand Down Expand Up @@ -136,6 +138,7 @@ void MockStorage::addTableDataForDeltaMerge(Context & context, const String & na
output->write(insert_block);
output->writeSuffix();
}
return table_id;
}

BlockInputStreamPtr MockStorage::getStreamFromDeltaMerge(Context & context, Int64 table_id, const FilterConditions * filter_conditions)
Expand Down
4 changes: 2 additions & 2 deletions dbms/src/Debug/MockStorage.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,9 @@ class MockStorage
bool tableExists(Int64 table_id);

/// for storage delta merge table scan
void addTableSchemaForDeltaMerge(const String & name, const MockColumnInfoVec & columnInfos);
Int64 addTableSchemaForDeltaMerge(const String & name, const MockColumnInfoVec & columnInfos);

void addTableDataForDeltaMerge(Context & context, const String & name, ColumnsWithTypeAndName & columns);
Int64 addTableDataForDeltaMerge(Context & context, const String & name, ColumnsWithTypeAndName & columns);

MockColumnInfoVec getTableSchemaForDeltaMerge(const String & name);

Expand Down
4 changes: 2 additions & 2 deletions dbms/src/Flash/tests/gtest_mock_storage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ TEST_F(MockStorageTestRunner, DeltaMergeStorageBasic)
try
{
ColumnsWithTypeAndName columns{toVec<Int64>("col0", col0), toNullableVec<String>("col1", col1)};
mock_storage.addTableSchemaForDeltaMerge("test", {{"col0", TiDB::TP::TypeLongLong}, {"col1", TiDB::TP::TypeString}});
auto table_id = mock_storage.addTableSchemaForDeltaMerge("test", {{"col0", TiDB::TP::TypeLongLong}, {"col1", TiDB::TP::TypeString}});
mock_storage.addTableDataForDeltaMerge(context.context, "test", columns);
auto in = mock_storage.getStreamFromDeltaMerge(context.context, 1);
auto in = mock_storage.getStreamFromDeltaMerge(context.context, table_id);

ASSERT_INPUTSTREAM_BLOCK_UR(
in,
Expand Down
8 changes: 8 additions & 0 deletions dbms/src/Storages/Transaction/tests/gtest_new_kvstore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ namespace DB
namespace tests
{
TEST_F(RegionKVStoreTest, KVStoreFailRecovery)
try
{
auto ctx = TiFlashTestEnv::getGlobalContext();
{
Expand Down Expand Up @@ -153,8 +154,10 @@ TEST_F(RegionKVStoreTest, KVStoreFailRecovery)
}
}
}
CATCH

TEST_F(RegionKVStoreTest, KVStoreInvalidWrites)
try
{
auto ctx = TiFlashTestEnv::getGlobalContext();
{
Expand Down Expand Up @@ -188,8 +191,10 @@ TEST_F(RegionKVStoreTest, KVStoreInvalidWrites)
}
}
}
CATCH

TEST_F(RegionKVStoreTest, KVStoreAdminCommands)
try
{
auto ctx = TiFlashTestEnv::getGlobalContext();
{
Expand Down Expand Up @@ -244,8 +249,10 @@ TEST_F(RegionKVStoreTest, KVStoreAdminCommands)
ASSERT_EQ(kvs.handleAdminRaftCmd(raft_cmdpb::AdminRequest{request}, std::move(response), 1999, 22, 6, ctx.getTMTContext()), EngineStoreApplyRes::NotFound);
}
}
CATCH

TEST_F(RegionKVStoreTest, KVStoreSnapshot)
try
{
auto ctx = TiFlashTestEnv::getGlobalContext();
{
Expand Down Expand Up @@ -403,6 +410,7 @@ TEST_F(RegionKVStoreTest, KVStoreSnapshot)
}
}
}
CATCH

} // namespace tests
} // namespace DB
8 changes: 6 additions & 2 deletions dbms/src/Storages/Transaction/tests/kvstore_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ class RegionKVStoreTest : public ::testing::Test
}
void initStorages()
{
bool v = false;
if (!has_init.compare_exchange_strong(v, true))
return;
try
{
registerStorages();
Expand All @@ -123,9 +126,9 @@ class RegionKVStoreTest : public ::testing::Test
}
String path = TiFlashTestEnv::getContext().getPath();
auto p = path + "/metadata/";
TiFlashTestEnv::tryRemovePath(p, /*recreate=*/true);
TiFlashTestEnv::tryCreatePath(p);
p = path + "/data/";
TiFlashTestEnv::tryRemovePath(p, /*recreate=*/true);
TiFlashTestEnv::tryCreatePath(p);
}

protected:
Expand All @@ -150,6 +153,7 @@ class RegionKVStoreTest : public ::testing::Test
return std::make_unique<PathPool>(main_data_paths, main_data_paths, Strings{}, path_capacity, provider);
}

std::atomic_bool has_init{false};
std::string test_path;

std::unique_ptr<PathPool> path_pool;
Expand Down
4 changes: 2 additions & 2 deletions dbms/src/TestUtils/FunctionTestUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ std::multiset<Row> columnsToRowSet(const ColumnsWithTypeAndName & cols)
r.resize(cols_size, true);
}

for (auto const & [col_id, col] : ext::enumerate(cols))
for (auto && [col_id, col] : ext::enumerate(cols))
{
for (size_t i = 0, size = col.column->size(); i < size; ++i)
{
Expand Down Expand Up @@ -300,7 +300,7 @@ ColumnsWithTypeAndName toColumnsWithUniqueName(const ColumnsWithTypeAndName & co
ColumnsWithTypeAndName toColumnsReordered(const ColumnsWithTypeAndName & columns, const ColumnNumbers & new_offsets)
{
ColumnsWithTypeAndName columns_reordered(columns.size());
for (const auto & [i, offset] : ext::enumerate(new_offsets))
for (auto && [i, offset] : ext::enumerate(new_offsets))
{
columns_reordered[offset] = columns[i];
}
Expand Down
14 changes: 14 additions & 0 deletions dbms/src/TestUtils/TiFlashTestEnv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,20 @@ String TiFlashTestEnv::getTemporaryPath(const std::string_view test_case, bool g
return poco_path.toString();
}

void TiFlashTestEnv::tryCreatePath(const std::string & path)
{
try
{
Poco::File p(path);
if (!p.exists())
p.createDirectories();
}
catch (...)
{
tryLogCurrentException("gtest", fmt::format("while removing dir `{}`", path));
}
}

void TiFlashTestEnv::tryRemovePath(const std::string & path, bool recreate)
{
try
Expand Down
2 changes: 2 additions & 0 deletions dbms/src/TestUtils/TiFlashTestEnv.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ class TiFlashTestEnv
public:
static String getTemporaryPath(const std::string_view test_case = "", bool get_abs = true);

static void tryCreatePath(const std::string & path);

static void tryRemovePath(const std::string & path, bool recreate = false);

static std::pair<Strings, Strings> getPathPool(const Strings & testdata_path = {})
Expand Down

0 comments on commit 3e8ce03

Please sign in to comment.