[BugFix] Fix primary key table add an visible rowset (#54491)

Signed-off-by: sevev <[email protected]> (cherry picked from commit 15aea9e)
StarRocks · Dec 31, 2024 · f6c762d · f6c762d
1 parent 1b0a9a5
commit f6c762d
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 14 deletions.
diff --git a/be/src/storage/data_dir.cpp b/be/src/storage/data_dir.cpp
@@ -415,17 +415,22 @@ Status DataDir::load() {
 
         } else if (rowset_meta->rowset_state() == RowsetStatePB::VISIBLE &&
                    rowset_meta->tablet_uid() == tablet->tablet_uid()) {
-            Status publish_status = tablet->load_rowset(rowset);
-            if (!rowset_meta->tablet_schema()) {
-                rowset_meta->set_tablet_schema(tablet->tablet_schema());
-                rowset_meta->set_skip_tablet_schema(true);
-            }
-            if (!publish_status.ok() && !publish_status.is_already_exist()) {
-                LOG(WARNING) << "Fail to add visible rowset=" << rowset->rowset_id()
-                             << " to tablet=" << rowset_meta->tablet_id() << " txn id=" << rowset_meta->txn_id()
-                             << " start version=" << rowset_meta->version().first
-                             << " end version=" << rowset_meta->version().second;
-                error_rowset_count++;
+            if (tablet->keys_type() == KeysType::PRIMARY_KEYS) {
+                VLOG(1) << "skip a visible rowset meta, tablet: " << tablet->tablet_id()
+                        << ", rowset: " << rowset_meta->rowset_id();
+            } else {
+                Status publish_status = tablet->load_rowset(rowset);
+                if (!rowset_meta->tablet_schema()) {
+                    rowset_meta->set_tablet_schema(tablet->tablet_schema());
+                    rowset_meta->set_skip_tablet_schema(true);
+                }
+                if (!publish_status.ok() && !publish_status.is_already_exist()) {
+                    LOG(WARNING) << "Fail to add visible rowset=" << rowset->rowset_id()
+                                 << " to tablet=" << rowset_meta->tablet_id() << " txn id=" << rowset_meta->txn_id()
+                                 << " start version=" << rowset_meta->version().first
+                                 << " end version=" << rowset_meta->version().second;
+                    error_rowset_count++;
+                }
             }
         } else {
             LOG(WARNING) << "Found invalid rowset=" << rowset_meta->rowset_id()

diff --git a/be/src/storage/tablet.cpp b/be/src/storage/tablet.cpp
@@ -1827,6 +1827,7 @@ const TabletSchemaCSPtr Tablet::thread_safe_get_tablet_schema() const {
     return _max_version_schema;
 }
 
+DEFINE_FAIL_POINT(tablet_get_visible_rowset);
 // for non-pk tablet, all published rowset will be rewrite when save `tablet_meta`
 // for pk tablet, we need to get the rowset which without `tablet_schema` and rewrite
 // the rowsets in `_committed_rs_map` is committed success but not publish yet, so if we update the
@@ -1841,6 +1842,14 @@ void Tablet::_get_rewrite_meta_rs(std::vector<RowsetSharedPtr>& rewrite_meta_rs)
     if (_updates) {
         _updates->rewrite_rs_meta(true);
     }
+    FAIL_POINT_TRIGGER_EXECUTE(tablet_get_visible_rowset, {
+        if (_updates) {
+            auto rowset_map = _updates->get_rowset_map();
+            for (const auto& [_, rs] : (*rowset_map)) {
+                rewrite_meta_rs.emplace_back(rs);
+            }
+        }
+    });
 }
 
 void Tablet::update_max_version_schema(const TabletSchemaCSPtr& tablet_schema) {
@@ -1857,7 +1866,8 @@ void Tablet::update_max_version_schema(const TabletSchemaCSPtr& tablet_schema) {
         }
         std::vector<RowsetSharedPtr> rewrite_meta_rs;
         _get_rewrite_meta_rs(rewrite_meta_rs);
-        _tablet_meta->save_tablet_schema(_max_version_schema, rewrite_meta_rs, _data_dir);
+        _tablet_meta->save_tablet_schema(_max_version_schema, rewrite_meta_rs, _data_dir,
+                                         _max_version_schema->keys_type() == KeysType::PRIMARY_KEYS);
         _committed_rs_map.clear();
     }
 }

diff --git a/be/src/storage/tablet_meta.cpp b/be/src/storage/tablet_meta.cpp
@@ -203,12 +203,16 @@ Status TabletMeta::save_meta(DataDir* data_dir, bool skip_tablet_schema) {
 }
 
 void TabletMeta::save_tablet_schema(const TabletSchemaCSPtr& tablet_schema, std::vector<RowsetSharedPtr>& committed_rs,
-                                    DataDir* data_dir) {
+                                    DataDir* data_dir, bool is_primary_key) {
     std::unique_lock wrlock(_meta_lock);
     _schema = tablet_schema;
     for (auto& rs : committed_rs) {
         RowsetMetaPB meta_pb;
         rs->rowset_meta()->get_full_meta_pb(&meta_pb);
+        if (is_primary_key && rs->rowset_meta()->rowset_state() == RowsetStatePB::VISIBLE) {
+            LOG(INFO) << "skip visible rowset: " << rs->rowset_meta()->rowset_id() << " of tablet: " << tablet_id();
+            continue;
+        }
         Status res = RowsetMetaManager::save(data_dir->get_meta(), tablet_uid(), meta_pb);
         LOG_IF(FATAL, !res.ok()) << "failed to save rowset " << rs->rowset_id() << " to local meta store: " << res;
         rs->rowset_meta()->set_skip_tablet_schema(false);

diff --git a/be/src/storage/tablet_meta.h b/be/src/storage/tablet_meta.h
@@ -165,7 +165,7 @@ class TabletMeta {
 
     void set_tablet_schema(const TabletSchemaCSPtr& tablet_schema) { _schema = tablet_schema; }
     void save_tablet_schema(const TabletSchemaCSPtr& tablet_schema, std::vector<RowsetSharedPtr>& committed_rs,
-                            DataDir* data_dir);
+                            DataDir* data_dir, bool is_primary_key);
 
     TabletSchemaCSPtr& tablet_schema_ptr() { return _schema; }
     const TabletSchemaCSPtr& tablet_schema_ptr() const { return _schema; }

diff --git a/be/test/storage/tablet_updates_test.cpp b/be/test/storage/tablet_updates_test.cpp
@@ -3919,6 +3919,20 @@ TEST_F(TabletUpdatesTest, test_skip_schema) {
         ASSERT_EQ(rs2->tablet_schema()->id(), old_schema_id);
         ASSERT_EQ(rs1->tablet_schema()->id(), old_schema_id);
     }
+
+    {
+        PFailPointTriggerMode trigger_mode;
+        trigger_mode.set_mode(FailPointTriggerModeType::ENABLE);
+        std::string fp_name = "tablet_get_visible_rowset";
+        auto fp = starrocks::failpoint::FailPointRegistry::GetInstance()->get(fp_name);
+        fp->setMode(trigger_mode);
+
+        auto tmp_tablet1 = create_tablet(rand(), rand(), false, _tablet->tablet_schema()->id() + 1,
+                                         _tablet->tablet_schema()->schema_version() + 1);
+        _tablet->update_max_version_schema(tmp_tablet1->tablet_schema());
+        trigger_mode.set_mode(FailPointTriggerModeType::DISABLE);
+        fp->setMode(trigger_mode);
+    }
 }
 
 } // namespace starrocks