From db61d3ef7a6a438e2d6b708633c5c6373f6905ae Mon Sep 17 00:00:00 2001 From: Yingchun Lai <405403881@qq.com> Date: Thu, 15 Oct 2020 19:39:22 +0800 Subject: [PATCH 1/2] [Refactor] Remove objects which are only used for unit test --- be/src/olap/fs/fs_util.cpp | 8 -------- be/src/olap/fs/fs_util.h | 3 --- be/src/olap/page_cache.cpp | 9 ++------- 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 ++- .../segment_v2/bloom_filter_index_reader_writer_test.cpp | 3 ++- .../olap/rowset/segment_v2/column_reader_writer_test.cpp | 5 +++-- .../olap/rowset/segment_v2/ordinal_page_index_test.cpp | 4 +++- be/test/olap/rowset/segment_v2/segment_test.cpp | 7 ++++--- be/test/olap/rowset/segment_v2/zone_map_index_test.cpp | 6 ++++-- 14 files changed, 23 insertions(+), 37 deletions(-) diff --git a/be/src/olap/fs/fs_util.cpp b/be/src/olap/fs/fs_util.cpp index a993e679b412e6..51d2f7f15a6401 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 9cac059d0c110d..c4c9a5e323399b 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 5862337bfe9c7f..f063a45b81a1ea 100644 --- a/be/src/olap/page_cache.cpp +++ b/be/src/olap/page_cache.cpp @@ -19,15 +19,10 @@ 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); - } + _s_instance = new StoragePageCache(capacity); } 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 72ea4c78f03546..d9e9f6c3319083 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 fcc6721569074a..7321358f7aa7f4 100644 --- a/be/src/olap/storage_engine.h +++ b/be/src/olap/storage_engine.h @@ -150,7 +150,6 @@ class StorageEngine { 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 @@ class StorageEngine { 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 4ffcb4e3c99e5c..862f9548ea7845 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 5a4fe34bb85b94..43f0ac65597c07 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 1e06d2e79fe6ce..1202d21a3b498a 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 ff9e27e3f74d48..18ae45ae946d12 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 1022825f077293..1eb4deb73dedb2 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 a004ea0a81f0e4..1f3f664775bbaa 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 6749dec769641c..ae48ca0a7734b3 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 8d8fc89996ce7b..b25a7a00a28d12 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 @@ class SegmentReaderWriterTest : public ::testing::Test { 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 d3c5ff50513ecc..641b13a6564ad6 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 @@ class ColumnZoneMapTest : public testing::Test { { 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(); } From 82e7a25de5ad95c1296ebcd328473e35ba92eeef Mon Sep 17 00:00:00 2001 From: Yingchun Lai <405403881@qq.com> Date: Fri, 16 Oct 2020 02:56:04 +0000 Subject: [PATCH 2/2] fix memory leak suspect --- be/src/olap/page_cache.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/be/src/olap/page_cache.cpp b/be/src/olap/page_cache.cpp index f063a45b81a1ea..f92868b3df8086 100644 --- a/be/src/olap/page_cache.cpp +++ b/be/src/olap/page_cache.cpp @@ -22,7 +22,9 @@ namespace doris { StoragePageCache* StoragePageCache::_s_instance = nullptr; void StoragePageCache::create_global_cache(size_t capacity) { - _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)) {