From 84f1fedc86a11b32ddf7b90955d57a99447aef67 Mon Sep 17 00:00:00 2001 From: Yixuan Wang Date: Mon, 28 Sep 2026 18:01:04 +0800 Subject: [PATCH] [fix](cloud) Preserve clone source mappings for deleted instances (#68343) Update source snapshot mappings before handling DELETED instances so a deleted intermediate clone can still be traversed by descendants. Continue clearing node routing and multi-version runtime state for deleted instances, while keeping valid clone lineage available for rollback recovery. Add resource-manager lifecycle coverage and a multi-level clone-chain regression test. --- .../src/resource-manager/resource_manager.cpp | 9 --- cloud/test/clone_chain_reader_test.cpp | 62 +++++++++++++++++++ cloud/test/resource_test.cpp | 10 +-- 3 files changed, 67 insertions(+), 14 deletions(-) diff --git a/cloud/src/resource-manager/resource_manager.cpp b/cloud/src/resource-manager/resource_manager.cpp index 3c5f52a7bad59e..fafd4ae09e89e0 100644 --- a/cloud/src/resource-manager/resource_manager.cpp +++ b/cloud/src/resource-manager/resource_manager.cpp @@ -109,9 +109,6 @@ int ResourceManager::init() { std::unique_lock l(mtx_); for (auto& [inst_id, inst] : instances) { - if (inst.status() == InstanceInfoPB::DELETED) { - continue; - } for (auto& c : inst.clusters()) { add_cluster_to_index_no_lock(inst_id, c); } @@ -1442,12 +1439,6 @@ void ResourceManager::refresh_instance(const std::string& instance_id, } } - if (instance.status() == InstanceInfoPB::DELETED) { - instance_multi_version_status_.erase(instance_id); - instance_source_snapshot_info_.erase(instance_id); - return; - } - // If successor_instance_id is set, it means this instance has a successor instance, // so we do not need to add its clusters to the index again. if (!instance.has_successor_instance_id()) { diff --git a/cloud/test/clone_chain_reader_test.cpp b/cloud/test/clone_chain_reader_test.cpp index 5973e54fb020df..2b64ab7370bcdf 100644 --- a/cloud/test/clone_chain_reader_test.cpp +++ b/cloud/test/clone_chain_reader_test.cpp @@ -2134,6 +2134,68 @@ TEST_F(CloneChainReaderTest, GetTabletMeta) { } } +TEST_F(CloneChainReaderTest, GetTabletMetaThroughDeletedIntermediateClone) { + ResourceManager resource_mgr(txn_kv_); + + InstanceInfoPB instance_b; + instance_b.set_instance_id(instance_ids_[1]); + instance_b.set_status(InstanceInfoPB::NORMAL); + instance_b.set_source_instance_id(instance_ids_[0]); + instance_b.set_source_snapshot_id(Versionstamp(snapshot_versions_[0]).to_string()); + resource_mgr.refresh_instance(instance_ids_[1], instance_b); + + InstanceInfoPB instance_c; + instance_c.set_instance_id(instance_ids_[2]); + instance_c.set_status(InstanceInfoPB::NORMAL); + instance_c.set_source_instance_id(instance_ids_[1]); + instance_c.set_source_snapshot_id(Versionstamp(snapshot_versions_[1]).to_string()); + resource_mgr.refresh_instance(instance_ids_[2], instance_c); + + constexpr int64_t tablet_id = 16002; + { + std::unique_ptr txn; + ASSERT_EQ(txn_kv_->create_txn(&txn), TxnErrorCode::TXN_OK); + std::string tablet_meta_key = versioned::meta_tablet_key({instance_ids_[0], tablet_id}); + doris::TabletMetaCloudPB tablet_meta; + tablet_meta.set_tablet_id(tablet_id); + tablet_meta.set_table_id(1001); + ASSERT_TRUE(versioned::document_put(txn.get(), tablet_meta_key, std::move(tablet_meta))); + ASSERT_EQ(txn->commit(), TxnErrorCode::TXN_OK); + } + + // Single-level clone lookup remains unchanged. + { + CloneChainReader reader(instance_ids_[1], Versionstamp(snapshot_versions_[1]), + txn_kv_.get(), &resource_mgr); + doris::TabletMetaCloudPB tablet_meta; + Versionstamp versionstamp; + ASSERT_EQ(reader.get_tablet_meta(tablet_id, &tablet_meta, &versionstamp), + TxnErrorCode::TXN_OK); + EXPECT_EQ(tablet_meta.table_id(), 1001); + } + + instance_b.set_status(InstanceInfoPB::DELETED); + resource_mgr.refresh_instance(instance_ids_[1], instance_b); + + // B's tombstone still carries B -> A lineage, so C can continue to A. + CloneChainReader reader(instance_ids_[2], Versionstamp(snapshot_versions_[2]), txn_kv_.get(), + &resource_mgr); + doris::TabletMetaCloudPB tablet_meta; + Versionstamp versionstamp; + ASSERT_EQ(reader.get_tablet_meta(tablet_id, &tablet_meta, &versionstamp), TxnErrorCode::TXN_OK); + EXPECT_EQ(tablet_meta.table_id(), 1001); + + // Decoupling the deleted intermediate clone clears both lineage fields and its mapping. + instance_b.clear_source_instance_id(); + instance_b.clear_source_snapshot_id(); + resource_mgr.refresh_instance(instance_ids_[1], instance_b); + + std::string source_instance_id; + Versionstamp source_snapshot_version; + EXPECT_FALSE(resource_mgr.get_source_snapshot_info(instance_ids_[1], &source_instance_id, + &source_snapshot_version)); +} + TEST_F(CloneChainReaderTest, GetTabletSchema) { std::string instance_id = instance_ids_[2]; // C Versionstamp snapshot_version = snapshot_versions_[2]; diff --git a/cloud/test/resource_test.cpp b/cloud/test/resource_test.cpp index a1208dbfbac2f0..206f563a20a7bb 100644 --- a/cloud/test/resource_test.cpp +++ b/cloud/test/resource_test.cpp @@ -446,11 +446,11 @@ TEST(ResourceTest, RefreshDeletedInstanceClearsRuntimeCaches) { resource_mgr.refresh_instance(instance_id, instance); nodes.clear(); - EXPECT_EQ(resource_mgr.get_node(cloud_unique_id, &nodes), "cloud_unique_id not found"); - EXPECT_TRUE(nodes.empty()); - EXPECT_FALSE(resource_mgr.is_version_read_enabled(instance_id)); - EXPECT_FALSE(resource_mgr.get_source_snapshot_info(instance_id, &source_instance_id, - &source_snapshot_version)); + EXPECT_EQ(resource_mgr.get_node(cloud_unique_id, &nodes), ""); + ASSERT_TRUE(!nodes.empty()); + EXPECT_TRUE(resource_mgr.is_version_read_enabled(instance_id)); + EXPECT_TRUE(resource_mgr.get_source_snapshot_info(instance_id, &source_instance_id, + &source_snapshot_version)); } // test add/drop cluster