[Enhancement] Optimize segment open for cloud native tablet (StarRock…

…s#32996) Signed-off-by: wyb <wybb86@gmail.com>
willWang8023 · Oct 18, 2023 · 965e880 · 965e880
1 parent 166da72
commit 965e880
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 27 deletions.
diff --git a/be/src/storage/lake/tablet.cpp b/be/src/storage/lake/tablet.cpp
@@ -147,18 +147,20 @@ StatusOr<std::vector<RowsetPtr>> Tablet::get_rowsets(const TabletMetadata& metad
 
 StatusOr<SegmentPtr> Tablet::load_segment(std::string_view segment_name, int seg_id, size_t* footer_size_hint,
                                           bool fill_data_cache, bool fill_metadata_cache) {
-    auto location = segment_location(segment_name);
-    auto segment = _mgr->lookup_segment(location);
-    if (segment != nullptr) {
-        return segment;
-    }
-    ASSIGN_OR_RETURN(auto tablet_schema, get_schema());
-    ASSIGN_OR_RETURN(auto fs, FileSystem::CreateSharedFromString(location));
-    ASSIGN_OR_RETURN(segment, Segment::open(fs, location, seg_id, std::move(tablet_schema), footer_size_hint, nullptr,
-                                            !fill_data_cache, _mgr));
-    if (fill_metadata_cache) {
-        _mgr->cache_segment(location, segment);
+    auto segment_path = segment_location(segment_name);
+    auto segment = _mgr->lookup_segment(segment_path);
+    if (segment == nullptr) {
+        ASSIGN_OR_RETURN(auto tablet_schema, get_schema());
+        ASSIGN_OR_RETURN(auto fs, FileSystem::CreateSharedFromString(segment_path));
+        segment = std::make_shared<Segment>(std::move(fs), segment_path, seg_id, std::move(tablet_schema), _mgr);
+        if (fill_metadata_cache) {
+            _mgr->cache_segment(segment_path, segment);
+        }
     }
+    // segment->open will read the footer, and it is time-consuming.
+    // separate it from static Segment::open is to prevent a large number of cache misses,
+    // and many temporary segment objects generation when loading the same segment concurrently.
+    RETURN_IF_ERROR(segment->open(footer_size_hint, nullptr, !fill_data_cache));
     return segment;
 }
 

diff --git a/be/src/storage/rowset/segment.cpp b/be/src/storage/rowset/segment.cpp
@@ -77,10 +77,8 @@ StatusOr<std::shared_ptr<Segment>> Segment::open(std::shared_ptr<FileSystem> fs,
                                                  size_t* footer_length_hint,
                                                  const FooterPointerPB* partial_rowset_footer,
                                                  bool skip_fill_local_cache, lake::TabletManager* tablet_manager) {
-    auto segment = std::make_shared<Segment>(private_type(0), std::move(fs), path, segment_id, std::move(tablet_schema),
-                                             tablet_manager);
-
-    RETURN_IF_ERROR(segment->_open(footer_length_hint, partial_rowset_footer, skip_fill_local_cache));
+    auto segment = std::make_shared<Segment>(std::move(fs), path, segment_id, std::move(tablet_schema), tablet_manager);
+    RETURN_IF_ERROR(segment->open(footer_length_hint, partial_rowset_footer, skip_fill_local_cache));
     return std::move(segment);
 }
 
@@ -174,8 +172,8 @@ Status Segment::parse_segment_footer(RandomAccessFile* read_file, SegmentFooterP
     return Status::OK();
 }
 
-Segment::Segment(const private_type&, std::shared_ptr<FileSystem> fs, std::string path, uint32_t segment_id,
-                 TabletSchemaCSPtr tablet_schema, lake::TabletManager* tablet_manager)
+Segment::Segment(std::shared_ptr<FileSystem> fs, std::string path, uint32_t segment_id, TabletSchemaCSPtr tablet_schema,
+                 lake::TabletManager* tablet_manager)
         : _fs(std::move(fs)),
           _fname(std::move(path)),
           _tablet_schema(std::move(tablet_schema)),
@@ -189,13 +187,28 @@ Segment::~Segment() {
     MEM_TRACKER_SAFE_RELEASE(GlobalEnv::GetInstance()->short_key_index_mem_tracker(), _short_key_index_mem_usage());
 }
 
+Status Segment::open(size_t* footer_length_hint, const FooterPointerPB* partial_rowset_footer,
+                     bool skip_fill_local_cache) {
+    if (invoked(_open_once)) {
+        return Status::OK();
+    }
+
+    auto res = success_once(_open_once, [&] {
+        Status st = _open(footer_length_hint, partial_rowset_footer, skip_fill_local_cache);
+        if (st.ok()) {
+            update_cache_size();
+        }
+        return st;
+    });
+    return res.status();
+}
+
 Status Segment::_open(size_t* footer_length_hint, const FooterPointerPB* partial_rowset_footer,
                       bool skip_fill_local_cache) {
     SegmentFooterPB footer;
     RandomAccessFileOptions opts{.skip_fill_local_cache = skip_fill_local_cache};
     ASSIGN_OR_RETURN(auto read_file, _fs->new_random_access_file(opts, _fname));
     RETURN_IF_ERROR(Segment::parse_segment_footer(read_file.get(), &footer, footer_length_hint, partial_rowset_footer));
-
     RETURN_IF_ERROR(_create_column_readers(&footer));
     _num_rows = footer.num_rows();
     _short_key_index_page = PagePointer(footer.short_key_index_page());

diff --git a/be/src/storage/rowset/segment.h b/be/src/storage/rowset/segment.h
@@ -80,10 +80,6 @@ using ChunkIteratorPtr = std::shared_ptr<ChunkIterator>;
 // is changed, this segment can not be used any more. For example, after a schema
 // change finished, client should disable all cached Segment for old TabletSchema.
 class Segment : public std::enable_shared_from_this<Segment> {
-    struct private_type {
-        explicit private_type(int) {}
-    };
-
 public:
     // Like above but share the ownership of |unsafe_tablet_schema_ref|.
     static StatusOr<std::shared_ptr<Segment>> open(std::shared_ptr<FileSystem> fs, const std::string& path,
@@ -97,11 +93,13 @@ class Segment : public std::enable_shared_from_this<Segment> {
                                                      size_t* footer_length_hint,
                                                      const FooterPointerPB* partial_rowset_footer);
 
-    Segment(const private_type&, std::shared_ptr<FileSystem> fs, std::string path, uint32_t segment_id,
-            TabletSchemaCSPtr tablet_schema, lake::TabletManager* tablet_manager);
+    Segment(std::shared_ptr<FileSystem> fs, std::string path, uint32_t segment_id, TabletSchemaCSPtr tablet_schema,
+            lake::TabletManager* tablet_manager);
 
     ~Segment();
 
+    Status open(size_t* footer_length_hint, const FooterPointerPB* partial_rowset_footer, bool skip_fill_local_cache);
+
     // may return EndOfFile
     StatusOr<ChunkIteratorPtr> new_iterator(const Schema& schema, const SegmentReadOptions& read_options);
 
@@ -272,6 +270,8 @@ class Segment : public std::enable_shared_from_this<Segment> {
 
     // for cloud native tablet
     lake::TabletManager* _tablet_manager = nullptr;
+    // used to guarantee that segment will be opened at most once in a thread-safe way
+    OnceFlag _open_once;
 };
 
 } // namespace starrocks
diff --git a/be/test/storage/rowset/column_reader_writer_test.cpp b/be/test/storage/rowset/column_reader_writer_test.cpp
@@ -99,7 +99,7 @@ class ColumnReaderWriterTest : public testing::Test {
     void TearDown() override {}
 
     std::shared_ptr<Segment> create_dummy_segment(const std::shared_ptr<FileSystem>& fs, const std::string& fname) {
-        return std::make_shared<Segment>(Segment::private_type(0), fs, fname, 1, _dummy_segment_schema, nullptr);
+        return std::make_shared<Segment>(fs, fname, 1, _dummy_segment_schema, nullptr);
     }
 
     template <LogicalType type, EncodingTypePB encoding, uint32_t version>

diff --git a/be/test/storage/rowset/map_column_rw_test.cpp b/be/test/storage/rowset/map_column_rw_test.cpp
@@ -48,7 +48,7 @@ class MapColumnRWTest : public testing::Test {
     void TearDown() override {}
 
     std::shared_ptr<Segment> create_dummy_segment(const std::shared_ptr<FileSystem>& fs, const std::string& fname) {
-        return std::make_shared<Segment>(Segment::private_type(0), fs, fname, 1, _dummy_segment_schema, nullptr);
+        return std::make_shared<Segment>(fs, fname, 1, _dummy_segment_schema, nullptr);
     }
 
     void test_int_map() {

diff --git a/be/test/storage/rowset/struct_column_rw_test.cpp b/be/test/storage/rowset/struct_column_rw_test.cpp
@@ -50,7 +50,7 @@ class StructColumnRWTest : public testing::Test {
     void TearDown() override {}
 
     std::shared_ptr<Segment> create_dummy_segment(const std::shared_ptr<FileSystem>& fs, const std::string& fname) {
-        return std::make_shared<Segment>(Segment::private_type(0), fs, fname, 1, _dummy_segment_schema, nullptr);
+        return std::make_shared<Segment>(fs, fname, 1, _dummy_segment_schema, nullptr);
     }
 
     void test_int_struct() {