You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by mo...@apache.org on 2020/10/18 13:37:29 UTC

[incubator-doris] branch master updated: [Refactor] Remove objects which are only used for unit test (#4751)

This is an automated email from the ASF dual-hosted git repository.

morningman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-doris.git


The following commit(s) were added to refs/heads/master by this push:
     new 45fa67a  [Refactor] Remove objects which are only used for unit test (#4751)
45fa67a is described below

commit 45fa67aa71d28528f0c130dbda359525b68f7cfb
Author: Yingchun Lai <40...@qq.com>
AuthorDate: Sun Oct 18 21:37:12 2020 +0800

    [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 ++-
 .../segment_v2/bloom_filter_index_reader_writer_test.cpp      |  3 ++-
 be/test/olap/rowset/segment_v2/column_reader_writer_test.cpp  |  5 +++--
 be/test/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, 25 insertions(+), 37 deletions(-)

diff --git a/be/src/olap/fs/fs_util.cpp b/be/src/olap/fs/fs_util.cpp
index a993e67..51d2f7f 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 9cac059..c4c9a5e 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 5862337..f92868b 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 9a3f125..ade3719 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 fcc6721..7321358 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<MemTableFlushExecutor> _memtable_flush_executor;
 
-    std::unique_ptr<fs::BlockManager> _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 4ffcb4e..862f954 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 5a4fe34..43f0ac6 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 1e06d2e..1202d21 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 ff9e27e..18ae45a 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<fs::WritableBlock> 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<BitmapIndexWriter> 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 1022825..1eb4deb 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<fs::WritableBlock> 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<BloomFilterIndexWriter> 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 a004ea0..1f3f664 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<fs::WritableBlock> 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<fs::ReadableBlock> 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 6749dec..ae48ca0 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<fs::WritableBlock> 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 8d8fc89..b25a7a0 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<fs::WritableBlock> 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<fs::WritableBlock> 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<fs::WritableBlock> 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 d3c5ff5..641b13a 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<fs::WritableBlock> 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<fs::WritableBlock> 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();
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org