Split SnapshotConcurrentAccessTest into 20 sub tests

Summary:
SnapshotConcurrentAccessTest sometimes times out when running on the test infra. This patch splits the test into smaller sub-tests to avoid the timeout. It also benefits from lower run-time of each sub-test and increases the coverage of the test. The overall run-time of each final sub-test is at most half of the original test so we should no longer see a timeout.
Closes https://github.com/facebook/rocksdb/pull/3435

Differential Revision: D6839427

Pulled By: maysamyabandeh

fbshipit-source-id: d53fdb157109e2438ca7fe447d0cf4b71f304bd8

utilities/transactions/transaction_test.h

@@ -36,8 +36,7 @@
 
 namespace rocksdb {
 
-class TransactionTest : public ::testing::TestWithParam<
+class TransactionTestBase : public ::testing::Test {
-                            std::tuple<bool, bool, TxnDBWritePolicy>> {
  public:
   TransactionDB* db;
   FaultInjectionTestEnv* env;
@@ -45,8 +44,11 @@ class TransactionTest : public ::testing::TestWithParam<
   Options options;
 
   TransactionDBOptions txn_db_options;
+  bool use_stackable_db_;
 
-  TransactionTest() {
+  TransactionTestBase(bool use_stackable_db, bool two_write_queue,
+                      TxnDBWritePolicy write_policy)
+      : use_stackable_db_(use_stackable_db) {
     options.create_if_missing = true;
     options.max_write_buffer_number = 2;
     options.write_buffer_size = 4 * 1024;
@@ -54,15 +56,15 @@ class TransactionTest : public ::testing::TestWithParam<
     options.merge_operator = MergeOperators::CreateFromStringId("stringappend");
     env = new FaultInjectionTestEnv(Env::Default());
     options.env = env;
-    options.two_write_queues = std::get<1>(GetParam());
+    options.two_write_queues = two_write_queue;
     dbname = test::TmpDir() + "/transaction_testdb";
 
     DestroyDB(dbname, options);
     txn_db_options.transaction_lock_timeout = 0;
     txn_db_options.default_lock_timeout = 0;
-    txn_db_options.write_policy = std::get<2>(GetParam());
+    txn_db_options.write_policy = write_policy;
     Status s;
-    if (std::get<0>(GetParam()) == false) {
+    if (use_stackable_db == false) {
       s = TransactionDB::Open(options, txn_db_options, dbname, &db);
     } else {
       s = OpenWithStackableDB();
@@ -70,7 +72,7 @@ class TransactionTest : public ::testing::TestWithParam<
     assert(s.ok());
   }
 
-  ~TransactionTest() {
+  ~TransactionTestBase() {
     delete db;
     // This is to skip the assert statement in FaultInjectionTestEnv. There
     // seems to be a bug in btrfs that the makes readdir return recently
@@ -88,7 +90,7 @@ class TransactionTest : public ::testing::TestWithParam<
     env->DropUnsyncedFileData();
     env->ResetState();
     Status s;
-    if (std::get<0>(GetParam()) == false) {
+    if (use_stackable_db_ == false) {
       s = TransactionDB::Open(options, txn_db_options, dbname, &db);
     } else {
       s = OpenWithStackableDB();
@@ -100,7 +102,7 @@ class TransactionTest : public ::testing::TestWithParam<
     delete db;
     DestroyDB(dbname, options);
     Status s;
-    if (std::get<0>(GetParam()) == false) {
+    if (use_stackable_db_ == false) {
       s = TransactionDB::Open(options, txn_db_options, dbname, &db);
     } else {
       s = OpenWithStackableDB();
@@ -397,6 +399,15 @@ class TransactionTest : public ::testing::TestWithParam<
   }
 };
 
+class TransactionTest : public TransactionTestBase,
+                        public ::testing::WithParamInterface<
+                            std::tuple<bool, bool, TxnDBWritePolicy>> {
+ public:
+  TransactionTest()
+      : TransactionTestBase(std::get<0>(GetParam()), std::get<1>(GetParam()),
+                            std::get<2>(GetParam())){};
+};
+
 class MySQLStyleTransactionTest : public TransactionTest {};
 
 }  // namespace rocksdb

utilities/transactions/write_prepared_transaction_test.cc

@@ -181,7 +181,12 @@ class WritePreparedTxnDBMock : public WritePreparedTxnDB {
   std::vector<SequenceNumber> snapshots_;
 };
 
-class WritePreparedTransactionTest : public TransactionTest {
+class WritePreparedTransactionTestBase : public TransactionTestBase {
+ public:
+  WritePreparedTransactionTestBase(bool use_stackable_db, bool two_write_queue,
+                                   TxnDBWritePolicy write_policy)
+      : TransactionTestBase(use_stackable_db, two_write_queue, write_policy){};
+
  protected:
   // If expect_update is set, check if it actually updated old_commit_map_. If
   // it did not and yet suggested not to check the next snapshot, do the
@@ -342,11 +347,87 @@ class WritePreparedTransactionTest : public TransactionTest {
   }
 };
 
+class WritePreparedTransactionTest
+    : public WritePreparedTransactionTestBase,
+      public ::testing::WithParamInterface<
+          std::tuple<bool, bool, TxnDBWritePolicy>> {
+ public:
+  WritePreparedTransactionTest()
+      : WritePreparedTransactionTestBase(std::get<0>(GetParam()),
+                                         std::get<1>(GetParam()),
+                                         std::get<2>(GetParam())){};
+};
+
+class SnapshotConcurrentAccessTest
+    : public WritePreparedTransactionTestBase,
+      public ::testing::WithParamInterface<
+          std::tuple<bool, bool, TxnDBWritePolicy, size_t, size_t>> {
+ public:
+  SnapshotConcurrentAccessTest()
+      : WritePreparedTransactionTestBase(std::get<0>(GetParam()),
+                                         std::get<1>(GetParam()),
+                                         std::get<2>(GetParam())),
+        split_id_(std::get<3>(GetParam())),
+        split_cnt_(std::get<4>(GetParam())){};
+
+ protected:
+  // A test is split into split_cnt_ tests, each identified with split_id_ where
+  // 0 <= split_id_ < split_cnt_
+  size_t split_id_;
+  size_t split_cnt_;
+};
+
 INSTANTIATE_TEST_CASE_P(
     WritePreparedTransactionTest, WritePreparedTransactionTest,
     ::testing::Values(std::make_tuple(false, false, WRITE_PREPARED),
                       std::make_tuple(false, true, WRITE_PREPARED)));
 
+INSTANTIATE_TEST_CASE_P(
+    TwoWriteQueues, SnapshotConcurrentAccessTest,
+    ::testing::Values(std::make_tuple(false, true, WRITE_PREPARED, 0, 20),
+                      std::make_tuple(false, true, WRITE_PREPARED, 1, 20),
+                      std::make_tuple(false, true, WRITE_PREPARED, 2, 20),
+                      std::make_tuple(false, true, WRITE_PREPARED, 3, 20),
+                      std::make_tuple(false, true, WRITE_PREPARED, 4, 20),
+                      std::make_tuple(false, true, WRITE_PREPARED, 5, 20),
+                      std::make_tuple(false, true, WRITE_PREPARED, 6, 20),
+                      std::make_tuple(false, true, WRITE_PREPARED, 7, 20),
+                      std::make_tuple(false, true, WRITE_PREPARED, 8, 20),
+                      std::make_tuple(false, true, WRITE_PREPARED, 9, 20),
+                      std::make_tuple(false, true, WRITE_PREPARED, 10, 20),
+                      std::make_tuple(false, true, WRITE_PREPARED, 11, 20),
+                      std::make_tuple(false, true, WRITE_PREPARED, 12, 20),
+                      std::make_tuple(false, true, WRITE_PREPARED, 13, 20),
+                      std::make_tuple(false, true, WRITE_PREPARED, 14, 20),
+                      std::make_tuple(false, true, WRITE_PREPARED, 15, 20),
+                      std::make_tuple(false, true, WRITE_PREPARED, 16, 20),
+                      std::make_tuple(false, true, WRITE_PREPARED, 17, 20),
+                      std::make_tuple(false, true, WRITE_PREPARED, 18, 20),
+                      std::make_tuple(false, true, WRITE_PREPARED, 19, 20)));
+
+INSTANTIATE_TEST_CASE_P(
+    OneWriteQueue, SnapshotConcurrentAccessTest,
+    ::testing::Values(std::make_tuple(false, false, WRITE_PREPARED, 0, 20),
+                      std::make_tuple(false, false, WRITE_PREPARED, 1, 20),
+                      std::make_tuple(false, false, WRITE_PREPARED, 2, 20),
+                      std::make_tuple(false, false, WRITE_PREPARED, 3, 20),
+                      std::make_tuple(false, false, WRITE_PREPARED, 4, 20),
+                      std::make_tuple(false, false, WRITE_PREPARED, 5, 20),
+                      std::make_tuple(false, false, WRITE_PREPARED, 6, 20),
+                      std::make_tuple(false, false, WRITE_PREPARED, 7, 20),
+                      std::make_tuple(false, false, WRITE_PREPARED, 8, 20),
+                      std::make_tuple(false, false, WRITE_PREPARED, 9, 20),
+                      std::make_tuple(false, false, WRITE_PREPARED, 10, 20),
+                      std::make_tuple(false, false, WRITE_PREPARED, 11, 20),
+                      std::make_tuple(false, false, WRITE_PREPARED, 12, 20),
+                      std::make_tuple(false, false, WRITE_PREPARED, 13, 20),
+                      std::make_tuple(false, false, WRITE_PREPARED, 14, 20),
+                      std::make_tuple(false, false, WRITE_PREPARED, 15, 20),
+                      std::make_tuple(false, false, WRITE_PREPARED, 16, 20),
+                      std::make_tuple(false, false, WRITE_PREPARED, 17, 20),
+                      std::make_tuple(false, false, WRITE_PREPARED, 18, 20),
+                      std::make_tuple(false, false, WRITE_PREPARED, 19, 20)));
+
 TEST_P(WritePreparedTransactionTest, CommitMapTest) {
   WritePreparedTxnDB* wp_db = dynamic_cast<WritePreparedTxnDB*>(db);
   assert(wp_db);
@@ -553,26 +634,24 @@ bool IsInCombination(size_t i, size_t comb) { return comb & (size_t(1) << i); }
 #ifndef TRAVIS
 // Test that CheckAgainstSnapshots will not miss a live snapshot if it is run in
 // parallel with UpdateSnapshots.
-TEST_P(WritePreparedTransactionTest, SnapshotConcurrentAccessTest) {
+TEST_P(SnapshotConcurrentAccessTest, SnapshotConcurrentAccessTest) {
   // We have a sync point in the method under test after checking each snapshot.
   // If you increase the max number of snapshots in this test, more sync points
   // in the methods must also be added.
-  const std::vector<SequenceNumber> snapshots = {10l, 20l, 30l, 40l, 50l, 60l};
+  const std::vector<SequenceNumber> snapshots = {10l, 20l, 30l, 40l, 50l,
-  // TODO(myabandeh): increase the snapshots list for pre-release tests
+                                                 60l, 70l, 80l, 90l, 100l};
-  // const std::vector<SequenceNumber> snapshots = {10l, 20l, 30l, 40l, 50l,
-  //                                               60l, 70l, 80l, 90l, 100l};
   const size_t snapshot_cache_bits = 2;
   // Safety check to express the intended size in the test. Can be adjusted if
   // the snapshots lists changed.
-  assert((1ul << snapshot_cache_bits) + 2 == snapshots.size());
+  assert((1ul << snapshot_cache_bits) * 2 + 2 == snapshots.size());
   SequenceNumber version = 1000l;
   // Choose the cache size so that the new snapshot list could replace all the
   // existing items in the cache and also have some overflow.
   DBImpl* mock_db = new DBImpl(options, dbname);
   std::unique_ptr<WritePreparedTxnDBMock> wp_db(
       new WritePreparedTxnDBMock(mock_db, txn_db_options, snapshot_cache_bits));
-  // TODO(myabandeh): increase this number for pre-release tests
+  const size_t extra = 2;
-  const size_t extra = 1;
+  size_t loop_id = 0;
   // Add up to extra items that do not fit into the cache
   for (size_t old_size = 1; old_size <= wp_db->SNAPSHOT_CACHE_SIZE + extra;
        old_size++) {
@@ -582,7 +661,8 @@ TEST_P(WritePreparedTransactionTest, SnapshotConcurrentAccessTest) {
     // Each member of old snapshot might or might not appear in the new list. We
     // create a common_snapshots for each combination.
     size_t new_comb_cnt = size_t(1) << old_size;
-    for (size_t new_comb = 0; new_comb < new_comb_cnt; new_comb++) {
+    for (size_t new_comb = 0; new_comb < new_comb_cnt; new_comb++, loop_id++) {
+      if (loop_id % split_cnt_ != split_id_) continue;
       printf(".");  // To signal progress
       fflush(stdout);
       std::vector<SequenceNumber> common_snapshots;
@@ -630,7 +710,7 @@ TEST_P(WritePreparedTransactionTest, SnapshotConcurrentAccessTest) {
   }
   printf("\n");
 }
-#endif
+#endif  // TRAVIS
 
 // This test clarifies the contract of AdvanceMaxEvictedSeq method
 TEST_P(WritePreparedTransactionTest, AdvanceMaxEvictedSeqBasicTest) {

utilities/transactions/write_prepared_txn_db.h

@@ -209,8 +209,8 @@ class WritePreparedTxnDB : public PessimisticTransactionDB {
   friend class WritePreparedTransactionTest_IsInSnapshotTest_Test;
   friend class WritePreparedTransactionTest_CheckAgainstSnapshotsTest_Test;
   friend class WritePreparedTransactionTest_CommitMapTest_Test;
-  friend class WritePreparedTransactionTest_SnapshotConcurrentAccessTest_Test;
+  friend class SnapshotConcurrentAccessTest_SnapshotConcurrentAccessTest_Test;
-  friend class WritePreparedTransactionTest;
+  friend class WritePreparedTransactionTestBase;
   friend class PreparedHeap_BasicsTest_Test;
   friend class WritePreparedTxnDBMock;
   friend class WritePreparedTransactionTest_AdvanceMaxEvictedSeqBasicTest_Test;