From cd1e82e86e2e1b39ebd0b9f4301042e613194cad Mon Sep 17 00:00:00 2001 From: Yongqiang YANG Date: Wed, 6 Dec 2023 14:44:17 +0800 Subject: [PATCH 1/7] [enhance](partition_id) check partition id before store meta --- be/src/olap/rowset/rowset_meta_manager.cpp | 6 ++++++ be/src/olap/tablet_meta_manager.cpp | 6 ++++++ be/src/olap/txn_manager.cpp | 7 ++++--- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/be/src/olap/rowset/rowset_meta_manager.cpp b/be/src/olap/rowset/rowset_meta_manager.cpp index 26876ea7197111..62786f0aa8561a 100644 --- a/be/src/olap/rowset/rowset_meta_manager.cpp +++ b/be/src/olap/rowset/rowset_meta_manager.cpp @@ -88,6 +88,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_meta_manager.cpp b/be/src/olap/tablet_meta_manager.cpp index ca14df587a20a6..43e076e68b62f0 100644 --- a/be/src/olap/tablet_meta_manager.cpp +++ b/be/src/olap/tablet_meta_manager.cpp @@ -93,6 +93,12 @@ Status TabletMetaManager::save(DataDir* store, TTabletId tablet_id, TSchemaHash std::string key = fmt::format("{}{}_{}", header_prefix, tablet_id, schema_hash); std::string value; static_cast(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 e4bf4191f718f0..098c0197f64cea 100644 --- a/be/src/olap/txn_manager.cpp +++ b/be/src/olap/txn_manager.cpp @@ -295,9 +295,10 @@ Status TxnManager::commit_txn(OlapMeta* meta, TPartitionId partition_id, const RowsetSharedPtr& rowset_ptr, PendingRowsetGuard guard, 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 8af7e5366161bdac6eae9006693b8ea91c6c84ed Mon Sep 17 00:00:00 2001 From: Yongqiang YANG Date: Wed, 6 Dec 2023 14:48:07 +0800 Subject: [PATCH 2/7] format --- be/src/olap/rowset/rowset_meta_manager.cpp | 2 +- be/src/olap/tablet_meta_manager.cpp | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/be/src/olap/rowset/rowset_meta_manager.cpp b/be/src/olap/rowset/rowset_meta_manager.cpp index 62786f0aa8561a..01384a560575d5 100644 --- a/be/src/olap/rowset/rowset_meta_manager.cpp +++ b/be/src/olap/rowset/rowset_meta_manager.cpp @@ -92,7 +92,7 @@ Status RowsetMetaManager::save(OlapMeta* meta, TabletUid tablet_uid, const Rowse 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()); + 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_meta_manager.cpp b/be/src/olap/tablet_meta_manager.cpp index 43e076e68b62f0..17d5d2e9f74b09 100644 --- a/be/src/olap/tablet_meta_manager.cpp +++ b/be/src/olap/tablet_meta_manager.cpp @@ -94,10 +94,10 @@ Status TabletMetaManager::save(DataDir* store, TTabletId tablet_id, TSchemaHash std::string value; static_cast(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 3c67480cd0936064eed951e133f007355c9b5f25 Mon Sep 17 00:00:00 2001 From: Yongqiang YANG Date: Wed, 6 Dec 2023 16:18:40 +0800 Subject: [PATCH 3/7] fix --- be/src/olap/data_dir.cpp | 18 +++++++++++++----- be/test/olap/delete_handler_test.cpp | 1 + 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/be/src/olap/data_dir.cpp b/be/src/olap/data_dir.cpp index 3dfdc7d87fb900..689d6f37084cf1 100644 --- a/be/src/olap/data_dir.cpp +++ b/be/src/olap/data_dir.cpp @@ -539,15 +539,23 @@ Status DataDir::load() { StorageEngine::instance()->pending_local_rowsets().add( rowset_meta->rowset_id()), 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 bd227c9a8e6ed1..8ee961ffa3bad9 100644 --- a/be/test/olap/delete_handler_test.cpp +++ b/be/test/olap/delete_handler_test.cpp @@ -102,6 +102,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 0c51dae03dd6bd272c3a2f59d88b3959dce2eaf6 Mon Sep 17 00:00:00 2001 From: Yongqiang YANG Date: Wed, 6 Dec 2023 16:55:29 +0800 Subject: [PATCH 4/7] 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 689d6f37084cf1..8aa07791ebc837 100644 --- a/be/src/olap/data_dir.cpp +++ b/be/src/olap/data_dir.cpp @@ -545,7 +545,7 @@ Status DataDir::load() { << " 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 6cc2dd9c3a6325af7094a345c89d7a0d707fc911 Mon Sep 17 00:00:00 2001 From: Yongqiang YANG Date: Wed, 6 Dec 2023 18:10:18 +0800 Subject: [PATCH 5/7] fix ut --- be/test/olap/delta_writer_test.cpp | 2 ++ be/test/olap/engine_storage_migration_task_test.cpp | 1 + be/test/olap/memtable_memory_limiter_test.cpp | 1 + be/test/olap/ordered_data_compaction_test.cpp | 1 + be/test/olap/path_gc_test.cpp | 1 + be/test/olap/remote_rowset_gc_test.cpp | 1 + be/test/olap/rowid_conversion_test.cpp | 1 + be/test/olap/segcompaction_test.cpp | 1 + be/test/olap/tablet_cooldown_test.cpp | 1 + be/test/runtime/load_stream_test.cpp | 1 + be/test/vec/olap/vertical_compaction_test.cpp | 1 + 11 files changed, 12 insertions(+) diff --git a/be/test/olap/delta_writer_test.cpp b/be/test/olap/delta_writer_test.cpp index f2274a9a76a24d..86e10dff7d1f96 100644 --- a/be/test/olap/delta_writer_test.cpp +++ b/be/test/olap/delta_writer_test.cpp @@ -105,6 +105,7 @@ static void create_tablet_request(int64_t tablet_id, int32_t schema_hash, TCreateTabletReq* request) { request->tablet_id = tablet_id; request->__set_version(1); + request->partition_id = 10001; request->tablet_schema.schema_hash = schema_hash; request->tablet_schema.short_key_column_count = 6; request->tablet_schema.keys_type = TKeysType::AGG_KEYS; @@ -268,6 +269,7 @@ static void create_tablet_request_with_sequence_col(int64_t tablet_id, int32_t s bool enable_mow = false) { request->tablet_id = tablet_id; request->__set_version(1); + request->partition_id = 30004; request->tablet_schema.schema_hash = schema_hash; request->tablet_schema.short_key_column_count = 2; request->tablet_schema.keys_type = TKeysType::UNIQUE_KEYS; diff --git a/be/test/olap/engine_storage_migration_task_test.cpp b/be/test/olap/engine_storage_migration_task_test.cpp index 87b30ae4da35a1..5dc00d302aed2f 100644 --- a/be/test/olap/engine_storage_migration_task_test.cpp +++ b/be/test/olap/engine_storage_migration_task_test.cpp @@ -102,6 +102,7 @@ static void create_tablet_request_with_sequence_col(int64_t tablet_id, int32_t s TCreateTabletReq* request) { request->tablet_id = tablet_id; request->__set_version(1); + request->partition_id = 10001; request->tablet_schema.schema_hash = schema_hash; request->tablet_schema.short_key_column_count = 2; request->tablet_schema.keys_type = TKeysType::UNIQUE_KEYS; diff --git a/be/test/olap/memtable_memory_limiter_test.cpp b/be/test/olap/memtable_memory_limiter_test.cpp index 11e791551b18e5..1ad7b47c4e9225 100644 --- a/be/test/olap/memtable_memory_limiter_test.cpp +++ b/be/test/olap/memtable_memory_limiter_test.cpp @@ -33,6 +33,7 @@ static void create_tablet_request(int64_t tablet_id, int32_t schema_hash, TCreateTabletReq* request) { request->tablet_id = tablet_id; request->__set_version(1); + request->partition_id = 30002; request->tablet_schema.schema_hash = schema_hash; request->tablet_schema.short_key_column_count = 3; request->tablet_schema.keys_type = TKeysType::AGG_KEYS; diff --git a/be/test/olap/ordered_data_compaction_test.cpp b/be/test/olap/ordered_data_compaction_test.cpp index 173396c94ffbb9..f93b9d9935ff3f 100644 --- a/be/test/olap/ordered_data_compaction_test.cpp +++ b/be/test/olap/ordered_data_compaction_test.cpp @@ -271,6 +271,7 @@ class OrderedDataCompactionTest : public ::testing::Test { std::string json_rowset_meta = R"({ "rowset_id": 540085, "tablet_id": 15674, + "partition_id": 10000, "txn_id": 4045, "tablet_schema_hash": 567997588, "rowset_type": "BETA_ROWSET", diff --git a/be/test/olap/path_gc_test.cpp b/be/test/olap/path_gc_test.cpp index 67ed23b8fde723..54edad8b9de4f4 100644 --- a/be/test/olap/path_gc_test.cpp +++ b/be/test/olap/path_gc_test.cpp @@ -54,6 +54,7 @@ TEST(PathGcTest, GcTabletAndRowset) { auto create_tablet = [&](int64_t tablet_id) { auto tablet_meta = std::make_shared(); tablet_meta->_tablet_id = tablet_id; + tablet_meta->set_partition_id(10000); tablet_meta->set_tablet_uid({tablet_id, 0}); tablet_meta->set_shard_id(tablet_id % 4); tablet_meta->_schema_hash = tablet_id; diff --git a/be/test/olap/remote_rowset_gc_test.cpp b/be/test/olap/remote_rowset_gc_test.cpp index 28d69fe8a74e9d..3727f3b8764197 100644 --- a/be/test/olap/remote_rowset_gc_test.cpp +++ b/be/test/olap/remote_rowset_gc_test.cpp @@ -112,6 +112,7 @@ static void create_tablet_request_with_sequence_col(int64_t tablet_id, int32_t s TCreateTabletReq* request) { request->tablet_id = tablet_id; request->__set_version(1); + request->partition_id = 30003; request->tablet_schema.schema_hash = schema_hash; request->tablet_schema.short_key_column_count = 2; request->tablet_schema.keys_type = TKeysType::UNIQUE_KEYS; diff --git a/be/test/olap/rowid_conversion_test.cpp b/be/test/olap/rowid_conversion_test.cpp index e161c89a58114d..6105737a4180dc 100644 --- a/be/test/olap/rowid_conversion_test.cpp +++ b/be/test/olap/rowid_conversion_test.cpp @@ -223,6 +223,7 @@ class TestRowIdConversion : public testing::TestWithParam data_dir = std::make_shared(lTestDir); TabletMetaSharedPtr tablet_meta = std::make_shared(); tablet_meta->_tablet_id = 1; + tablet_meta->set_partition_id(10000); tablet_meta->_schema = tablet_schema; auto tablet = std::make_shared(tablet_meta, data_dir.get(), "test_str"); char* tmp_str = (char*)malloc(20); diff --git a/be/test/olap/tablet_cooldown_test.cpp b/be/test/olap/tablet_cooldown_test.cpp index e19ba3bd32c477..d5328871f52361 100644 --- a/be/test/olap/tablet_cooldown_test.cpp +++ b/be/test/olap/tablet_cooldown_test.cpp @@ -267,6 +267,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 = 30003; request->__set_version(1); request->tablet_schema.schema_hash = schema_hash; request->tablet_schema.short_key_column_count = 2; diff --git a/be/test/runtime/load_stream_test.cpp b/be/test/runtime/load_stream_test.cpp index b1ad08261777fe..7cba945dd230c2 100644 --- a/be/test/runtime/load_stream_test.cpp +++ b/be/test/runtime/load_stream_test.cpp @@ -142,6 +142,7 @@ void construct_schema(OlapTableSchemaParam* schema) { static void create_tablet_request(int64_t tablet_id, int32_t schema_hash, TCreateTabletReq* request) { request->tablet_id = tablet_id; + request->partition_id = 30001; request->__set_version(1); request->tablet_schema.schema_hash = schema_hash; request->tablet_schema.short_key_column_count = 6; diff --git a/be/test/vec/olap/vertical_compaction_test.cpp b/be/test/vec/olap/vertical_compaction_test.cpp index 3fa639937d081a..13db760f48d106 100644 --- a/be/test/vec/olap/vertical_compaction_test.cpp +++ b/be/test/vec/olap/vertical_compaction_test.cpp @@ -273,6 +273,7 @@ class VerticalCompactionTest : public ::testing::Test { std::string json_rowset_meta = R"({ "rowset_id": 540081, "tablet_id": 15673, + "partition_id": 10000, "tablet_schema_hash": 567997577, "rowset_type": "BETA_ROWSET", "rowset_state": "VISIBLE", From 516a4ff7ba3d0590ae5b53076d759017017937d3 Mon Sep 17 00:00:00 2001 From: Yongqiang YANG Date: Wed, 6 Dec 2023 23:46:09 +0800 Subject: [PATCH 6/7] fix ut --- be/test/olap/path_gc_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/be/test/olap/path_gc_test.cpp b/be/test/olap/path_gc_test.cpp index 54edad8b9de4f4..c1b28bf31dd6f6 100644 --- a/be/test/olap/path_gc_test.cpp +++ b/be/test/olap/path_gc_test.cpp @@ -54,7 +54,7 @@ TEST(PathGcTest, GcTabletAndRowset) { auto create_tablet = [&](int64_t tablet_id) { auto tablet_meta = std::make_shared(); tablet_meta->_tablet_id = tablet_id; - tablet_meta->set_partition_id(10000); + (void)tablet_meta->set_partition_id(10000); tablet_meta->set_tablet_uid({tablet_id, 0}); tablet_meta->set_shard_id(tablet_id % 4); tablet_meta->_schema_hash = tablet_id; From 72a515c90b885be8082223106eb3770437634006 Mon Sep 17 00:00:00 2001 From: Yongqiang YANG Date: Tue, 19 Dec 2023 10:20:46 +0800 Subject: [PATCH 7/7] fix ut --- be/src/olap/txn_manager.cpp | 12 ++++-------- be/test/olap/test_data/rowset_meta.json | 1 + be/test/olap/test_data/rowset_meta2.json | 1 + 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/be/src/olap/txn_manager.cpp b/be/src/olap/txn_manager.cpp index 098c0197f64cea..2b1e57925fd14f 100644 --- a/be/src/olap/txn_manager.cpp +++ b/be/src/olap/txn_manager.cpp @@ -384,10 +384,8 @@ Status TxnManager::commit_txn(OlapMeta* meta, TPartitionId partition_id, } }); if (!save_status.ok()) { - return Status::Error( - "save committed rowset failed. when commit txn rowset_id: {}, tablet id: {}, " - "txn id: {}", - rowset_ptr->rowset_id().to_string(), tablet_id, transaction_id); + save_status.append(fmt::format(", txn id: {}", transaction_id)); + return save_status; } } @@ -538,10 +536,8 @@ Status TxnManager::publish_txn(OlapMeta* meta, TPartitionId partition_id, rowset->rowset_meta()->get_rowset_pb(), enable_binlog); stats->save_meta_time_us += MonotonicMicros() - t5; if (!status.ok()) { - return Status::Error( - "save committed rowset failed. when publish txn rowset_id: {}, tablet id: {}, txn " - "id: {}", - rowset->rowset_id().to_string(), tablet_id, transaction_id); + status.append(fmt::format(", txn id: {}", transaction_id)); + return status; } // TODO(Drogon): remove these test codes diff --git a/be/test/olap/test_data/rowset_meta.json b/be/test/olap/test_data/rowset_meta.json index 04016427ce3557..e2ac4bb8082dd2 100644 --- a/be/test/olap/test_data/rowset_meta.json +++ b/be/test/olap/test_data/rowset_meta.json @@ -1,5 +1,6 @@ { "rowset_id": 10000, + "partition_id": 10001, "tablet_id": 12046, "tablet_schema_hash": 365187263, "rowset_type": "BETA_ROWSET", diff --git a/be/test/olap/test_data/rowset_meta2.json b/be/test/olap/test_data/rowset_meta2.json index cd7cfb49358d65..5f5a0aba3a1b32 100644 --- a/be/test/olap/test_data/rowset_meta2.json +++ b/be/test/olap/test_data/rowset_meta2.json @@ -1,5 +1,6 @@ { "rowset_id": 10001, + "partition_id": 10001, "tablet_id": 20487, "tablet_schema_hash": 1520686811, "rowset_type": "BETA_ROWSET",