From 3873ac965a902418b3b288bef00235724302f566 Mon Sep 17 00:00:00 2001
From: myoungwon oh <ohmyoungwon@gmail.com>
Date: Tue, 26 Jul 2022 09:46:29 +0900
Subject: [PATCH] test/seastore: add write seqeunce to avoid racing

In the middle of test_random_write_concurrrent(), there is a chance
that consume(laddr A), regarding transaction 1, in try_submit_transaction
can be called after transaction 2's consume(laddr A) is invoked (note that
seastore handles both transaction 1 and 2 in order, and each other's laddr
are the same).

To avoid this, this commit adds a write seqeunce to prevent overwriting
old checksum associated with laddr---old checksum will not be updated
after write sequence comparison.

Signed-off-by: Myoungwon Oh <myoungwon.oh@samsung.com>
---
 .../seastore/test_transaction_manager.cc        | 17 ++++++++++++-----
 .../seastore/transaction_manager_test_state.h   | 12 ++++++++++++
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/src/test/crimson/seastore/test_transaction_manager.cc b/src/test/crimson/seastore/test_transaction_manager.cc
index 6cfc5b607331..aa61f167ba74 100644
--- a/src/test/crimson/seastore/test_transaction_manager.cc
+++ b/src/test/crimson/seastore/test_transaction_manager.cc
@@ -89,6 +89,7 @@ struct transaction_manager_test_t :
 
   struct test_extents_t : std::map<laddr_t, test_extent_record_t> {
     using delta_t = std::map<laddr_t, std::optional<test_extent_record_t>>;
+    std::map<laddr_t, uint64_t> laddr_write_seq;
 
     struct delta_overlay_t {
       const test_extents_t &extents;
@@ -339,10 +340,14 @@ struct transaction_manager_test_t :
       }
     }
 
-    void consume(const delta_t &delta) {
+    void consume(const delta_t &delta, const uint64_t write_seq = 0) {
       for (const auto &i : delta) {
 	if (i.second) {
-	  (*this)[i.first] = *i.second;
+	  if (laddr_write_seq.find(i.first) == laddr_write_seq.end() ||
+	      laddr_write_seq[i.first] <= write_seq) {
+	    (*this)[i.first] = *i.second;
+	    laddr_write_seq[i.first] = write_seq;
+	  }
 	} else {
 	  erase(i.first);
 	}
@@ -591,8 +596,10 @@ struct transaction_manager_test_t :
   bool try_submit_transaction(test_transaction_t t) {
     using ertr = with_trans_ertr<TransactionManager::submit_transaction_iertr>;
     using ret = ertr::future<bool>;
-    bool success = submit_transaction_fut(*t.t
-    ).safe_then([]() -> ret {
+    uint64_t write_seq = 0;
+    bool success = submit_transaction_fut_with_seq(*t.t
+    ).safe_then([&write_seq](auto seq) -> ret {
+      write_seq = seq;
       return ertr::make_ready_future<bool>(true);
     }).handle_error(
       [](const crimson::ct_error::eagain &e) {
@@ -606,7 +613,7 @@ struct transaction_manager_test_t :
     }).get0();
 
     if (success) {
-      test_mappings.consume(t.mapping_delta);
+      test_mappings.consume(t.mapping_delta, write_seq);
     }
 
     return success;
diff --git a/src/test/crimson/seastore/transaction_manager_test_state.h b/src/test/crimson/seastore/transaction_manager_test_state.h
index 1245999a2fa1..5a7800d9ec46 100644
--- a/src/test/crimson/seastore/transaction_manager_test_state.h
+++ b/src/test/crimson/seastore/transaction_manager_test_state.h
@@ -148,6 +148,7 @@ protected:
   BackrefManager *backref_manager;
   Cache* cache;
   AsyncCleaner *async_cleaner;
+  uint64_t seq = 0;
 
   TMTestState() : EphemeralTestState(1) {}
 
@@ -249,6 +250,17 @@ protected:
 	return tm->submit_transaction(t);
       });
   }
+  auto submit_transaction_fut_with_seq(Transaction &t) {
+    using ertr = TransactionManager::base_iertr;
+    return with_trans_intr(
+      t,
+      [this](auto &t) {
+	return tm->submit_transaction(t
+	).si_then([this] {
+	  return ertr::make_ready_future<uint64_t>(seq++);
+	});
+      });
+  }
 
   void submit_transaction(TransactionRef t) {
     submit_transaction_fut(*t).unsafe_get0();
-- 
2.47.3