crimson/os/seastore/cache: cleanup try_construct_record() optional return

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>

diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc
index 7c9d883c5741986dfab08977e96e63c7e45d03f7..ce4df9360459ed611fcc36c5c5038559e8556d5a 100644 (file)
--- a/src/crimson/os/seastore/cache.cc
+++ b/src/crimson/os/seastore/cache.cc
@@ -235,7 +235,7 @@ CachedExtentRef Cache::duplicate_for_write(
   return ret;
 }
 
-std::optional<record_t> Cache::try_construct_record(Transaction &t)
+record_t Cache::try_construct_record(Transaction &t)
 {
   LOG_PREFIX(Cache::try_construct_record);
   DEBUGT("enter", t);
@@ -343,7 +343,7 @@ std::optional<record_t> Cache::try_construct_record(Transaction &t)
       });
   }
 
-  return std::make_optional<record_t>(std::move(record));
+  return record;
 }
 
 void Cache::complete_commit(

diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index 1a9998cc7f2a886a16c81b124b1e6fd285ef044e..551f6e7d0a7c56274444ca0cc69b4f850dce8272 100644 (file)
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -372,12 +372,9 @@ public:
   /**
    * try_construct_record
    *
-   * First checks for conflicts.  If a racing write has mutated/retired
-   * an extent mutated by this transaction, nullopt will be returned.
-   *
-   * Otherwise, a record will be returned valid for use with Journal.
+   * Construct the record for Journal from transaction.
    */
-  std::optional<record_t> try_construct_record(
+  record_t try_construct_record(
     Transaction &t ///< [in, out] current transaction
   );
 

diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc
index 8f34581eb4445ebe019f745e5dc86a3f1001f92d..688e3e803970093535f3c09f55913df604d41414 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.cc
+++ b/src/crimson/os/seastore/transaction_manager.cc
@@ -240,13 +240,12 @@ TransactionManager::submit_transaction_direct(
   ).then_interruptible([this, FNAME, &tref]() mutable
                       -> submit_transaction_iertr::future<> {
     auto record = cache->try_construct_record(tref);
-    assert(record); // interruptible future would have already failed
 
     tref.get_handle().maybe_release_collection_lock();
 
     DEBUGT("about to submit to journal", tref);
 
-    return journal->submit_record(std::move(*record), tref.get_handle()
+    return journal->submit_record(std::move(record), tref.get_handle()
     ).safe_then([this, FNAME, &tref](auto p) mutable {
       auto [addr, journal_seq] = p;
       DEBUGT("journal commit to {} seq {}", tref, addr, journal_seq);

diff --git a/src/test/crimson/seastore/test_btree_lba_manager.cc b/src/test/crimson/seastore/test_btree_lba_manager.cc
index a4b6d1db339f787ba0c07da790a570ebe2f2d022..175ca86eaff05a792a20c756d8f96633098895dd 100644 (file)
--- a/src/test/crimson/seastore/test_btree_lba_manager.cc
+++ b/src/test/crimson/seastore/test_btree_lba_manager.cc
@@ -59,11 +59,7 @@ struct btree_lba_manager_test :
   seastar::future<> submit_transaction(TransactionRef t)
   {
     auto record = cache.try_construct_record(*t);
-    if (!record) {
-      ceph_assert(0 == "cannot fail");
-    }
-
-    return journal.submit_record(std::move(*record), t->get_handle()).safe_then(
+    return journal.submit_record(std::move(record), t->get_handle()).safe_then(
       [this, t=std::move(t)](auto p) mutable {
        auto [addr, seq] = p;
        cache.complete_commit(*t, addr, seq);

diff --git a/src/test/crimson/seastore/test_seastore_cache.cc b/src/test/crimson/seastore/test_seastore_cache.cc
index 31b7720909071b453213dd5a0aded6348b8d4193..09cf6dc0562bf65ed6cc48f0b4dc11b09f5ea6ac 100644 (file)
--- a/src/test/crimson/seastore/test_seastore_cache.cc
+++ b/src/test/crimson/seastore/test_seastore_cache.cc
@@ -29,16 +29,12 @@ struct cache_test_t : public seastar_test_suite_t {
     : segment_manager(segment_manager::create_test_ephemeral()),
       cache(*segment_manager) {}
 
-  seastar::future<std::optional<paddr_t>> submit_transaction(
+  seastar::future<paddr_t> submit_transaction(
     TransactionRef t) {
     auto record = cache.try_construct_record(*t);
-    if (!record) {
-      return seastar::make_ready_future<std::optional<paddr_t>>(
-       std::nullopt);
-    }
 
     bufferlist bl;
-    for (auto &&block : record->extents) {
+    for (auto &&block : record.extents) {
       bl.append(block.bl);
     }
 
@@ -57,7 +53,7 @@ struct cache_test_t : public seastar_test_suite_t {
     ).safe_then(
       [this, prev, t=std::move(t)]() mutable {
        cache.complete_commit(*t, prev, seq /* TODO */);
-       return seastar::make_ready_future<std::optional<paddr_t>>(prev);
+        return prev;
       },
       crimson::ct_error::all_same_way([](auto e) {
        ASSERT_FALSE("failed to submit");
@@ -90,9 +86,7 @@ struct cache_test_t : public seastar_test_suite_t {
            return cache.mkfs(*transaction).safe_then(
              [this, &transaction] {
                return submit_transaction(std::move(transaction)).then(
-                 [](auto p) {
-                   ASSERT_TRUE(p);
-                 });
+                 [](auto p) {});
              });
          });
       }).handle_error(
@@ -120,8 +114,7 @@ TEST_F(cache_test_t, test_addr_fixup)
        TestBlockPhysical::SIZE);
       extent->set_contents('c');
       csum = extent->get_crc32c();
-      auto ret = submit_transaction(std::move(t)).get0();
-      ASSERT_TRUE(ret);
+      submit_transaction(std::move(t)).get0();
       addr = extent->get_paddr();
     }
     {
@@ -165,8 +158,7 @@ TEST_F(cache_test_t, test_dirty_extent)
        ASSERT_EQ(extent->get_version(), 0);
        ASSERT_EQ(csum, extent->get_crc32c());
       }
-      auto ret = submit_transaction(std::move(t)).get0();
-      ASSERT_TRUE(ret);
+      submit_transaction(std::move(t)).get0();
       addr = extent->get_paddr();
     }
     {
@@ -222,8 +214,7 @@ TEST_F(cache_test_t, test_dirty_extent)
        ASSERT_EQ(csum2, extent->get_crc32c());
       }
       // submit transaction
-      auto ret = submit_transaction(std::move(t)).get0();
-      ASSERT_TRUE(ret);
+      submit_transaction(std::move(t)).get0();
       ASSERT_TRUE(extent->is_dirty());
       ASSERT_EQ(addr, extent->get_paddr());
       ASSERT_EQ(extent->get_version(), 1);