This is an automated email from the ASF dual-hosted git repository.
gavinchou pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 9c0ede9665d [fix](cloud) clarifying the key recycling rules for
multiple versions (#58723)
9c0ede9665d is described below
commit 9c0ede9665d52e58fa4bfee91310a7db1f6e8c2f
Author: walter <[email protected]>
AuthorDate: Tue Dec 9 17:37:35 2025 +0800
[fix](cloud) clarifying the key recycling rules for multiple versions
(#58723)
---
cloud/src/recycler/recycler.cpp | 54 ++++++++++++++++-----------
cloud/src/recycler/recycler.h | 8 +++-
cloud/src/recycler/recycler_operation_log.cpp | 10 +++--
cloud/src/recycler/recycler_snapshot.cpp | 21 +++++++++++
cloud/test/meta_service_test.cpp | 1 -
cloud/test/rate_limiter_test.cpp | 4 ++
cloud/test/recycler_test.cpp | 1 +
7 files changed, 72 insertions(+), 27 deletions(-)
diff --git a/cloud/src/recycler/recycler.cpp b/cloud/src/recycler/recycler.cpp
index 789ba13af0b..eabe17dce34 100644
--- a/cloud/src/recycler/recycler.cpp
+++ b/cloud/src/recycler/recycler.cpp
@@ -2023,15 +2023,8 @@ int InstanceRecycler::recycle_partitions() {
}
int InstanceRecycler::recycle_versions() {
- if (instance_info_.has_multi_version_status() &&
- instance_info_.multi_version_status() !=
MultiVersionStatus::MULTI_VERSION_DISABLED) {
- // If the data migration is not finished, skip recycling the orphan
partitions.
- if (instance_info_.multi_version_status() !=
MultiVersionStatus::MULTI_VERSION_WRITE_ONLY ||
- (instance_info_.has_snapshot_switch_status() &&
- instance_info_.snapshot_switch_status() !=
- SnapshotSwitchStatus::SNAPSHOT_SWITCH_DISABLED)) {
- return recycle_orphan_partitions();
- }
+ if (should_recycle_versioned_keys()) {
+ return recycle_orphan_partitions();
}
int64_t num_scanned = 0;
@@ -2324,6 +2317,7 @@ int InstanceRecycler::recycle_tablets(int64_t table_id,
int64_t index_id,
tablet_idx_keys.push_back(meta_tablet_idx_key({instance_id_,
tablet_id}));
restore_job_keys.push_back(job_restore_tablet_key({instance_id_,
tablet_id}));
if (is_multi_version) {
+ // The tablet index/inverted index are recycled in
recycle_versioned_tablet.
tablet_compact_stats_keys.push_back(
versioned::tablet_compact_stats_key({instance_id_,
tablet_id}));
tablet_load_stats_keys.push_back(
@@ -3336,9 +3330,15 @@ int InstanceRecycler::recycle_tablet(int64_t tablet_id,
RecyclerMetricsContext&
.tag("instance_id", instance_id_)
.tag("tablet_id", tablet_id);
- if (instance_info_.has_multi_version_status() &&
- instance_info_.multi_version_status() !=
MultiVersionStatus::MULTI_VERSION_DISABLED) {
- return recycle_versioned_tablet(tablet_id, metrics_context);
+ if (should_recycle_versioned_keys()) {
+ int ret = recycle_versioned_tablet(tablet_id, metrics_context);
+ if (ret != 0) {
+ return ret;
+ }
+ // Continue to recycle non-versioned rowsets, if multi-version is set
to DISABLED
+ // during the recycle_versioned_tablet process.
+ //
+ // .. And remove restore job rowsets of this tablet too
}
int ret = 0;
@@ -3714,20 +3714,26 @@ int InstanceRecycler::recycle_versioned_tablet(int64_t
tablet_id,
for (const auto& [rs_meta, versionstamp] : load_rowset_metas) {
update_rowset_stats(rs_meta);
concurrent_delete_executor.add([tablet_id, versionstamp, rs_meta_pb =
rs_meta, this]() {
- std::string rowset_key = versioned::meta_rowset_load_key(
+ // recycle both versioned and non-versioned rowset meta key
+ std::string rowset_load_key = versioned::meta_rowset_load_key(
{instance_id_, tablet_id, rs_meta_pb.end_version()});
- return
recycle_rowset_meta_and_data(encode_versioned_key(rowset_key, versionstamp),
- rs_meta_pb);
+ std::string rowset_key =
+ meta_rowset_key({instance_id_, tablet_id,
rs_meta_pb.end_version()});
+ return
recycle_rowset_meta_and_data(encode_versioned_key(rowset_load_key,
versionstamp),
+ rs_meta_pb, rowset_key);
});
}
for (const auto& [rs_meta, versionstamp] : compact_rowset_metas) {
update_rowset_stats(rs_meta);
concurrent_delete_executor.add([tablet_id, versionstamp, rs_meta_pb =
rs_meta, this]() {
- std::string rowset_key = versioned::meta_rowset_compact_key(
+ // recycle both versioned and non-versioned rowset meta key
+ std::string rowset_load_key = versioned::meta_rowset_compact_key(
{instance_id_, tablet_id, rs_meta_pb.end_version()});
- return
recycle_rowset_meta_and_data(encode_versioned_key(rowset_key, versionstamp),
- rs_meta_pb);
+ std::string rowset_key =
+ meta_rowset_key({instance_id_, tablet_id,
rs_meta_pb.end_version()});
+ return
recycle_rowset_meta_and_data(encode_versioned_key(rowset_load_key,
versionstamp),
+ rs_meta_pb, rowset_key);
});
}
@@ -3881,8 +3887,7 @@ int InstanceRecycler::recycle_versioned_tablet(int64_t
tablet_id,
}
int InstanceRecycler::recycle_rowsets() {
- if (instance_info_.has_multi_version_status() &&
- instance_info_.multi_version_status() !=
MultiVersionStatus::MULTI_VERSION_DISABLED) {
+ if (should_recycle_versioned_keys()) {
return recycle_versioned_rowsets();
}
@@ -4545,7 +4550,8 @@ int InstanceRecycler::recycle_versioned_rowsets() {
}
int InstanceRecycler::recycle_rowset_meta_and_data(std::string_view
recycle_rowset_key,
- const RowsetMetaCloudPB&
rowset_meta) {
+ const RowsetMetaCloudPB&
rowset_meta,
+ std::string_view
secondary_rowset_key) {
constexpr int MAX_RETRY = 10;
int64_t tablet_id = rowset_meta.tablet_id();
const std::string& rowset_id = rowset_meta.rowset_id_v2();
@@ -4634,6 +4640,12 @@ int
InstanceRecycler::recycle_rowset_meta_and_data(std::string_view recycle_rows
}
txn->remove(recycle_rowset_key);
+ LOG_INFO("remove recycle rowset key").tag("key",
hex(recycle_rowset_key));
+ if (!secondary_rowset_key.empty()) {
+ txn->remove(secondary_rowset_key);
+ LOG_INFO("remove secondary rowset key").tag("key",
hex(secondary_rowset_key));
+ }
+
err = txn->commit();
if (err == TxnErrorCode::TXN_CONFLICT) { // unlikely
// The rowset ref count key has been changed, we need to retry.
diff --git a/cloud/src/recycler/recycler.h b/cloud/src/recycler/recycler.h
index 5d631fd0c2b..a9f2b2c662d 100644
--- a/cloud/src/recycler/recycler.h
+++ b/cloud/src/recycler/recycler.h
@@ -466,13 +466,19 @@ private:
// Recycle rowset meta and data, return 0 for success otherwise error
//
+ // Both recycle_rowset_key and secondary_rowset_key will be removed in the
same transaction.
+ //
// This function will decrease the rowset ref count and remove the rowset
meta and data if the ref count is 1.
int recycle_rowset_meta_and_data(std::string_view recycle_rowset_key,
- const RowsetMetaCloudPB& rowset_meta);
+ const RowsetMetaCloudPB& rowset_meta,
+ std::string_view secondary_rowset_key =
"");
// Whether the instance has any snapshots, return 0 for success otherwise
error.
int has_cluster_snapshots(bool* any);
+ // Whether need to recycle versioned keys
+ bool should_recycle_versioned_keys() const;
+
/**
* Parse the path of a packed-file fragment and output the owning tablet
and rowset identifiers.
*
diff --git a/cloud/src/recycler/recycler_operation_log.cpp
b/cloud/src/recycler/recycler_operation_log.cpp
index af5ce1224bd..424b2b83409 100644
--- a/cloud/src/recycler/recycler_operation_log.cpp
+++ b/cloud/src/recycler/recycler_operation_log.cpp
@@ -605,11 +605,13 @@ static TxnErrorCode get_txn_info(TxnKv* txn_kv,
std::string_view instance_id, in
}
int InstanceRecycler::recycle_operation_logs() {
- if (!instance_info_.has_multi_version_status() ||
- (instance_info_.multi_version_status() !=
MultiVersionStatus::MULTI_VERSION_ENABLED &&
- instance_info_.multi_version_status() !=
MultiVersionStatus::MULTI_VERSION_READ_WRITE)) {
+ if (!should_recycle_versioned_keys()) {
VLOG_DEBUG << "instance " << instance_id_
- << " is not multi-version enabled, skip recycling operation
logs.";
+ << " is not need to recycle versioned keys, skip recycling
operation logs. "
+ "multi version status: "
+ <<
MultiVersionStatus_Name(instance_info_.multi_version_status())
+ << " snapshot switch status: "
+ <<
SnapshotSwitchStatus_Name(instance_info_.snapshot_switch_status());
return 0;
}
diff --git a/cloud/src/recycler/recycler_snapshot.cpp
b/cloud/src/recycler/recycler_snapshot.cpp
index 38c34c09bea..17bc4a19338 100644
--- a/cloud/src/recycler/recycler_snapshot.cpp
+++ b/cloud/src/recycler/recycler_snapshot.cpp
@@ -82,4 +82,25 @@ int InstanceRecycler::has_cluster_snapshots(bool* any) {
return 0;
}
+bool InstanceRecycler::should_recycle_versioned_keys() const {
+ if (!instance_info_.has_multi_version_status()) {
+ return false;
+ }
+
+ if (instance_info_.multi_version_status() == MULTI_VERSION_DISABLED) {
+ return false;
+ }
+
+ // When multi version is write only and snapshot switch is disabled,
+ // we do not need to recycle versioned keys. Because there has some
+ // keys which are not migrated to versioned keys yet.
+ if (instance_info_.multi_version_status() == MULTI_VERSION_WRITE_ONLY &&
+ (!instance_info_.has_snapshot_switch_status() ||
+ instance_info_.snapshot_switch_status() == SNAPSHOT_SWITCH_DISABLED))
{
+ return false;
+ }
+
+ return true;
+}
+
} // namespace doris::cloud
diff --git a/cloud/test/meta_service_test.cpp b/cloud/test/meta_service_test.cpp
index c8c3980ada1..38df623a13c 100644
--- a/cloud/test/meta_service_test.cpp
+++ b/cloud/test/meta_service_test.cpp
@@ -2351,7 +2351,6 @@ TEST(MetaServiceTest, CommitTxnWithSubTxnTest2) {
meta_service->commit_txn(reinterpret_cast<::google::protobuf::RpcController*>(&cntl),
&req,
&res, nullptr);
ASSERT_EQ(res.status().code(), MetaServiceCode::OK);
- std::cout << res.DebugString() << std::endl;
ASSERT_EQ(res.table_ids().size(), 3);
ASSERT_EQ(res.table_ids()[0], t2);
diff --git a/cloud/test/rate_limiter_test.cpp b/cloud/test/rate_limiter_test.cpp
index 166c3107395..66385767b77 100644
--- a/cloud/test/rate_limiter_test.cpp
+++ b/cloud/test/rate_limiter_test.cpp
@@ -36,6 +36,10 @@
int main(int argc, char** argv) {
doris::cloud::config::init(nullptr, true);
+ if (!doris::cloud::init_glog("rate_limiter_test")) {
+ std::cerr << "failed to init glog" << std::endl;
+ return -1;
+ }
::testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
}
diff --git a/cloud/test/recycler_test.cpp b/cloud/test/recycler_test.cpp
index c02bfcd035b..abb1d706998 100644
--- a/cloud/test/recycler_test.cpp
+++ b/cloud/test/recycler_test.cpp
@@ -1382,6 +1382,7 @@ TEST(RecyclerTest, recycle_rowsets_with_data_ref_count) {
InstanceInfoPB instance;
instance.set_instance_id(instance_id);
instance.set_multi_version_status(MultiVersionStatus::MULTI_VERSION_WRITE_ONLY);
+ instance.set_snapshot_switch_status(SNAPSHOT_SWITCH_OFF);
auto obj_info = instance.add_obj_info();
obj_info->set_id("recycle_rowsets_with_data_ref_count");
obj_info->set_ak(config::test_s3_ak);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
