From dc66775a9f2f88527aedb832f899ec700251feaa Mon Sep 17 00:00:00 2001 From: obdev Date: Wed, 18 Sep 2024 08:03:39 +0000 Subject: [PATCH] Fix: handle set obj should be the last step --- src/storage/meta_mem/ob_tablet_pointer_map.cpp | 12 ++++++------ src/storage/meta_mem/ob_tenant_meta_mem_mgr.cpp | 11 +++++++---- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/storage/meta_mem/ob_tablet_pointer_map.cpp b/src/storage/meta_mem/ob_tablet_pointer_map.cpp index 2f5d754fa..51a847e37 100644 --- a/src/storage/meta_mem/ob_tablet_pointer_map.cpp +++ b/src/storage/meta_mem/ob_tablet_pointer_map.cpp @@ -631,6 +631,7 @@ int ObTabletPointerMap::get_meta_obj_with_external_memory( if (OB_FAIL(load_meta_obj(key, t_ptr, allocator, disk_addr, t))) { STORAGE_LOG(WARN, "load obj from disk fail", K(ret), K(key), KPC(t_ptr), K(lbt())); } else { + ObTenantMetaMemMgr *t3m = MTL(ObTenantMetaMemMgr*); ObTabletPointerHandle tmp_ptr_hdl(*this); common::ObBucketHashWLockGuard lock_guard(ResourceMap::bucket_lock_, hash_val); // some other thread finish loading @@ -660,13 +661,12 @@ int ObTabletPointerMap::get_meta_obj_with_external_memory( } } else if (OB_FAIL(t->deserialize_post_work(allocator))) { STORAGE_LOG(WARN, "fail to deserialize post work", K(ret), KP(t)); - } else { - ObTenantMetaMemMgr *t3m = MTL(ObTenantMetaMemMgr*); - guard.set_obj(t, &allocator, t3m); + } else if (OB_FAIL(t3m->inc_external_tablet_cnt(t->get_tablet_id().id(), t->get_transfer_seq()))) { // TODO FEIDU t->pointer_hdl_.reset(); (external tablet should not hold tablet_pointer) - if (OB_FAIL(t3m->inc_external_tablet_cnt(t->get_tablet_id().id(), t->get_transfer_seq()))) { - STORAGE_LOG(WARN, "fail to inc external tablet cnt", K(ret), KP(t), KPC(t)); - } + // !CAUTION: t3m->inc_external_tablet_cnt must be the last step which can modify ret; or, we have to dec_external_tablet_cnt in the failure process + STORAGE_LOG(WARN, "fail to inc external tablet cnt", K(ret), KP(t), KPC(t)); + } else { + guard.set_obj(t, &allocator, t3m); } } // write lock end if ((OB_FAIL(ret) && OB_NOT_NULL(t)) || need_free_obj) { diff --git a/src/storage/meta_mem/ob_tenant_meta_mem_mgr.cpp b/src/storage/meta_mem/ob_tenant_meta_mem_mgr.cpp index 12869dcc7..228a3a8ae 100644 --- a/src/storage/meta_mem/ob_tenant_meta_mem_mgr.cpp +++ b/src/storage/meta_mem/ob_tenant_meta_mem_mgr.cpp @@ -1693,10 +1693,13 @@ int ObTenantMetaMemMgr::get_tablet_with_allocator( } else if (OB_UNLIKELY(!handle.is_valid())) { ret = OB_ERR_UNEXPECTED; LOG_WARN("tablet is null", K(ret), K(key), K(handle)); - } else { - handle.set_wash_priority(priority); - if (&allocator == handle.get_allocator()) { - handle.disallow_copy_and_assign(); + } + + // get_meta_obj success or not set handle is disallow_copy_and_assign + handle.set_wash_priority(priority); + if (&allocator == handle.get_allocator()) { + handle.disallow_copy_and_assign(); + if (OB_NOT_NULL(handle.get_obj())) { handle.get_obj()->set_allocator(handle.get_allocator()); } }