From: Samuel Just <sjust@redhat.com>
Date: Thu, 18 Mar 2021 05:47:37 +0000 (-0700)
Subject: test/crimson/seastore: disable concurrent gc
X-Git-Tag: v17.1.0~2478^2~17
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=884efb6fc64d7e0a304a5ed63673d2fc011f6ece;p=ceph.git

test/crimson/seastore: disable concurrent gc

The concurrent gc configuration is simpler for the most part, but
greatly complicates these unit tests as they might hit an eagain or not
based on gc interleaving.  For the purposes of these unit tests (and
other basically functional tests), we'll instead disable the background
behavior and invoke it directly after each submitted transaction.

Signed-off-by: Samuel Just <sjust@redhat.com>
---

diff --git a/src/test/crimson/seastore/onode_tree/test_fltree_onode_manager.cc b/src/test/crimson/seastore/onode_tree/test_fltree_onode_manager.cc
index 3dad60850975..96c8384c3171 100644
--- a/src/test/crimson/seastore/onode_tree/test_fltree_onode_manager.cc
+++ b/src/test/crimson/seastore/onode_tree/test_fltree_onode_manager.cc
@@ -102,6 +102,7 @@ struct fltree_onode_manager_test_t
     auto t = tm->create_transaction();
     std::invoke(f, *t);
     tm->submit_transaction(std::move(t)).unsafe_get0();
+    segment_cleaner->run_until_halt().get0();
   }
 
   template <typename F>
diff --git a/src/test/crimson/seastore/test_collection_manager.cc b/src/test/crimson/seastore/test_collection_manager.cc
index 07cd36461c9e..511868444e6c 100644
--- a/src/test/crimson/seastore/test_collection_manager.cc
+++ b/src/test/crimson/seastore/test_collection_manager.cc
@@ -70,6 +70,10 @@ struct collection_manager_test_t :
     checking_mappings(coll_root, *t);
   }
 
+  void submit_transaction(TransactionRef &&t) {
+    tm->submit_transaction(std::move(t)).unsafe_get0();
+    segment_cleaner->run_until_halt().get0();
+  }
 };
 
 TEST_F(collection_manager_test_t, basic)
@@ -79,7 +83,7 @@ TEST_F(collection_manager_test_t, basic)
     {
       auto t = tm->create_transaction();
       coll_root = collection_manager->mkfs(*t).unsafe_get0();
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
     {
       auto t = tm->create_transaction();
@@ -89,7 +93,7 @@ TEST_F(collection_manager_test_t, basic)
         test_coll_mappings.emplace(cid, coll_info_t(i));
       }
       checking_mappings(coll_root, *t);
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
       EXPECT_EQ(test_coll_mappings.size(), 20);
     }
 
@@ -101,13 +105,13 @@ TEST_F(collection_manager_test_t, basic)
         collection_manager->remove(coll_root, *t, ite.first).unsafe_get0();
         test_coll_mappings.erase(ite.first);
       }
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
     replay();
     {
       auto t = tm->create_transaction();
       auto list_ret = collection_manager->list(coll_root, *t).unsafe_get0();
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
       EXPECT_EQ(list_ret.size(), test_coll_mappings.size());
     }
   });
@@ -120,7 +124,7 @@ TEST_F(collection_manager_test_t, overflow)
     {
       auto t = tm->create_transaction();
       coll_root = collection_manager->mkfs(*t).unsafe_get0();
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
     auto old_location = coll_root.get_location();
 
@@ -130,7 +134,7 @@ TEST_F(collection_manager_test_t, overflow)
       collection_manager->create(coll_root, *t, cid, coll_info_t(i)).unsafe_get0();
       test_coll_mappings.emplace(cid, coll_info_t(i));
     }
-    tm->submit_transaction(std::move(t)).unsafe_get();
+    submit_transaction(std::move(t));
     EXPECT_NE(old_location, coll_root.get_location());
     checking_mappings(coll_root);
 
@@ -146,7 +150,7 @@ TEST_F(collection_manager_test_t, update)
     {
       auto t = tm->create_transaction();
       coll_root = collection_manager->mkfs(*t).unsafe_get0();
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
     {
       auto t = tm->create_transaction();
@@ -155,7 +159,7 @@ TEST_F(collection_manager_test_t, update)
         collection_manager->create(coll_root, *t, cid, coll_info_t(i)).unsafe_get0();
         test_coll_mappings.emplace(cid, coll_info_t(i));
       }
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
     {
        auto iter1= test_coll_mappings.begin();
@@ -163,7 +167,7 @@ TEST_F(collection_manager_test_t, update)
        EXPECT_NE(iter1->second.split_bits, iter2->second.split_bits);
        auto t = tm->create_transaction();
        collection_manager->update(coll_root, *t, iter1->first, iter2->second).unsafe_get0();
-       tm->submit_transaction(std::move(t)).unsafe_get();
+       submit_transaction(std::move(t));
        iter1->second.split_bits = iter2->second.split_bits;
     }
     replay();
diff --git a/src/test/crimson/seastore/test_extmap_manager.cc b/src/test/crimson/seastore/test_extmap_manager.cc
index 8b25880119d8..c68a6dbc4672 100644
--- a/src/test/crimson/seastore/test_extmap_manager.cc
+++ b/src/test/crimson/seastore/test_extmap_manager.cc
@@ -114,7 +114,10 @@ struct extentmap_manager_test_t :
     logger().debug("{}: end", __func__);
   }
 
-
+  void submit_transaction(TransactionRef &&t) {
+    tm->submit_transaction(std::move(t)).unsafe_get0();
+    segment_cleaner->run_until_halt().get0();
+  }
 };
 
 TEST_F(extentmap_manager_test_t, basic)
@@ -124,7 +127,7 @@ TEST_F(extentmap_manager_test_t, basic)
     {
       auto t = tm->create_transaction();
       extmap_root = extmap_manager->initialize_extmap(*t).unsafe_get0();
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
 
     uint32_t len = 4096;
@@ -134,7 +137,7 @@ TEST_F(extentmap_manager_test_t, basic)
       logger().debug("first transaction");
       [[maybe_unused]] auto addref = insert_extent(extmap_root, *t, lo, {lo, len});
       [[maybe_unused]] auto seekref = find_extent(extmap_root, *t, lo, len);
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
     {
       auto t = tm->create_transaction();
@@ -142,13 +145,13 @@ TEST_F(extentmap_manager_test_t, basic)
       auto seekref = find_extent(extmap_root, *t, lo, len);
       rm_extent(extmap_root, *t, lo, {seekref.front().laddr, len});
       [[maybe_unused]] auto seekref2 = findno_extent(extmap_root, *t, lo, len);
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
     {
       auto t = tm->create_transaction();
       logger().debug("third transaction");
       [[maybe_unused]] auto seekref = findno_extent(extmap_root, *t, lo, len);
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
   });
 }
@@ -160,7 +163,7 @@ TEST_F(extentmap_manager_test_t, force_leafnode_split)
     {
       auto t = tm->create_transaction();
       extmap_root = extmap_manager->initialize_extmap(*t).unsafe_get0();
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
     uint32_t len = 4096;
     uint32_t lo = 0;
@@ -175,7 +178,7 @@ TEST_F(extentmap_manager_test_t, force_leafnode_split)
         }
       }
       logger().debug("force split submit transaction i = {}", i);
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
       check_mappings(extmap_root);
     }
   });
@@ -189,7 +192,7 @@ TEST_F(extentmap_manager_test_t, force_leafnode_split_merge)
     {
       auto t = tm->create_transaction();
       extmap_root = extmap_manager->initialize_extmap(*t).unsafe_get0();
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
     uint32_t len = 4096;
     uint32_t lo = 0;
@@ -204,7 +207,7 @@ TEST_F(extentmap_manager_test_t, force_leafnode_split_merge)
         }
       }
       logger().debug("submitting transaction");
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
       if (i % 50 == 0) {
         check_mappings(extmap_root);
       }
@@ -221,7 +224,7 @@ TEST_F(extentmap_manager_test_t, force_leafnode_split_merge)
 
       if (i % 10 == 0) {
         logger().debug("submitting transaction i= {}", i);
-        tm->submit_transaction(std::move(t)).unsafe_get();
+        submit_transaction(std::move(t));
         t = tm->create_transaction();
       }
       if (i % 100 == 0) {
@@ -231,7 +234,7 @@ TEST_F(extentmap_manager_test_t, force_leafnode_split_merge)
       }
     }
     logger().debug("finally submitting transaction ");
-    tm->submit_transaction(std::move(t)).unsafe_get();
+    submit_transaction(std::move(t));
   });
 }
 
@@ -242,7 +245,7 @@ TEST_F(extentmap_manager_test_t, force_leafnode_split_merge_replay)
     {
       auto t = tm->create_transaction();
       extmap_root = extmap_manager->initialize_extmap(*t).unsafe_get0();
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
       replay();
     }
     uint32_t len = 4096;
@@ -255,7 +258,7 @@ TEST_F(extentmap_manager_test_t, force_leafnode_split_merge_replay)
         lo += len;
       }
       logger().debug("submitting transaction");
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
     replay();
     auto t = tm->create_transaction();
@@ -268,7 +271,7 @@ TEST_F(extentmap_manager_test_t, force_leafnode_split_merge_replay)
 
       if (i % 10 == 0) {
         logger().debug("submitting transaction i= {}", i);
-        tm->submit_transaction(std::move(t)).unsafe_get();
+        submit_transaction(std::move(t));
         t = tm->create_transaction();
       }
       if (i% 100 == 0){
@@ -276,7 +279,7 @@ TEST_F(extentmap_manager_test_t, force_leafnode_split_merge_replay)
       }
     }
     logger().debug("finally submitting transaction ");
-    tm->submit_transaction(std::move(t)).unsafe_get();
+    submit_transaction(std::move(t));
     replay();
     check_mappings(extmap_root);
   });
diff --git a/src/test/crimson/seastore/test_omap_manager.cc b/src/test/crimson/seastore/test_omap_manager.cc
index 699ea3a086a4..04ea79bbdd06 100644
--- a/src/test/crimson/seastore/test_omap_manager.cc
+++ b/src/test/crimson/seastore/test_omap_manager.cc
@@ -185,6 +185,11 @@ struct omap_manager_test_t :
     omap_manager = omap_manager::create_omap_manager(*tm);
     logger().debug("{}: end", __func__);
   }
+
+  void submit_transaction(TransactionRef &&t) {
+    tm->submit_transaction(std::move(t)).unsafe_get0();
+    segment_cleaner->run_until_halt().get0();
+  }
 };
 
 TEST_F(omap_manager_test_t, basic)
@@ -194,7 +199,7 @@ TEST_F(omap_manager_test_t, basic)
     {
       auto t = tm->create_transaction();
       omap_root = omap_manager->initialize_omap(*t).unsafe_get0();
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
 
     string key = "owner";
@@ -205,7 +210,7 @@ TEST_F(omap_manager_test_t, basic)
       logger().debug("first transaction");
       set_key(omap_root, *t, key, val);
       get_value(omap_root, *t, key);
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
     {
       auto t = tm->create_transaction();
@@ -213,13 +218,13 @@ TEST_F(omap_manager_test_t, basic)
       get_value(omap_root, *t, key);
       rm_key(omap_root, *t, key);
       get_value(omap_root, *t, key);
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
     {
       auto t = tm->create_transaction();
       logger().debug("third transaction");
       get_value(omap_root, *t, key);
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
   });
 }
@@ -231,7 +236,7 @@ TEST_F(omap_manager_test_t, force_leafnode_split)
     {
       auto t = tm->create_transaction();
       omap_root = omap_manager->initialize_omap(*t).unsafe_get0();
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
     for (unsigned i = 0; i < 40; i++) {
       auto t = tm->create_transaction();
@@ -243,7 +248,7 @@ TEST_F(omap_manager_test_t, force_leafnode_split)
         }
       }
       logger().debug("force split submit transaction i = {}", i);
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
       check_mappings(omap_root);
     }
   });
@@ -256,7 +261,7 @@ TEST_F(omap_manager_test_t, force_leafnode_split_merge)
     {
       auto t = tm->create_transaction();
       omap_root = omap_manager->initialize_omap(*t).unsafe_get0();
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
 
     for (unsigned i = 0; i < 80; i++) {
@@ -269,7 +274,7 @@ TEST_F(omap_manager_test_t, force_leafnode_split_merge)
         }
       }
       logger().debug("submitting transaction");
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
       if (i % 50 == 0) {
         check_mappings(omap_root);
       }
@@ -283,7 +288,7 @@ TEST_F(omap_manager_test_t, force_leafnode_split_merge)
 
       if (i % 10 == 0) {
         logger().debug("submitting transaction i= {}", i);
-        tm->submit_transaction(std::move(t)).unsafe_get();
+        submit_transaction(std::move(t));
         t = tm->create_transaction();
       }
       if (i % 100 == 0) {
@@ -294,7 +299,7 @@ TEST_F(omap_manager_test_t, force_leafnode_split_merge)
       i++;
     }
     logger().debug("finally submitting transaction ");
-    tm->submit_transaction(std::move(t)).unsafe_get();
+    submit_transaction(std::move(t));
   });
 }
 
@@ -305,7 +310,7 @@ TEST_F(omap_manager_test_t, force_leafnode_split_merge_fullandbalanced)
     {
       auto t = tm->create_transaction();
       omap_root = omap_manager->initialize_omap(*t).unsafe_get0();
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
 
     for (unsigned i = 0; i < 50; i++) {
@@ -318,7 +323,7 @@ TEST_F(omap_manager_test_t, force_leafnode_split_merge_fullandbalanced)
         }
       }
       logger().debug("submitting transaction");
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
       if (i % 50 == 0) {
         check_mappings(omap_root);
       }
@@ -333,7 +338,7 @@ TEST_F(omap_manager_test_t, force_leafnode_split_merge_fullandbalanced)
 
       if (i % 10 == 0) {
       logger().debug("submitting transaction i= {}", i);
-        tm->submit_transaction(std::move(t)).unsafe_get();
+        submit_transaction(std::move(t));
         t = tm->create_transaction();
       }
       if (i % 50 == 0) {
@@ -346,7 +351,7 @@ TEST_F(omap_manager_test_t, force_leafnode_split_merge_fullandbalanced)
  break;
     }
     logger().debug("finally submitting transaction ");
-    tm->submit_transaction(std::move(t)).unsafe_get();
+    submit_transaction(std::move(t));
     check_mappings(omap_root);
   });
 }
@@ -358,7 +363,7 @@ TEST_F(omap_manager_test_t, force_split_listkeys_list_clear)
     {
       auto t = tm->create_transaction();
       omap_root = omap_manager->initialize_omap(*t).unsafe_get0();
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
     string temp;
     for (unsigned i = 0; i < 40; i++) {
@@ -373,7 +378,7 @@ TEST_F(omap_manager_test_t, force_split_listkeys_list_clear)
         }
       }
       logger().debug("force split submit transaction i = {}", i);
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
       check_mappings(omap_root);
     }
 
@@ -390,7 +395,7 @@ TEST_F(omap_manager_test_t, force_split_listkeys_list_clear)
     {
       auto t = tm->create_transaction();
       clear(omap_root, *t);
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
   });
 }
@@ -402,7 +407,7 @@ TEST_F(omap_manager_test_t, internal_force_split)
     {
       auto t = tm->create_transaction();
       omap_root = omap_manager->initialize_omap(*t).unsafe_get0();
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
     for (unsigned i = 0; i < 10; i++) {
       logger().debug("opened split transaction");
@@ -415,7 +420,7 @@ TEST_F(omap_manager_test_t, internal_force_split)
         }
       }
       logger().debug("submitting transaction i = {}", i);
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
     check_mappings(omap_root);
   });
@@ -428,7 +433,7 @@ TEST_F(omap_manager_test_t, internal_force_merge_fullandbalanced)
     {
       auto t = tm->create_transaction();
       omap_root = omap_manager->initialize_omap(*t).unsafe_get0();
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
 
     for (unsigned i = 0; i < 8; i++) {
@@ -442,7 +447,7 @@ TEST_F(omap_manager_test_t, internal_force_merge_fullandbalanced)
         }
       }
       logger().debug("submitting transaction");
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
     auto t = tm->create_transaction();
     int i = 0;
@@ -452,7 +457,7 @@ TEST_F(omap_manager_test_t, internal_force_merge_fullandbalanced)
 
       if (i % 10 == 0) {
       logger().debug("submitting transaction i= {}", i);
-        tm->submit_transaction(std::move(t)).unsafe_get();
+        submit_transaction(std::move(t));
         t = tm->create_transaction();
       }
       if (i % 50 == 0) {
@@ -463,7 +468,7 @@ TEST_F(omap_manager_test_t, internal_force_merge_fullandbalanced)
       i++;
     }
     logger().debug("finally submitting transaction ");
-    tm->submit_transaction(std::move(t)).unsafe_get();
+    submit_transaction(std::move(t));
     check_mappings(omap_root);
   });
 }
@@ -475,7 +480,7 @@ TEST_F(omap_manager_test_t, replay)
     {
       auto t = tm->create_transaction();
       omap_root = omap_manager->initialize_omap(*t).unsafe_get0();
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
       replay();
     }
 
@@ -490,7 +495,7 @@ TEST_F(omap_manager_test_t, replay)
         }
       }
       logger().debug("submitting transaction i = {}", i);
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
     replay();
     check_mappings(omap_root);
@@ -503,7 +508,7 @@ TEST_F(omap_manager_test_t, replay)
 
       if (i % 10 == 0) {
       logger().debug("submitting transaction i= {}", i);
-        tm->submit_transaction(std::move(t)).unsafe_get();
+        submit_transaction(std::move(t));
         replay();
         t = tm->create_transaction();
       }
@@ -515,7 +520,7 @@ TEST_F(omap_manager_test_t, replay)
       i++;
     }
     logger().debug("finally submitting transaction ");
-    tm->submit_transaction(std::move(t)).unsafe_get();
+    submit_transaction(std::move(t));
     replay();
     check_mappings(omap_root);
   });
@@ -529,7 +534,7 @@ TEST_F(omap_manager_test_t, internal_force_split_to_root)
     {
       auto t = tm->create_transaction();
       omap_root = omap_manager->initialize_omap(*t).unsafe_get0();
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
 
     logger().debug("set big keys");
@@ -540,7 +545,7 @@ TEST_F(omap_manager_test_t, internal_force_split_to_root)
         set_random_key(omap_root, *t);
       }
       logger().debug("submitting transaction i = {}", i);
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
     }
      logger().debug("set small keys");
      for (unsigned i = 0; i < 100; i++) {
@@ -549,7 +554,7 @@ TEST_F(omap_manager_test_t, internal_force_split_to_root)
          set_random_key(omap_root, *t);
        }
       logger().debug("submitting transaction last");
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
      }
     check_mappings(omap_root);
   });
diff --git a/src/test/crimson/seastore/test_transaction_manager.cc b/src/test/crimson/seastore/test_transaction_manager.cc
index 91b39917dad4..745efae1ef69 100644
--- a/src/test/crimson/seastore/test_transaction_manager.cc
+++ b/src/test/crimson/seastore/test_transaction_manager.cc
@@ -513,11 +513,16 @@ struct transaction_manager_test_t :
       [](const crimson::ct_error::eagain &e) {
 	return seastar::make_ready_future<bool>(false);
       },
-      crimson::ct_error::pass_further_all{}
-    ).unsafe_get0();
+      crimson::ct_error::assert_all{
+	"try_submit_transaction hit invalid error"
+      }
+    ).then([this](auto ret) {
+      return segment_cleaner->run_until_halt().then([ret] { return ret; });
+    }).get0();
 
-    if (success)
+    if (success) {
       test_mappings.consume(t.mapping_delta);
+    }
 
     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 0158da44ce0d..efcb25d02067 100644
--- a/src/test/crimson/seastore/transaction_manager_test_state.h
+++ b/src/test/crimson/seastore/transaction_manager_test_state.h
@@ -143,7 +143,11 @@ protected:
     return tm->mount(
     ).handle_error(
       crimson::ct_error::assert_all{"Error in mount"}
-    );
+    ).then([this] {
+      return segment_cleaner->stop();
+    }).then([this] {
+      return segment_cleaner->run_until_halt();
+    });
   }
 
   virtual seastar::future<> _mkfs() {