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

kxiao pushed a commit to branch branch-2.0
in repository https://gitbox.apache.org/repos/asf/doris.git


The following commit(s) were added to refs/heads/branch-2.0 by this push:
     new 26dd70a0181 [enhance](partitionid) check partition id to avoid 
unexpected behavior (#28045)
26dd70a0181 is described below

commit 26dd70a018193969696120f30d717c24ff58fe44
Author: Yongqiang YANG <98214048+dataroar...@users.noreply.github.com>
AuthorDate: Wed Dec 6 19:40:11 2023 +0800

    [enhance](partitionid) check partition id to avoid unexpected behavior 
(#28045)
---
 be/src/olap/data_dir.cpp                            | 17 ++++++++++++-----
 be/src/olap/rowset/rowset_meta_manager.cpp          |  6 ++++++
 be/src/olap/tablet.cpp                              |  5 +++++
 be/src/olap/tablet_meta_manager.cpp                 |  6 ++++++
 be/src/olap/txn_manager.cpp                         |  7 ++++---
 be/test/olap/delete_handler_test.cpp                |  2 ++
 be/test/olap/delta_writer_test.cpp                  |  2 ++
 be/test/olap/engine_storage_migration_task_test.cpp |  1 +
 be/test/olap/remote_rowset_gc_test.cpp              |  1 +
 be/test/olap/tablet_cooldown_test.cpp               |  1 +
 be/test/olap/tablet_mgr_test.cpp                    |  3 +++
 11 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/be/src/olap/data_dir.cpp b/be/src/olap/data_dir.cpp
index c54998c76d3..47434bd8226 100644
--- a/be/src/olap/data_dir.cpp
+++ b/be/src/olap/data_dir.cpp
@@ -506,15 +506,22 @@ Status DataDir::load() {
                     _meta, rowset_meta->partition_id(), rowset_meta->txn_id(),
                     rowset_meta->tablet_id(), 
rowset_meta->tablet_schema_hash(),
                     rowset_meta->tablet_uid(), rowset_meta->load_id(), rowset, 
true);
-            if (!commit_txn_status && 
!commit_txn_status.is<PUSH_TRANSACTION_ALREADY_EXIST>()) {
-                LOG(WARNING) << "failed to add committed rowset: " << 
rowset_meta->rowset_id()
-                             << " to tablet: " << rowset_meta->tablet_id()
-                             << " for txn: " << rowset_meta->txn_id();
-            } else {
+            if (commit_txn_status || 
commit_txn_status.is<PUSH_TRANSACTION_ALREADY_EXIST>()) {
                 LOG(INFO) << "successfully to add committed rowset: " << 
rowset_meta->rowset_id()
                           << " to tablet: " << rowset_meta->tablet_id()
                           << " schema hash: " << 
rowset_meta->tablet_schema_hash()
                           << " for txn: " << rowset_meta->txn_id();
+            } else if (commit_txn_status.is<ErrorCode::INTERNAL_ERROR>()) {
+                LOG(WARNING) << "failed to add committed rowset: " << 
rowset_meta->rowset_id()
+                             << " to tablet: " << rowset_meta->tablet_id()
+                             << " for txn: " << rowset_meta->txn_id()
+                             << " error: " << commit_txn_status;
+                return commit_txn_status;
+            } else {
+                LOG(WARNING) << "failed to add committed rowset: " << 
rowset_meta->rowset_id()
+                             << " to tablet: " << rowset_meta->tablet_id()
+                             << " for txn: " << rowset_meta->txn_id()
+                             << " error: " << commit_txn_status;
             }
         } else if (rowset_meta->rowset_state() == RowsetStatePB::VISIBLE &&
                    rowset_meta->tablet_uid() == tablet->tablet_uid()) {
diff --git a/be/src/olap/rowset/rowset_meta_manager.cpp 
b/be/src/olap/rowset/rowset_meta_manager.cpp
index f879aaf8995..16bda887c35 100644
--- a/be/src/olap/rowset/rowset_meta_manager.cpp
+++ b/be/src/olap/rowset/rowset_meta_manager.cpp
@@ -91,6 +91,12 @@ Status RowsetMetaManager::get_json_rowset_meta(OlapMeta* 
meta, TabletUid tablet_
 }
 Status RowsetMetaManager::save(OlapMeta* meta, TabletUid tablet_uid, const 
RowsetId& rowset_id,
                                const RowsetMetaPB& rowset_meta_pb, bool 
enable_binlog) {
+    if (rowset_meta_pb.partition_id() <= 0) {
+        LOG(WARNING) << "invalid partition id " << 
rowset_meta_pb.partition_id() << " tablet "
+                     << rowset_meta_pb.tablet_id();
+        return Status::InternalError("invaid partition id {} tablet {}",
+                                     rowset_meta_pb.partition_id(), 
rowset_meta_pb.tablet_id());
+    }
     if (enable_binlog) {
         return _save_with_binlog(meta, tablet_uid, rowset_id, rowset_meta_pb);
     } else {
diff --git a/be/src/olap/tablet.cpp b/be/src/olap/tablet.cpp
index 22276e4041e..0d9f21da326 100644
--- a/be/src/olap/tablet.cpp
+++ b/be/src/olap/tablet.cpp
@@ -242,6 +242,11 @@ void 
WriteCooldownMetaExecutors::WriteCooldownMetaExecutors::submit(TabletShared
 
 TabletSharedPtr Tablet::create_tablet_from_meta(TabletMetaSharedPtr 
tablet_meta,
                                                 DataDir* data_dir) {
+    if (tablet_meta->partition_id() <= 0) {
+        LOG(WARNING) << "invalid partition id " << tablet_meta->partition_id() 
<< ", tablet "
+                     << tablet_meta->tablet_id();
+        return nullptr;
+    }
     return std::make_shared<Tablet>(tablet_meta, data_dir);
 }
 
diff --git a/be/src/olap/tablet_meta_manager.cpp 
b/be/src/olap/tablet_meta_manager.cpp
index 20e5747e2f0..20f3872a03e 100644
--- a/be/src/olap/tablet_meta_manager.cpp
+++ b/be/src/olap/tablet_meta_manager.cpp
@@ -92,6 +92,12 @@ Status TabletMetaManager::save(DataDir* store, TTabletId 
tablet_id, TSchemaHash
     std::string key = fmt::format("{}{}_{}", header_prefix, tablet_id, 
schema_hash);
     std::string value;
     tablet_meta->serialize(&value);
+    if (tablet_meta->partition_id() <= 0) {
+        LOG(WARNING) << "invalid partition id " << tablet_meta->partition_id() 
<< " tablet "
+                     << tablet_meta->tablet_id();
+        return Status::InternalError("invaid partition id {} tablet {}",
+                                     tablet_meta->partition_id(), 
tablet_meta->tablet_id());
+    }
     OlapMeta* meta = store->get_meta();
     VLOG_NOTICE << "save tablet meta"
                 << ", key:" << key << ", meta length:" << value.length();
diff --git a/be/src/olap/txn_manager.cpp b/be/src/olap/txn_manager.cpp
index 4dbe317ad1f..23efbcd1a73 100644
--- a/be/src/olap/txn_manager.cpp
+++ b/be/src/olap/txn_manager.cpp
@@ -257,9 +257,10 @@ Status TxnManager::commit_txn(OlapMeta* meta, TPartitionId 
partition_id,
                               const PUniqueId& load_id, const RowsetSharedPtr& 
rowset_ptr,
                               bool is_recovery) {
     if (partition_id < 1 || transaction_id < 1 || tablet_id < 1) {
-        LOG(FATAL) << "invalid commit req "
-                   << " partition_id=" << partition_id << " transaction_id=" 
<< transaction_id
-                   << " tablet_id=" << tablet_id;
+        LOG(WARNING) << "invalid commit req "
+                     << " partition_id=" << partition_id << " transaction_id=" 
<< transaction_id
+                     << " tablet_id=" << tablet_id;
+        return Status::InternalError("invalid partition id");
     }
 
     pair<int64_t, int64_t> key(partition_id, transaction_id);
diff --git a/be/test/olap/delete_handler_test.cpp 
b/be/test/olap/delete_handler_test.cpp
index 792fc18f78c..3029220903f 100644
--- a/be/test/olap/delete_handler_test.cpp
+++ b/be/test/olap/delete_handler_test.cpp
@@ -100,6 +100,7 @@ static void tear_down() {
 static void set_default_create_tablet_request(TCreateTabletReq* request) {
     request->tablet_id = 10003;
     request->__set_version(1);
+    request->partition_id = 10004;
     request->tablet_schema.schema_hash = 270068375;
     request->tablet_schema.short_key_column_count = 2;
     request->tablet_schema.keys_type = TKeysType::AGG_KEYS;
@@ -185,6 +186,7 @@ static void 
set_default_create_tablet_request(TCreateTabletReq* request) {
 
 static void set_create_duplicate_tablet_request(TCreateTabletReq* request) {
     request->tablet_id = 10009;
+    request->partition_id = 10010;
     request->__set_version(1);
     request->tablet_schema.schema_hash = 270068376;
     request->tablet_schema.short_key_column_count = 2;
diff --git a/be/test/olap/delta_writer_test.cpp 
b/be/test/olap/delta_writer_test.cpp
index f54570b88f1..b9bbd557976 100644
--- a/be/test/olap/delta_writer_test.cpp
+++ b/be/test/olap/delta_writer_test.cpp
@@ -101,6 +101,7 @@ static void tear_down() {
 static void create_tablet_request(int64_t tablet_id, int32_t schema_hash,
                                   TCreateTabletReq* request) {
     request->tablet_id = tablet_id;
+    request->partition_id = 1000;
     request->__set_version(1);
     request->tablet_schema.schema_hash = schema_hash;
     request->tablet_schema.short_key_column_count = 6;
@@ -264,6 +265,7 @@ static void create_tablet_request_with_sequence_col(int64_t 
tablet_id, int32_t s
                                                     TCreateTabletReq* request,
                                                     bool enable_mow = false) {
     request->tablet_id = tablet_id;
+    request->partition_id = 10000;
     request->__set_version(1);
     request->tablet_schema.schema_hash = schema_hash;
     request->tablet_schema.short_key_column_count = 2;
diff --git a/be/test/olap/engine_storage_migration_task_test.cpp 
b/be/test/olap/engine_storage_migration_task_test.cpp
index 9c99ef308ae..532cc38d05c 100644
--- a/be/test/olap/engine_storage_migration_task_test.cpp
+++ b/be/test/olap/engine_storage_migration_task_test.cpp
@@ -100,6 +100,7 @@ static void tear_down() {
 static void create_tablet_request_with_sequence_col(int64_t tablet_id, int32_t 
schema_hash,
                                                     TCreateTabletReq* request) 
{
     request->tablet_id = tablet_id;
+    request->partition_id = 20001;
     request->__set_version(1);
     request->tablet_schema.schema_hash = schema_hash;
     request->tablet_schema.short_key_column_count = 2;
diff --git a/be/test/olap/remote_rowset_gc_test.cpp 
b/be/test/olap/remote_rowset_gc_test.cpp
index a8fc13470f8..779722c9c48 100644
--- a/be/test/olap/remote_rowset_gc_test.cpp
+++ b/be/test/olap/remote_rowset_gc_test.cpp
@@ -117,6 +117,7 @@ public:
 static void create_tablet_request_with_sequence_col(int64_t tablet_id, int32_t 
schema_hash,
                                                     TCreateTabletReq* request) 
{
     request->tablet_id = tablet_id;
+    request->partition_id = 1000;
     request->__set_version(1);
     request->tablet_schema.schema_hash = schema_hash;
     request->tablet_schema.short_key_column_count = 2;
diff --git a/be/test/olap/tablet_cooldown_test.cpp 
b/be/test/olap/tablet_cooldown_test.cpp
index 2582746291a..d02586592c2 100644
--- a/be/test/olap/tablet_cooldown_test.cpp
+++ b/be/test/olap/tablet_cooldown_test.cpp
@@ -276,6 +276,7 @@ public:
 static void create_tablet_request_with_sequence_col(int64_t tablet_id, int32_t 
schema_hash,
                                                     TCreateTabletReq* request) 
{
     request->tablet_id = tablet_id;
+    request->partition_id = 1000;
     request->__set_version(1);
     request->tablet_schema.schema_hash = schema_hash;
     request->tablet_schema.short_key_column_count = 2;
diff --git a/be/test/olap/tablet_mgr_test.cpp b/be/test/olap/tablet_mgr_test.cpp
index 5954d0329fb..dd901df8137 100644
--- a/be/test/olap/tablet_mgr_test.cpp
+++ b/be/test/olap/tablet_mgr_test.cpp
@@ -107,6 +107,7 @@ TEST_F(TabletMgrTest, CreateTablet) {
     create_tablet_req.__set_tablet_schema(tablet_schema);
     create_tablet_req.__set_tablet_id(111);
     create_tablet_req.__set_version(2);
+    create_tablet_req.__set_partition_id(1000);
     std::vector<DataDir*> data_dirs;
     data_dirs.push_back(_data_dir);
     RuntimeProfile profile("CreateTablet");
@@ -167,6 +168,7 @@ TEST_F(TabletMgrTest, CreateTabletWithSequence) {
     TCreateTabletReq create_tablet_req;
     create_tablet_req.__set_tablet_schema(tablet_schema);
     create_tablet_req.__set_tablet_id(111);
+    create_tablet_req.__set_partition_id(1000);
     create_tablet_req.__set_version(2);
     std::vector<DataDir*> data_dirs;
     data_dirs.push_back(_data_dir);
@@ -210,6 +212,7 @@ TEST_F(TabletMgrTest, DropTablet) {
     TCreateTabletReq create_tablet_req;
     create_tablet_req.__set_tablet_schema(tablet_schema);
     create_tablet_req.__set_tablet_id(111);
+    create_tablet_req.__set_partition_id(1000);
     create_tablet_req.__set_version(2);
     std::vector<DataDir*> data_dirs;
     data_dirs.push_back(_data_dir);


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

Reply via email to