From: Zhang Song Date: Fri, 15 Jul 2022 06:40:46 +0000 (+0800) Subject: crimson/os/seastore: fix bugs in test_map_existing_extent_concurrent X-Git-Tag: v18.0.0~495^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=29dded0bee55d34f3b7f00fb843ce715111526fd;p=ceph.git crimson/os/seastore: fix bugs in test_map_existing_extent_concurrent Signed-off-by: Zhang Song --- diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index d5c153df43cf..6a5b82f047f4 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -388,6 +388,8 @@ public: ceph_assert(existing_paddr.is_absolute()); assert(t.is_retired(existing_paddr, length)); + SUBDEBUGT(seastore_tm, " laddr_hint: {} existing_paddr: {} length: {}", + t, laddr_hint, existing_paddr, length); auto bp = ceph::bufferptr(buffer::create_page_aligned(length)); bp.zero(); @@ -407,9 +409,7 @@ public: laddr_hint, length, existing_paddr - ).si_then([ext=std::move(ext), laddr_hint, &t, this, FNAME](auto &&ref) { - SUBDEBUGT(seastore_tm, "map existing extent: {}, laddr_hint: {} pin: {}", - t, *ext, laddr_hint, *ref); + ).si_then([ext=std::move(ext), laddr_hint, this](auto &&ref) { ceph_assert(laddr_hint == ref->get_key()); ext->set_pin(std::move(ref)); return epm->read( diff --git a/src/test/crimson/seastore/test_transaction_manager.cc b/src/test/crimson/seastore/test_transaction_manager.cc index 34f379511289..e3b0f486619b 100644 --- a/src/test/crimson/seastore/test_transaction_manager.cc +++ b/src/test/crimson/seastore/test_transaction_manager.cc @@ -754,18 +754,27 @@ struct transaction_manager_test_t : }); } - TestBlockRef map_existing_extent( + std::optional map_existing_extent( test_transaction_t &t, laddr_t hint, paddr_t existing_paddr, extent_len_t length) { + if (t.t->is_conflicted()) { + return std::nullopt; + } auto extent = with_trans_intr(*(t.t), [&](auto& trans) { return tm->map_existing_extent(trans, hint, existing_paddr, length); - }).unsafe_get0(); + }).handle_error(crimson::ct_error::eagain::handle([] { + return TCachedExtentRef(new TestBlock(0)); + }), crimson::ct_error::pass_further_all{}).unsafe_get0(); + if (t.t->is_conflicted()) { + return std::nullopt; + } + EXPECT_TRUE(extent->get_length() != 0); EXPECT_FALSE(test_mappings.contains(extent->get_laddr(), t.mapping_delta)); EXPECT_EQ(length, extent->get_length()); test_mappings.alloced(hint, *extent, t.mapping_delta); - return extent; + return std::make_optional(std::move(extent)); } void test_map_existing_extent() { @@ -783,14 +792,17 @@ struct transaction_manager_test_t : auto base_paddr = extent->get_paddr(); dec_ref(t, offset); auto extent1 = map_existing_extent(t, offset, base_paddr, 4 << 10); + ASSERT_TRUE(extent1.has_value()); auto extent2 = map_existing_extent(t, offset + (4 << 10), base_paddr.add_offset(4 << 10), 4 << 10); + ASSERT_TRUE(extent2.has_value()); auto extent3 = map_existing_extent(t, offset + (8 << 10), base_paddr.add_offset(8 << 10), 8 << 10); - ASSERT_TRUE(extent1->is_exist_clean()); - ASSERT_TRUE(extent2->is_exist_clean()); - ASSERT_TRUE(extent3->is_exist_clean()); - auto extent4 = mutate_extent(t, extent3); + ASSERT_TRUE(extent3.has_value()); + ASSERT_TRUE((*extent1)->is_exist_clean()); + ASSERT_TRUE((*extent2)->is_exist_clean()); + ASSERT_TRUE((*extent3)->is_exist_clean()); + auto extent4 = mutate_extent(t, (*extent3)); ASSERT_TRUE(extent4->is_exist_mutation_pending()); - ASSERT_TRUE(extent3.get() == extent4.get()); + ASSERT_TRUE((*extent3).get() == extent4.get()); submit_transaction(std::move(t)); check(); } @@ -812,6 +824,7 @@ struct transaction_manager_test_t : } int success = 0; int early_exit = 0; + int conflicted = 0; seastar::parallel_for_each( boost::make_counting_iterator(0u), @@ -822,7 +835,7 @@ struct transaction_manager_test_t : std::set split_points; for (uint32_t i = 0; i < pieces; i++) { auto p = std::uniform_int_distribution<>(1, 256)(gen); - split_points.insert((p - p % 4)%length); + split_points.insert(p - p % 4); } auto t = create_transaction(); @@ -835,12 +848,18 @@ struct transaction_manager_test_t : dec_ref(t, offset); auto base = 0; + ASSERT_TRUE(!split_points.empty()); for (auto off : split_points) { if (off == 0) { continue; } - auto ext = map_existing_extent(t, base << 10, paddr.add_offset(base << 10), (off - base) << 10); + auto ext_ = map_existing_extent(t, base << 10, paddr.add_offset(base << 10), (off - base) << 10); + if (!ext_) { + conflicted++; + return; + } + auto ext = *ext_; ASSERT_TRUE(ext->is_exist_clean()); if (get_random_contents() % 2 == 0) { auto ext1 = mutate_extent(t, ext); @@ -848,20 +867,34 @@ struct transaction_manager_test_t : } base = off; } - auto ext = map_existing_extent(t, base << 10, paddr.add_offset(base << 10), length - (base << 10)); - ASSERT_TRUE(ext->is_exist_clean()); - if (get_random_contents() % 2 == 0) { - auto ext1 = mutate_extent(t, ext); - ASSERT_TRUE(ext1->is_exist_mutation_pending()); - } - success += try_submit_transaction(std::move(t)); + base <<= 10; + if (base != length) { + auto ext_ = map_existing_extent(t, base, paddr.add_offset(base), length - base); + if (!ext_) { + conflicted++; + return; + } + auto ext = *ext_; + ASSERT_TRUE(ext->is_exist_clean()); + if (get_random_contents() % 2 == 0) { + auto ext1 = mutate_extent(t, ext); + ASSERT_TRUE(ext1->is_exist_mutation_pending()); + } + } + if (try_submit_transaction(std::move(t))) { + success++; + logger().info("transaction {} submit the transction", static_cast(t.t.get())); + } else { + conflicted++; + } }); }).handle_exception([](std::exception_ptr e) { logger().info("{}", e); }).get0(); + logger().info("test_map_existing_extent_concurrent: early_exit {} conflicted {} success {}", early_exit, conflicted, success); ASSERT_TRUE(success == 1); - logger().info("test_map_existing_extent_concurrent: early_exit {}", early_exit); + ASSERT_EQ(success + conflicted + early_exit, REMAP_NUM); replay(); check(); });