From 9302d179877673c84c25ccc52c6a6cb3283f7cb7 Mon Sep 17 00:00:00 2001 From: Yongqiang YANG Date: Wed, 6 Dec 2023 11:28:00 +0800 Subject: [PATCH 1/9] [enhance](partitionid) check partition id to avoid unexpected behavior --- 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 ++++--- 4 files changed, 21 insertions(+), 3 deletions(-) diff --git a/be/src/olap/rowset/rowset_meta_manager.cpp b/be/src/olap/rowset/rowset_meta_manager.cpp index f879aaf89957f9..58a4208f209717 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("invalid 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 22276e4041ea71..e5ef2eaad6aa80 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) << "unexpected partition id " << tablet_meta->partition_id() + << ", tablet " << tablet_meta->tablet_id(); + return nullptr; + } return std::make_shared(tablet_meta, data_dir); } diff --git a/be/src/olap/tablet_meta_manager.cpp b/be/src/olap/tablet_meta_manager.cpp index 20e5747e2f0ad4..7f775664df0fac 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 4dbe317ad1fda5..23efbcd1a73d96 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 key(partition_id, transaction_id); From 4f0cbcc597c064fccac8e51ae1c84750900f5cac Mon Sep 17 00:00:00 2001 From: Yongqiang YANG Date: Wed, 6 Dec 2023 15:06:09 +0800 Subject: [PATCH 2/9] format --- be/src/olap/rowset/rowset_meta_manager.cpp | 6 +++--- be/src/olap/tablet.cpp | 4 ++-- be/src/olap/tablet_meta_manager.cpp | 8 ++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/be/src/olap/rowset/rowset_meta_manager.cpp b/be/src/olap/rowset/rowset_meta_manager.cpp index 58a4208f209717..16bda887c358e1 100644 --- a/be/src/olap/rowset/rowset_meta_manager.cpp +++ b/be/src/olap/rowset/rowset_meta_manager.cpp @@ -91,11 +91,11 @@ 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) { + 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("invalid partition id {}, tablet {}", - rowset_meta_pb.partition_id(), 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); diff --git a/be/src/olap/tablet.cpp b/be/src/olap/tablet.cpp index e5ef2eaad6aa80..0d9f21da326cda 100644 --- a/be/src/olap/tablet.cpp +++ b/be/src/olap/tablet.cpp @@ -243,8 +243,8 @@ 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) << "unexpected partition id " << tablet_meta->partition_id() - << ", tablet " << tablet_meta->tablet_id(); + LOG(WARNING) << "invalid partition id " << tablet_meta->partition_id() << ", tablet " + << tablet_meta->tablet_id(); return nullptr; } return std::make_shared(tablet_meta, data_dir); diff --git a/be/src/olap/tablet_meta_manager.cpp b/be/src/olap/tablet_meta_manager.cpp index 7f775664df0fac..20f3872a03e958 100644 --- a/be/src/olap/tablet_meta_manager.cpp +++ b/be/src/olap/tablet_meta_manager.cpp @@ -93,10 +93,10 @@ Status TabletMetaManager::save(DataDir* store, TTabletId tablet_id, TSchemaHash 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()); + 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" From bbc060da487bc173585751c3aa80bae42de4d9cd Mon Sep 17 00:00:00 2001 From: Yongqiang YANG Date: Wed, 6 Dec 2023 16:14:52 +0800 Subject: [PATCH 3/9] fix --- be/src/olap/data_dir.cpp | 17 ++++++++++++----- be/test/olap/delete_handler_test.cpp | 1 + 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/be/src/olap/data_dir.cpp b/be/src/olap/data_dir.cpp index c54998c76d3075..b434ea69d627e9 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()) { - 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()) { 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()) { + 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/test/olap/delete_handler_test.cpp b/be/test/olap/delete_handler_test.cpp index 792fc18f78ca87..cb355fd7c1cbe1 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; From d38fc2e4bb297c472b09a0a73a070e0e20270e25 Mon Sep 17 00:00:00 2001 From: Yongqiang YANG Date: Wed, 6 Dec 2023 16:16:29 +0800 Subject: [PATCH 4/9] format --- be/src/olap/data_dir.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/be/src/olap/data_dir.cpp b/be/src/olap/data_dir.cpp index b434ea69d627e9..5a96af0211659b 100644 --- a/be/src/olap/data_dir.cpp +++ b/be/src/olap/data_dir.cpp @@ -506,7 +506,7 @@ 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()) { + if (commit_txn_status || commit_txn_status.is()) { LOG(INFO) << "successfully to add committed rowset: " << rowset_meta->rowset_id() << " to tablet: " << rowset_meta->tablet_id() << " schema hash: " << rowset_meta->tablet_schema_hash() From ba37965ea7b8050b1ea04719628e2edfe78f009d Mon Sep 17 00:00:00 2001 From: Yongqiang YANG Date: Wed, 6 Dec 2023 16:56:41 +0800 Subject: [PATCH 5/9] fix --- be/src/olap/data_dir.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/be/src/olap/data_dir.cpp b/be/src/olap/data_dir.cpp index 5a96af0211659b..47434bd82261a5 100644 --- a/be/src/olap/data_dir.cpp +++ b/be/src/olap/data_dir.cpp @@ -511,7 +511,7 @@ Status DataDir::load() { << " 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()) { + } else if (commit_txn_status.is()) { LOG(WARNING) << "failed to add committed rowset: " << rowset_meta->rowset_id() << " to tablet: " << rowset_meta->tablet_id() << " for txn: " << rowset_meta->txn_id() From 1bd15f6bcd3d3276c164326dc2878d98606e6374 Mon Sep 17 00:00:00 2001 From: Yongqiang YANG Date: Wed, 6 Dec 2023 17:33:54 +0800 Subject: [PATCH 6/9] fix --- be/test/olap/delete_handler_test.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/be/test/olap/delete_handler_test.cpp b/be/test/olap/delete_handler_test.cpp index cb355fd7c1cbe1..3029220903f006 100644 --- a/be/test/olap/delete_handler_test.cpp +++ b/be/test/olap/delete_handler_test.cpp @@ -186,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; From 2b958c69d1ebcec35c68e81ef680563f82e2477c Mon Sep 17 00:00:00 2001 From: Yongqiang YANG Date: Wed, 6 Dec 2023 17:45:24 +0800 Subject: [PATCH 7/9] fix ut --- be/test/olap/delta_writer_test.cpp | 1 + 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 + 4 files changed, 4 insertions(+) diff --git a/be/test/olap/delta_writer_test.cpp b/be/test/olap/delta_writer_test.cpp index f54570b88f1e3d..158e687601f6d3 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; diff --git a/be/test/olap/engine_storage_migration_task_test.cpp b/be/test/olap/engine_storage_migration_task_test.cpp index 9c99ef308ae97b..532cc38d05cdc0 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 a8fc13470f868e..779722c9c48561 100644 --- a/be/test/olap/remote_rowset_gc_test.cpp +++ b/be/test/olap/remote_rowset_gc_test.cpp @@ -117,6 +117,7 @@ class RemoteRowsetGcTest : public testing::Test { 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 2582746291a32a..d02586592c2716 100644 --- a/be/test/olap/tablet_cooldown_test.cpp +++ b/be/test/olap/tablet_cooldown_test.cpp @@ -276,6 +276,7 @@ class TabletCooldownTest : public testing::Test { 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; From 7a4eb1919c11f57ed7373d4294b4aec51c5da102 Mon Sep 17 00:00:00 2001 From: Yongqiang YANG Date: Wed, 6 Dec 2023 17:59:48 +0800 Subject: [PATCH 8/9] fix ut --- be/test/olap/delta_writer_test.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/be/test/olap/delta_writer_test.cpp b/be/test/olap/delta_writer_test.cpp index 158e687601f6d3..b9bbd55797668e 100644 --- a/be/test/olap/delta_writer_test.cpp +++ b/be/test/olap/delta_writer_test.cpp @@ -265,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; From da882e4108c07d82eb0ce98771eaed4778132914 Mon Sep 17 00:00:00 2001 From: Yongqiang YANG Date: Wed, 6 Dec 2023 18:29:55 +0800 Subject: [PATCH 9/9] fix ut --- be/test/olap/tablet_mgr_test.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/be/test/olap/tablet_mgr_test.cpp b/be/test/olap/tablet_mgr_test.cpp index 5954d0329fbe2c..dd901df8137752 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 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 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 data_dirs; data_dirs.push_back(_data_dir);