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

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

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(

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
+   * Construct the record for Journal from transaction.
-   * an extent mutated by this transaction, nullopt will be returned.
-   *
-   * Otherwise, a record will be returned valid for use with Journal.
    */
-  std::optional<record_t> try_construct_record(
+  record_t try_construct_record(
     Transaction &t ///< [in, out] current transaction
   );
 

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);

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) {
+    return journal.submit_record(std::move(record), t->get_handle()).safe_then(
-      ceph_assert(0 == "cannot fail");
-    }
-
-    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);

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) {
+		  [](auto p) {});
-		    ASSERT_TRUE(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();
+      submit_transaction(std::move(t)).get0();
-      ASSERT_TRUE(ret);
       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();
+      submit_transaction(std::move(t)).get0();
-      ASSERT_TRUE(ret);
       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();
+      submit_transaction(std::move(t)).get0();
-      ASSERT_TRUE(ret);
       ASSERT_TRUE(extent->is_dirty());
       ASSERT_EQ(addr, extent->get_paddr());
       ASSERT_EQ(extent->get_version(), 1);