Refactor, still poc

Author: ognyanstoimenov

cpp/arcticdb/util/test/test_storage_lock.cpp

@@ -365,69 +365,76 @@ TEST(StorageLock, OptimisticForceReleaseLock) {
     first_lock._test_release_local_lock();
 }
 
-TEST(StorageLock, ConcurrentWrites) {
-    log::lock().set_level(spdlog::level::debug);
-    const StorageFailureSimulator::ParamActionSequence SLOW_ON_2ND_CALL = {
-        action_factories::slow_action(1, 10, 10), action_factories::slow_action(1, 1700, 1700)
+
+class StorageLockWithSlowWrites : public ::testing::TestWithParam<std::tuple<int, int>> {
+protected:
+    void SetUp() override {
+        log::lock().set_level(spdlog::level::debug);
+        StorageFailureSimulator::instance()->reset();
+    }
+};
+
+TEST_P(StorageLockWithSlowWrites, ConcurrentWrites) {
+    const int first_delay = std::get<0>(GetParam());
+    const int second_delay = std::get<1>(GetParam());
+
+    const StorageFailureSimulator::ParamActionSequence SLOW_ACTIONS = {
+        action_factories::slow_action(1, first_delay, first_delay),
+        action_factories::slow_action(1, second_delay, second_delay)
     };
+
     constexpr size_t num_writers = 2;
+    FutureExecutor<CPUThreadPoolExecutor> exec{num_writers};
+
+    StorageFailureSimulator::instance()->configure({{FailureType::WRITE_SLOW, SLOW_ACTIONS}});
 
-    using StorageLockType = StorageLock<>;
-    std::vector<std::unique_ptr<StorageLockType>> locks;
-    locks.reserve(num_writers);
-    while (locks.size() < num_writers) { locks.emplace_back(std::make_unique<StorageLockType>("test")); }
-    auto store = std::make_shared<InMemoryStore>();
-    folly::FutureExecutor<folly::CPUThreadPoolExecutor> exec{num_writers};
-    StorageFailureSimulator::instance()->configure({{FailureType::WRITE_SLOW, SLOW_ON_2ND_CALL}});
     std::vector<Future<Unit>> futures;
-    std::vector<bool> writer_has_lock;
-    writer_has_lock.resize(num_writers);
-    for(size_t i = 0; i < num_writers; ++i) {
-        futures.emplace_back(exec.addFuture([&store, &lock = locks[i], i, &writer_has_lock] {
-            writer_has_lock[i] = lock->try_lock(store);
-        }));
+    auto lock_data = std::make_shared<LockData>(num_writers);
+    lock_data->store_ = std::make_shared<InMemoryStore>();
+
+    for (size_t i = 0; i < num_writers; ++i) {
+        futures.emplace_back(exec.addFuture(OptimisticLockTask(lock_data)));
     }
     collect(futures).get();
 
-    const long have_lock = std::ranges::count(writer_has_lock, true);
-    ASSERT_EQ(have_lock, 0); // Both should fail
+    const int expected_locks = second_delay < 1000 ? 0 : 1;
+    ASSERT_EQ(lock_data->atomic_, expected_locks); // Both should fail
 }
 
-TEST(StorageLock, ConcurrentWrites2) {
-    log::lock().set_level(spdlog::level::debug);
-    log::lock().set_level(spdlog::level::debug);
-    const StorageFailureSimulator::ParamActionSequence SLOW_ON_2ND_CALL = {
-        action_factories::slow_action(1, 10, 10), action_factories::slow_action(1, 500, 600)
+TEST_P(StorageLockWithSlowWrites, ConcurrentWritesWithRetrying) {
+    const int first_delay = std::get<0>(GetParam());
+    const int second_delay = std::get<1>(GetParam());
+
+    const StorageFailureSimulator::ParamActionSequence SLOW_ACTIONS = {
+        action_factories::slow_action(1, first_delay, first_delay),
+        action_factories::slow_action(1, second_delay, second_delay)
     };
     constexpr size_t num_writers = 2;
 
-    using StorageLockType = StorageLock<>;
-    std::vector<std::unique_ptr<StorageLockType>> locks;
-    locks.reserve(num_writers);
-    while (locks.size() < num_writers) { locks.emplace_back(std::make_unique<StorageLockType>("test")); }
-    auto store = std::make_shared<InMemoryStore>();
-    folly::FutureExecutor<folly::CPUThreadPoolExecutor> exec{num_writers};
-    StorageFailureSimulator::instance()->configure({{FailureType::WRITE_SLOW, SLOW_ON_2ND_CALL}});
+    auto lock_data = std::make_shared<LockData>(num_writers);
+    lock_data->store_ = std::make_shared<InMemoryStore>();
+    FutureExecutor<CPUThreadPoolExecutor> exec{num_writers};
+    StorageFailureSimulator::instance()->configure({{FailureType::WRITE_SLOW, SLOW_ACTIONS}});
     std::vector<Future<Unit>> futures;
     std::vector<bool> writer_has_lock;
     writer_has_lock.resize(num_writers);
     for(size_t i = 0; i < num_writers; ++i) {
-        futures.emplace_back(exec.addFuture([&store, &lock = locks[i], i, &writer_has_lock] {
-            try {
-                lock->lock_timeout(store, 2000);
-                writer_has_lock[i] = true;
-            }
-            catch(const StorageLockTimeout&) {
-                writer_has_lock[i] = false;
-            }
-        }));
+        futures.emplace_back(exec.addFuture(PessimisticLockTask(lock_data, 3000)));
     }
     collect(futures).get();
 
-    const long have_lock = std::ranges::count(writer_has_lock, true);
-    ASSERT_EQ(have_lock, 1);
+    ASSERT_EQ(lock_data->atomic_, 1);
 }
 
+INSTANTIATE_TEST_SUITE_P(
+    DelayPairs,
+    StorageLockWithSlowWrites,
+    ::testing::Values(
+        std::make_tuple(10, 800),
+        std::make_tuple(10, 1700),
+        std::make_tuple(10, 2000)
+    )
+);
 
 TEST(StorageLock, SlowWrites) {
     const auto current_lock_sleep_wait_ms = ConfigsMap::instance()->get_int("StorageLock.WaitMs", StorageLock<>::DEFAULT_WAIT_MS);