From 45fa67aa71d28528f0c130dbda359525b68f7cfb Mon Sep 17 00:00:00 2001 From: Yingchun Lai <405403881@qq.com> Date: Sun, 18 Oct 2020 21:37:12 +0800 Subject: [PATCH] [Refactor] Remove objects which are only used for unit test (#4751) We create some objects which are only used for unit tests, it's not necessary, and it may cause create duplicate instances for some classes. This patch remove unnecessary instance of class BlockManager and StoragePageCache. --- be/src/olap/fs/fs_util.cpp | 8 -------- be/src/olap/fs/fs_util.h | 3 --- be/src/olap/page_cache.cpp | 11 ++++------- be/src/olap/storage_engine.cpp | 5 ----- be/src/olap/storage_engine.h | 3 --- be/test/agent/cgroups_mgr_test.cpp | 2 +- be/test/olap/rowset/beta_rowset_test.cpp | 1 + be/test/olap/rowset/rowset_converter_test.cpp | 1 + be/test/olap/rowset/segment_v2/bitmap_index_test.cpp | 3 ++- .../bloom_filter_index_reader_writer_test.cpp | 3 ++- .../rowset/segment_v2/column_reader_writer_test.cpp | 5 +++-- .../rowset/segment_v2/ordinal_page_index_test.cpp | 4 +++- be/test/olap/rowset/segment_v2/segment_test.cpp | 7 ++++--- .../olap/rowset/segment_v2/zone_map_index_test.cpp | 6 ++++-- 14 files changed, 25 insertions(+), 37 deletions(-) diff --git a/be/src/olap/fs/fs_util.cpp b/be/src/olap/fs/fs_util.cpp index a993e679b4..51d2f7f15a 100644 --- a/be/src/olap/fs/fs_util.cpp +++ b/be/src/olap/fs/fs_util.cpp @@ -28,14 +28,6 @@ namespace fs { namespace fs_util { BlockManager* block_manager() { -#ifdef BE_TEST - return block_mgr_for_ut(); -#else - return ExecEnv::GetInstance()->storage_engine()->block_manager(); -#endif -} - -BlockManager* block_mgr_for_ut() { fs::BlockManagerOptions bm_opts; bm_opts.read_only = false; static FileBlockManager block_mgr(Env::Default(), std::move(bm_opts)); diff --git a/be/src/olap/fs/fs_util.h b/be/src/olap/fs/fs_util.h index 9cac059d0c..c4c9a5e323 100644 --- a/be/src/olap/fs/fs_util.h +++ b/be/src/olap/fs/fs_util.h @@ -28,9 +28,6 @@ namespace fs_util { // method for each type(instead of a factory method which require same params) BlockManager* block_manager(); -// For UnitTest. -BlockManager* block_mgr_for_ut(); - } // namespace fs_util } // namespace fs } // namespace doris diff --git a/be/src/olap/page_cache.cpp b/be/src/olap/page_cache.cpp index 5862337bfe..f92868b3df 100644 --- a/be/src/olap/page_cache.cpp +++ b/be/src/olap/page_cache.cpp @@ -19,15 +19,12 @@ namespace doris { -// This should only be used in unit test. 1GB -static StoragePageCache s_ut_cache(1073741824); - -StoragePageCache* StoragePageCache::_s_instance = &s_ut_cache; +StoragePageCache* StoragePageCache::_s_instance = nullptr; void StoragePageCache::create_global_cache(size_t capacity) { - if (_s_instance == &s_ut_cache) { - _s_instance = new StoragePageCache(capacity); - } + DCHECK(_s_instance == nullptr); + static StoragePageCache instance(capacity); + _s_instance = &instance; } StoragePageCache::StoragePageCache(size_t capacity) : _cache(new_lru_cache(capacity)) { diff --git a/be/src/olap/storage_engine.cpp b/be/src/olap/storage_engine.cpp index 9a3f12503a..ade3719380 100644 --- a/be/src/olap/storage_engine.cpp +++ b/be/src/olap/storage_engine.cpp @@ -119,7 +119,6 @@ StorageEngine::StorageEngine(const EngineOptions& options) _txn_manager(new TxnManager(config::txn_map_shard_size, config::txn_shard_size)), _rowset_id_generator(new UniqueRowsetIdGenerator(options.backend_uid)), _memtable_flush_executor(nullptr), - _block_manager(nullptr), _default_rowset_type(ALPHA_ROWSET), _heartbeat_flags(nullptr) { if (_s_instance == nullptr) { @@ -181,10 +180,6 @@ Status StorageEngine::_open() { _memtable_flush_executor.reset(new MemTableFlushExecutor()); _memtable_flush_executor->init(dirs); - fs::BlockManagerOptions bm_opts; - bm_opts.read_only = false; - _block_manager.reset(new fs::FileBlockManager(Env::Default(), std::move(bm_opts))); - _parse_default_rowset_type(); return Status::OK(); diff --git a/be/src/olap/storage_engine.h b/be/src/olap/storage_engine.h index fcc6721569..7321358f7a 100644 --- a/be/src/olap/storage_engine.h +++ b/be/src/olap/storage_engine.h @@ -150,7 +150,6 @@ public: TabletManager* tablet_manager() { return _tablet_manager.get(); } TxnManager* txn_manager() { return _txn_manager.get(); } MemTableFlushExecutor* memtable_flush_executor() { return _memtable_flush_executor.get(); } - fs::BlockManager* block_manager() { return _block_manager.get(); } bool check_rowset_id_in_unused_rowsets(const RowsetId& rowset_id); @@ -336,8 +335,6 @@ private: std::unique_ptr _memtable_flush_executor; - std::unique_ptr _block_manager; - // Used to control the migration from segment_v1 to segment_v2, can be deleted in futrue. // Type of new loaded data RowsetTypePB _default_rowset_type; diff --git a/be/test/agent/cgroups_mgr_test.cpp b/be/test/agent/cgroups_mgr_test.cpp index 4ffcb4e3c9..862f9548ea 100644 --- a/be/test/agent/cgroups_mgr_test.cpp +++ b/be/test/agent/cgroups_mgr_test.cpp @@ -40,7 +40,7 @@ class CgroupsMgrTest : public testing::Test { public: // create a mock cgroup folder static void SetUpTestCase() { - ASSERT_FALSE(boost::filesystem::exists(_s_cgroup_path)); + ASSERT_TRUE(boost::filesystem::remove_all(_s_cgroup_path)); // create a mock cgroup path ASSERT_TRUE(boost::filesystem::create_directory(_s_cgroup_path)); diff --git a/be/test/olap/rowset/beta_rowset_test.cpp b/be/test/olap/rowset/beta_rowset_test.cpp index 5a4fe34bb8..43f0ac6559 100644 --- a/be/test/olap/rowset/beta_rowset_test.cpp +++ b/be/test/olap/rowset/beta_rowset_test.cpp @@ -351,6 +351,7 @@ TEST_F(BetaRowsetTest, BasicFunctionTest) { } // namespace doris int main(int argc, char **argv) { + doris::StoragePageCache::create_global_cache(1<<30); ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); } diff --git a/be/test/olap/rowset/rowset_converter_test.cpp b/be/test/olap/rowset/rowset_converter_test.cpp index 1e06d2e79f..1202d21a3b 100644 --- a/be/test/olap/rowset/rowset_converter_test.cpp +++ b/be/test/olap/rowset/rowset_converter_test.cpp @@ -293,6 +293,7 @@ TEST_F(RowsetConverterTest, TestConvertBetaRowsetToAlpha) { } // namespace doris int main(int argc, char **argv) { + doris::StoragePageCache::create_global_cache(1<<30); ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); } diff --git a/be/test/olap/rowset/segment_v2/bitmap_index_test.cpp b/be/test/olap/rowset/segment_v2/bitmap_index_test.cpp index ff9e27e3f7..18ae45ae94 100644 --- a/be/test/olap/rowset/segment_v2/bitmap_index_test.cpp +++ b/be/test/olap/rowset/segment_v2/bitmap_index_test.cpp @@ -65,7 +65,7 @@ void write_index_file(std::string& filename, const void* values, { std::unique_ptr wblock; fs::CreateBlockOptions opts({ filename }); - ASSERT_TRUE(fs::fs_util::block_mgr_for_ut()->create_block(opts, &wblock).ok()); + ASSERT_TRUE(fs::fs_util::block_manager()->create_block(opts, &wblock).ok()); std::unique_ptr writer; BitmapIndexWriter::create(type_info, &writer); @@ -238,6 +238,7 @@ TEST_F(BitmapIndexTest, test_null) { } int main(int argc, char** argv) { + doris::StoragePageCache::create_global_cache(1<<30); ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); } diff --git a/be/test/olap/rowset/segment_v2/bloom_filter_index_reader_writer_test.cpp b/be/test/olap/rowset/segment_v2/bloom_filter_index_reader_writer_test.cpp index 1022825f07..1eb4deb73d 100644 --- a/be/test/olap/rowset/segment_v2/bloom_filter_index_reader_writer_test.cpp +++ b/be/test/olap/rowset/segment_v2/bloom_filter_index_reader_writer_test.cpp @@ -62,7 +62,7 @@ void write_bloom_filter_index_file(const std::string& file_name, const void* val { std::unique_ptr wblock; fs::CreateBlockOptions opts({ fname }); - Status st = fs::fs_util::block_mgr_for_ut()->create_block(opts, &wblock); + Status st = fs::fs_util::block_manager()->create_block(opts, &wblock); ASSERT_TRUE(st.ok()) << st.to_string(); std::unique_ptr bloom_filter_index_writer; @@ -285,6 +285,7 @@ TEST_F(BloomFilterIndexReaderWriterTest, test_decimal) { } int main(int argc, char** argv) { + doris::StoragePageCache::create_global_cache(1<<30); ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); } diff --git a/be/test/olap/rowset/segment_v2/column_reader_writer_test.cpp b/be/test/olap/rowset/segment_v2/column_reader_writer_test.cpp index a004ea0a81..1f3f664775 100644 --- a/be/test/olap/rowset/segment_v2/column_reader_writer_test.cpp +++ b/be/test/olap/rowset/segment_v2/column_reader_writer_test.cpp @@ -77,7 +77,7 @@ void test_nullable_data(uint8_t* src_data, uint8_t* src_is_null, int num_rows, s { std::unique_ptr wblock; fs::CreateBlockOptions opts({ fname }); - Status st = fs::fs_util::block_mgr_for_ut()->create_block(opts, &wblock); + Status st = fs::fs_util::block_manager()->create_block(opts, &wblock); ASSERT_TRUE(st.ok()) << st.get_error_msg(); ColumnWriterOptions writer_opts; @@ -131,7 +131,7 @@ void test_nullable_data(uint8_t* src_data, uint8_t* src_is_null, int num_rows, s st = reader->new_iterator(&iter); ASSERT_TRUE(st.ok()); std::unique_ptr rblock; - fs::BlockManager* block_manager = fs::fs_util::block_mgr_for_ut(); + fs::BlockManager* block_manager = fs::fs_util::block_manager(); block_manager->open_block(fname, &rblock); ASSERT_TRUE(st.ok()); @@ -445,6 +445,7 @@ TEST_F(ColumnReaderWriterTest, test_default_value) { } // namespace doris int main(int argc, char** argv) { + doris::StoragePageCache::create_global_cache(1<<30); ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); } diff --git a/be/test/olap/rowset/segment_v2/ordinal_page_index_test.cpp b/be/test/olap/rowset/segment_v2/ordinal_page_index_test.cpp index 6749dec769..ae48ca0a77 100644 --- a/be/test/olap/rowset/segment_v2/ordinal_page_index_test.cpp +++ b/be/test/olap/rowset/segment_v2/ordinal_page_index_test.cpp @@ -25,6 +25,7 @@ #include "common/logging.h" #include "env/env.h" #include "olap/fs/fs_util.h" +#include "olap/page_cache.h" #include "util/file_utils.h" namespace doris { @@ -61,7 +62,7 @@ TEST_F(OrdinalPageIndexTest, normal) { { std::unique_ptr wblock; fs::CreateBlockOptions opts({ filename }); - ASSERT_TRUE(fs::fs_util::block_mgr_for_ut()->create_block(opts, &wblock).ok()); + ASSERT_TRUE(fs::fs_util::block_manager()->create_block(opts, &wblock).ok()); ASSERT_TRUE(builder.finish(wblock.get(), &index_meta).ok()); ASSERT_EQ(ORDINAL_INDEX, index_meta.type()); @@ -157,6 +158,7 @@ TEST_F(OrdinalPageIndexTest, one_data_page) { } int main(int argc, char** argv) { + doris::StoragePageCache::create_global_cache(1<<30); ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); } diff --git a/be/test/olap/rowset/segment_v2/segment_test.cpp b/be/test/olap/rowset/segment_v2/segment_test.cpp index 8d8fc89996..b25a7a00a2 100644 --- a/be/test/olap/rowset/segment_v2/segment_test.cpp +++ b/be/test/olap/rowset/segment_v2/segment_test.cpp @@ -110,7 +110,7 @@ protected: string filename = strings::Substitute("$0/seg_$1.dat", kSegmentDir, seg_id++); std::unique_ptr wblock; fs::CreateBlockOptions block_opts({ filename }); - Status st = fs::fs_util::block_mgr_for_ut()->create_block(block_opts, &wblock); + Status st = fs::fs_util::block_manager()->create_block(block_opts, &wblock); ASSERT_TRUE(st.ok()); SegmentWriter writer(wblock.get(), 0, &build_schema, opts); st = writer.init(10); @@ -609,7 +609,7 @@ TEST_F(SegmentReaderWriterTest, estimate_segment_size) { std::string fname = dname + "/int_case"; std::unique_ptr wblock; fs::CreateBlockOptions wblock_opts({ fname }); - Status st = fs::fs_util::block_mgr_for_ut()->create_block(wblock_opts, &wblock); + Status st = fs::fs_util::block_manager()->create_block(wblock_opts, &wblock); ASSERT_TRUE(st.ok()); SegmentWriter writer(wblock.get(), 0, tablet_schema.get(), opts); st = writer.init(10); @@ -775,7 +775,7 @@ TEST_F(SegmentReaderWriterTest, TestStringDict) { std::string fname = dname + "/string_case"; std::unique_ptr wblock; fs::CreateBlockOptions wblock_opts({ fname }); - Status st = fs::fs_util::block_mgr_for_ut()->create_block(wblock_opts, &wblock); + Status st = fs::fs_util::block_manager()->create_block(wblock_opts, &wblock); ASSERT_TRUE(st.ok()); SegmentWriter writer(wblock.get(), 0, tablet_schema.get(), opts); st = writer.init(10); @@ -1139,6 +1139,7 @@ TEST_F(SegmentReaderWriterTest, TestBloomFilterIndexUniqueModel) { } int main(int argc, char** argv) { + doris::StoragePageCache::create_global_cache(1<<30); ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); } diff --git a/be/test/olap/rowset/segment_v2/zone_map_index_test.cpp b/be/test/olap/rowset/segment_v2/zone_map_index_test.cpp index d3c5ff5051..641b13a656 100644 --- a/be/test/olap/rowset/segment_v2/zone_map_index_test.cpp +++ b/be/test/olap/rowset/segment_v2/zone_map_index_test.cpp @@ -23,6 +23,7 @@ #include "env/env.h" #include "olap/fs/block_manager.h" #include "olap/fs/fs_util.h" +#include "olap/page_cache.h" #include "olap/rowset/segment_v2/zone_map_index.h" #include "olap/tablet_schema_helper.h" #include "util/file_utils.h" @@ -72,7 +73,7 @@ public: { std::unique_ptr wblock; fs::CreateBlockOptions opts({ filename }); - ASSERT_TRUE(fs::fs_util::block_mgr_for_ut()->create_block(opts, &wblock).ok()); + ASSERT_TRUE(fs::fs_util::block_manager()->create_block(opts, &wblock).ok()); ASSERT_TRUE(builder.finish(wblock.get(), &index_meta).ok()); ASSERT_EQ(ZONE_MAP_INDEX, index_meta.type()); ASSERT_TRUE(wblock->close().ok()); @@ -125,7 +126,7 @@ TEST_F(ColumnZoneMapTest, NormalTestIntPage) { { std::unique_ptr wblock; fs::CreateBlockOptions opts({ filename }); - ASSERT_TRUE(fs::fs_util::block_mgr_for_ut()->create_block(opts, &wblock).ok()); + ASSERT_TRUE(fs::fs_util::block_manager()->create_block(opts, &wblock).ok()); ASSERT_TRUE(builder.finish(wblock.get(), &index_meta).ok()); ASSERT_EQ(ZONE_MAP_INDEX, index_meta.type()); ASSERT_TRUE(wblock->close().ok()); @@ -173,6 +174,7 @@ TEST_F(ColumnZoneMapTest, NormalTestCharPage) { } int main(int argc, char** argv) { + doris::StoragePageCache::create_global_cache(1<<30); testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); }