From 144f975c9e5ae97957e3c6959f03e7d768874c94 Mon Sep 17 00:00:00 2001 From: Arkadiusz Jedrzejewski Date: Fri, 30 Oct 2026 11:06:18 +0100 Subject: [PATCH] kvs: changed approach to locking - "Wait for unlock" approach instead of "fail on busy". - Updated safety docs. - Removed unused C++ `KMutexLockFailed` error. - Removed obsolete tests. --- .../persistency/safety_analysis/dfa.rst | 12 +- score/kvs/docs/safety_analysis/dfa.rst | 4 +- score/kvs/error.cpp | 2 - score/kvs/error.h | 5 - score/kvs/kvs.cpp | 109 ++------ score/kvs/kvs_deprecated_api_test.cpp | 2 - score/kvs/kvs_error_test.cpp | 1 - score/kvs/kvs_test.cpp | 260 ------------------ 8 files changed, 30 insertions(+), 365 deletions(-) diff --git a/docs/features/persistency/safety_analysis/dfa.rst b/docs/features/persistency/safety_analysis/dfa.rst index 4d272cce2..04993906c 100644 --- a/docs/features/persistency/safety_analysis/dfa.rst +++ b/docs/features/persistency/safety_analysis/dfa.rst @@ -138,7 +138,7 @@ Unintended impacts to function due to various failures. * - UI_01_04 - Deadlocks - no - - One mutex per instance (Rust additionally one for the instance pool in the builder); never more than one lock is held at a time. Rust waits for the lock; C++ does not wait and returns MutexLockFailed. + - One mutex per instance (Rust additionally one for the instance pool in the builder). Both implementations wait for the lock. A call holds at most one lock and does not call back into the application while holding it. Exception: the C++ move assignment takes the locks of both instances at once with ``std::scoped_lock``, which avoids deadlock. * - UI_01_05 - Livelocks - no @@ -185,10 +185,14 @@ DFA :mitigated_by: aou_req__persistency__error_handling :sufficient: yes :status: valid - :version: 2 + :version: 3 - Persistency runs in the calling context; blocking (by the application or OS scheduling) leads to no or a - too late response, handled by :need:`aou_req__persistency__error_handling`. + Persistency runs in the calling context. Blocking leads to no or a too late response, handled by + :need:`aou_req__persistency__error_handling`. + + Causes: the application or OS scheduling, and waiting for the instance lock while another thread of the + process uses the same instance. The wait lasts until that call returns, including its file access (e.g. + flush or snapshot restore). .. feat_saf_dfa:: Corruption of persisted data :violates: feat_arc_sta__persistency__static diff --git a/score/kvs/docs/safety_analysis/dfa.rst b/score/kvs/docs/safety_analysis/dfa.rst index e22300d2f..b1b1291bb 100644 --- a/score/kvs/docs/safety_analysis/dfa.rst +++ b/score/kvs/docs/safety_analysis/dfa.rst @@ -144,11 +144,11 @@ Unintended impacts to function due to various failures. * - UI_01_05 - Livelocks - no - - No retry loops; a failed lock is reported (C++ ``MutexLockFailed``). + - No retry loops. Calls wait for the instance lock and do not retry. * - UI_01_06 - Blocking of execution - no - - No component specific effect, see :need:`feat_saf_dfa__persistency__execution_blocking`. + - Calls wait for the instance lock held by another thread of the process. The effect is covered by :need:`feat_saf_dfa__persistency__execution_blocking`. * - UI_01_07 - Incorrect allocation of execution time - no diff --git a/score/kvs/error.cpp b/score/kvs/error.cpp index c7815faa9..9478d3e6c 100644 --- a/score/kvs/error.cpp +++ b/score/kvs/error.cpp @@ -61,8 +61,6 @@ std::string_view KvsErrorDomain::MessageFor(const score::result::ErrorCode& code return "Invalid snapshot ID"; case ErrorCode::kConversionFailed: return "Conversion failed"; - case ErrorCode::kMutexLockFailed: - return "Mutex failed"; case ErrorCode::kInvalidValueType: return "Invalid value type"; case ErrorCode::kInvalidKey: diff --git a/score/kvs/error.h b/score/kvs/error.h index bd4b486ff..f7af11cf8 100644 --- a/score/kvs/error.h +++ b/score/kvs/error.h @@ -77,9 +77,6 @@ enum class ErrorCode : score::result::ErrorCode /// Conversion failed. kConversionFailed = 18, - /// Mutex failed. - kMutexLockFailed = 19, - /// Invalid value type. kInvalidValueType = 20, @@ -124,8 +121,6 @@ enum class ErrorCode : score::result::ErrorCode kInvalidSnapshotId, ConversionFailed [[deprecated("Use kConversionFailed instead. This name is removed in a future release.")]] = kConversionFailed, - MutexLockFailed [[deprecated("Use kMutexLockFailed instead. This name is removed in a future release.")]] = - kMutexLockFailed, InvalidValueType [[deprecated("Use kInvalidValueType instead. This name is removed in a future release.")]] = kInvalidValueType, InvalidKey [[deprecated("Use kInvalidKey instead. This name is removed in a future release.")]] = kInvalidKey, diff --git a/score/kvs/kvs.cpp b/score/kvs/kvs.cpp index b27def17e..14647236d 100644 --- a/score/kvs/kvs.cpp +++ b/score/kvs/kvs.cpp @@ -329,12 +329,7 @@ score::Result Kvs::Open(const InstanceId& instance_id, score::Result Kvs::Reset() noexcept { - std::unique_lock lock{kvs_mutex_, std::try_to_lock}; - if (!lock.owns_lock()) - { - GetLogger().LogWarn() << "Failed to lock KVS mutex"; - return score::MakeUnexpected(ErrorCode::kMutexLockFailed); - } + std::lock_guard lock{kvs_mutex_}; kvs_.clear(); return {}; @@ -342,12 +337,7 @@ score::Result Kvs::Reset() noexcept score::Result> Kvs::GetAllKeys() const noexcept { - std::unique_lock lock{kvs_mutex_, std::try_to_lock}; - if (!lock.owns_lock()) - { - GetLogger().LogWarn() << "Failed to lock KVS mutex"; - return score::MakeUnexpected(ErrorCode::kMutexLockFailed); - } + std::lock_guard lock{kvs_mutex_}; std::vector keys; keys.reserve(kvs_.size()); @@ -360,12 +350,7 @@ score::Result> Kvs::GetAllKeys() const noexcept score::Result Kvs::KeyExists(const std::string_view key) const noexcept { - std::unique_lock lock{kvs_mutex_, std::try_to_lock}; - if (!lock.owns_lock()) - { - GetLogger().LogWarn() << "Failed to lock KVS mutex"; - return score::MakeUnexpected(ErrorCode::kMutexLockFailed); - } + std::lock_guard lock{kvs_mutex_}; // unordered_map find() needs string and doesn't work with string_view. // C++20 heterogeneous lookup would avoid the copy (applies to more functions). @@ -375,12 +360,7 @@ score::Result Kvs::KeyExists(const std::string_view key) const noexcept score::Result Kvs::GetValue(const std::string_view key) const noexcept { - std::unique_lock lock_kvs{kvs_mutex_, std::try_to_lock}; - if (!lock_kvs.owns_lock()) - { - GetLogger().LogWarn() << "Failed to lock KVS mutex"; - return score::MakeUnexpected(ErrorCode::kMutexLockFailed); - } + std::lock_guard lock{kvs_mutex_}; const auto search_kvs{kvs_.find(std::string(key))}; if (search_kvs == kvs_.end()) @@ -419,12 +399,7 @@ score::Result Kvs::SetValueToDefault(const std::string_view key) noexcept } // Set value. - std::unique_lock lock{kvs_mutex_, std::try_to_lock}; - if (!lock.owns_lock()) - { - GetLogger().LogWarn() << "Failed to lock KVS mutex"; - return score::MakeUnexpected(ErrorCode::kMutexLockFailed); - } + std::lock_guard lock{kvs_mutex_}; score::cpp::ignore = kvs_.insert_or_assign(std::string(key), std::move(*default_value_result)); return {}; @@ -432,12 +407,7 @@ score::Result Kvs::SetValueToDefault(const std::string_view key) noexcept score::Result Kvs::IsValueDefault(const std::string_view key) const noexcept { - std::unique_lock lock(kvs_mutex_, std::try_to_lock); - if (!lock.owns_lock()) - { - GetLogger().LogWarn() << "Failed to lock KVS mutex"; - return score::MakeUnexpected(ErrorCode::kMutexLockFailed); - } + std::lock_guard lock(kvs_mutex_); const std::string key_str{key}; if (kvs_.find(key_str) != kvs_.end()) @@ -473,12 +443,7 @@ score::Result Kvs::SetValue(const std::string_view key, const KvsValue& va return score::MakeUnexpected(ErrorCode::kInvalidValueType); } - std::unique_lock lock{kvs_mutex_, std::try_to_lock}; - if (!lock.owns_lock()) - { - GetLogger().LogWarn() << "Failed to lock KVS mutex"; - return score::MakeUnexpected(ErrorCode::kMutexLockFailed); - } + std::lock_guard lock{kvs_mutex_}; score::cpp::ignore = kvs_.insert_or_assign(std::string(key), value); return {}; @@ -486,12 +451,7 @@ score::Result Kvs::SetValue(const std::string_view key, const KvsValue& va score::Result Kvs::RemoveKey(const std::string_view key) noexcept { - std::unique_lock lock{kvs_mutex_, std::try_to_lock}; - if (!lock.owns_lock()) - { - GetLogger().LogWarn() << "Failed to lock KVS mutex"; - return score::MakeUnexpected(ErrorCode::kMutexLockFailed); - } + std::lock_guard lock{kvs_mutex_}; if (kvs_.erase(std::string(key)) == 0U) { @@ -503,12 +463,7 @@ score::Result Kvs::RemoveKey(const std::string_view key) noexcept score::Result Kvs::RemoveAllKeys() noexcept { - std::unique_lock lock{kvs_mutex_, std::try_to_lock}; - if (!lock.owns_lock()) - { - GetLogger().LogWarn() << "Failed to lock KVS mutex"; - return score::MakeUnexpected(ErrorCode::kMutexLockFailed); - } + std::lock_guard lock{kvs_mutex_}; kvs_.clear(); return {}; @@ -516,12 +471,7 @@ score::Result Kvs::RemoveAllKeys() noexcept score::Result Kvs::DiscardPendingChanges() noexcept { - std::unique_lock lock{kvs_mutex_, std::try_to_lock}; - if (!lock.owns_lock()) - { - GetLogger().LogWarn() << "Failed to lock KVS mutex"; - return score::MakeUnexpected(ErrorCode::kMutexLockFailed); - } + std::lock_guard lock{kvs_mutex_}; // Snapshot 0 is the current persisted state: written by Flush(), read by Open(). const score::filesystem::Path kvs_path{filename_prefix_.Native() + "_0"}; @@ -539,6 +489,8 @@ score::Result Kvs::DiscardPendingChanges() noexcept score::Result Kvs::Flush() noexcept { + std::lock_guard lock{kvs_mutex_}; + auto result{SerializeAndCheck()}; if (!result) { @@ -585,12 +537,7 @@ size_t Kvs::SnapshotMaxCount() const noexcept score::Result Kvs::SnapshotRestore(const SnapshotId& snapshot_id) noexcept { - std::unique_lock lock{kvs_mutex_, std::try_to_lock}; - if (!lock.owns_lock()) - { - GetLogger().LogWarn() << "Failed to lock KVS mutex"; - return score::MakeUnexpected(ErrorCode::kMutexLockFailed); - } + std::lock_guard lock{kvs_mutex_}; const auto snapshot_count_result{SnapshotCount()}; if (!snapshot_count_result) @@ -687,6 +634,8 @@ score::Result Kvs::GetStorageFileSize() const noexcept score::Result Kvs::CalculatePotentialSize() const noexcept { + std::lock_guard lock{kvs_mutex_}; + auto result{SerializeAndCheck()}; if (!result) { @@ -715,13 +664,6 @@ Kvs::Kvs() noexcept score::Result Kvs::SnapshotRotate() const noexcept { - std::unique_lock lock{kvs_mutex_, std::try_to_lock}; - if (!lock.owns_lock()) - { - GetLogger().LogWarn() << "Failed to lock KVS mutex"; - return score::MakeUnexpected(ErrorCode::kMutexLockFailed); - } - for (size_t index{max_snapshots_}; index > 0; --index) { const score::filesystem::Path hash_old{filename_prefix_.Native() + "_" + std::to_string(index - 1) + @@ -913,25 +855,14 @@ score::Result Kvs::WriteJsonData(const std::string& buffer) const noexcept score::Result Kvs::SerializeAndCheck() const noexcept { - score::json::Any root_obj; - // 1. Serialize the current KVS map. + auto converted{KvsValueToAny(kvs_)}; + if (!converted) { - std::unique_lock lock{kvs_mutex_, std::try_to_lock}; - if (!lock.owns_lock()) - { - GetLogger().LogWarn() << "Failed to lock KVS mutex"; - return score::MakeUnexpected(ErrorCode::kMutexLockFailed); - } - - auto converted{KvsValueToAny(kvs_)}; - if (!converted) - { - GetLogger().LogError() << "Failed to serialize KVS data"; - return score::MakeUnexpected(converted.error()); - } - root_obj = std::move(*converted); + GetLogger().LogError() << "Failed to serialize KVS data"; + return score::MakeUnexpected(converted.error()); } + const score::json::Any root_obj{std::move(*converted)}; auto buffer_result{writer_->ToBuffer(root_obj)}; if (!buffer_result) diff --git a/score/kvs/kvs_deprecated_api_test.cpp b/score/kvs/kvs_deprecated_api_test.cpp index 45b357d45..a877871ba 100644 --- a/score/kvs/kvs_deprecated_api_test.cpp +++ b/score/kvs/kvs_deprecated_api_test.cpp @@ -49,7 +49,6 @@ static_assert(static_cast(ErrorCode::kKeyNotFound) == static_assert(static_cast(ErrorCode::kSerializationFailed) == 16, ""); static_assert(static_cast(ErrorCode::kInvalidSnapshotId) == 17, ""); static_assert(static_cast(ErrorCode::kConversionFailed) == 18, ""); -static_assert(static_cast(ErrorCode::kMutexLockFailed) == 19, ""); static_assert(static_cast(ErrorCode::kInvalidValueType) == 20, ""); static_assert(static_cast(ErrorCode::kInvalidKey) == 21, ""); @@ -99,7 +98,6 @@ static_assert(ErrorCode::KeyNotFound == ErrorCode::kKeyNotFound, ""); static_assert(ErrorCode::SerializationFailed == ErrorCode::kSerializationFailed, ""); static_assert(ErrorCode::InvalidSnapshotId == ErrorCode::kInvalidSnapshotId, ""); static_assert(ErrorCode::ConversionFailed == ErrorCode::kConversionFailed, ""); -static_assert(ErrorCode::MutexLockFailed == ErrorCode::kMutexLockFailed, ""); static_assert(ErrorCode::InvalidValueType == ErrorCode::kInvalidValueType, ""); static_assert(ErrorCode::InvalidKey == ErrorCode::kInvalidKey, ""); DISABLE_WARNING_POP diff --git a/score/kvs/kvs_error_test.cpp b/score/kvs/kvs_error_test.cpp index e9c5f6cd5..52422d67e 100644 --- a/score/kvs/kvs_error_test.cpp +++ b/score/kvs/kvs_error_test.cpp @@ -43,7 +43,6 @@ TEST(kvs_MessageFor, MessageFor) {ErrorCode::kSerializationFailed, "Serialization failed"}, {ErrorCode::kInvalidSnapshotId, "Invalid snapshot ID"}, {ErrorCode::kConversionFailed, "Conversion failed"}, - {ErrorCode::kMutexLockFailed, "Mutex failed"}, {ErrorCode::kInvalidValueType, "Invalid value type"}, {ErrorCode::kInvalidKey, "Key is not valid"}, }; diff --git a/score/kvs/kvs_test.cpp b/score/kvs/kvs_test.cpp index 6f3dee1b6..9dc47621a 100644 --- a/score/kvs/kvs_test.cpp +++ b/score/kvs/kvs_test.cpp @@ -687,28 +687,6 @@ TEST(kvs_reset, reset_success) CleanupEnvironment(); } -TEST(kvs_reset, reset_failure) -{ - PrepareEnvironment(); - - // Mutex locked. - auto result{Kvs::Open(kInstanceId, - kSnapshotId, - KvsDefaults::kRequired, - KvsLoad::kRequired, - std::string(kDataDir), - std::nullopt, - kDefaultMaxSnapshots)}; - ASSERT_TRUE(result); - - std::unique_lock lock{result.value().kvs_mutex_}; - auto reset_result{result.value().Reset()}; - EXPECT_FALSE(reset_result); - EXPECT_EQ(static_cast(*reset_result.error()), ErrorCode::kMutexLockFailed); - - CleanupEnvironment(); -} - TEST(kvs_get_all_keys, get_all_keys_success) { PrepareEnvironment(); @@ -738,28 +716,6 @@ TEST(kvs_get_all_keys, get_all_keys_success) CleanupEnvironment(); } -TEST(kvs_get_all_keys, get_all_keys_failure) -{ - PrepareEnvironment(); - - // Mutex locked. - auto result{Kvs::Open(kInstanceId, - kSnapshotId, - KvsDefaults::kRequired, - KvsLoad::kRequired, - std::string(kDataDir), - std::nullopt, - kDefaultMaxSnapshots)}; - ASSERT_TRUE(result); - std::unique_lock lock{result.value().kvs_mutex_}; - - auto get_all_keys_result{result.value().GetAllKeys()}; - EXPECT_FALSE(get_all_keys_result); - EXPECT_EQ(static_cast(*get_all_keys_result.error()), ErrorCode::kMutexLockFailed); - - CleanupEnvironment(); -} - TEST(kvs_key_exists, key_exists_success) { PrepareEnvironment(); @@ -785,28 +741,6 @@ TEST(kvs_key_exists, key_exists_success) CleanupEnvironment(); } -TEST(kvs_key_exists, key_exists_failure) -{ - PrepareEnvironment(); - - // Mutex locked. - auto result{Kvs::Open(kInstanceId, - kSnapshotId, - KvsDefaults::kRequired, - KvsLoad::kRequired, - std::string(kDataDir), - std::nullopt, - kDefaultMaxSnapshots)}; - ASSERT_TRUE(result); - - std::unique_lock lock{result.value().kvs_mutex_}; - auto exists_result{result.value().KeyExists("kvs")}; - EXPECT_FALSE(exists_result); - EXPECT_EQ(static_cast(*exists_result.error()), ErrorCode::kMutexLockFailed); - - CleanupEnvironment(); -} - TEST(kvs_get_value, get_value_success) { PrepareEnvironment(); @@ -857,20 +791,6 @@ TEST(kvs_get_value, get_value_failure) EXPECT_FALSE(get_value_result); EXPECT_EQ(get_value_result.error(), ErrorCode::kKeyNotFound); - // Mutex locked. - result = Kvs::Open(kInstanceId, - kSnapshotId, - KvsDefaults::kRequired, - KvsLoad::kRequired, - std::string(kDataDir), - std::nullopt, - kDefaultMaxSnapshots); - ASSERT_TRUE(result); - std::unique_lock lock{result.value().kvs_mutex_}; - get_value_result = result.value().GetValue("kvs"); - EXPECT_FALSE(get_value_result); - EXPECT_EQ(static_cast(*get_value_result.error()), ErrorCode::kMutexLockFailed); - CleanupEnvironment(); } @@ -1003,20 +923,6 @@ TEST(kvs_set_value_to_default, set_value_to_default_failure) EXPECT_FALSE(set_value_to_default_result); EXPECT_EQ(set_value_to_default_result.error(), ErrorCode::kKeyNotFound); - // Mutex locked. - result = Kvs::Open(kInstanceId, - kSnapshotId, - KvsDefaults::kRequired, - KvsLoad::kRequired, - std::string(kDataDir), - std::nullopt, - kDefaultMaxSnapshots); - ASSERT_TRUE(result); - std::unique_lock lock{result.value().kvs_mutex_}; - set_value_to_default_result = result.value().SetValueToDefault("default"); - EXPECT_FALSE(set_value_to_default_result); - EXPECT_EQ(static_cast(*set_value_to_default_result.error()), ErrorCode::kMutexLockFailed); - CleanupEnvironment(); } @@ -1101,28 +1007,6 @@ TEST(kvs_is_value_default, is_value_default) CleanupEnvironment(); } -TEST(kvs_is_value_default, is_value_default_failure) -{ - PrepareEnvironment(); - - // Mutex locked. - auto result{Kvs::Open(kInstanceId, - kSnapshotId, - KvsDefaults::kRequired, - KvsLoad::kRequired, - std::string(kDataDir), - std::nullopt, - kDefaultMaxSnapshots)}; - ASSERT_TRUE(result); - - std::unique_lock lock{result.value().kvs_mutex_}; - auto default_result{result.value().IsValueDefault("default")}; - EXPECT_FALSE(default_result); - EXPECT_EQ(static_cast(*default_result.error()), ErrorCode::kMutexLockFailed); - - CleanupEnvironment(); -} - TEST(kvs_set_value, set_value_success) { PrepareEnvironment(); @@ -1349,28 +1233,6 @@ TEST(kvs_get_value, get_value_failure_invalid_characters) CleanupEnvironment(); } -TEST(kvs_set_value, set_value_failure) -{ - PrepareEnvironment(); - - // Mutex locked. - auto result{Kvs::Open(kInstanceId, - kSnapshotId, - KvsDefaults::kRequired, - KvsLoad::kRequired, - std::string(kDataDir), - std::nullopt, - kDefaultMaxSnapshots)}; - ASSERT_TRUE(result); - - std::unique_lock lock{result.value().kvs_mutex_}; - auto set_value_result{result.value().SetValue("new_key", KvsValue(3.0))}; - EXPECT_FALSE(set_value_result); - EXPECT_EQ(static_cast(*set_value_result.error()), ErrorCode::kMutexLockFailed); - - CleanupEnvironment(); -} - TEST(kvs_set_value, set_value_duplicate_object_key) { PrepareEnvironment(); @@ -1558,20 +1420,6 @@ TEST(kvs_remove_key, remove_key_failure) EXPECT_FALSE(remove_key_result); EXPECT_EQ(remove_key_result.error(), ErrorCode::kKeyNotFound); - // Mutex locked. - result = Kvs::Open(kInstanceId, - kSnapshotId, - KvsDefaults::kRequired, - KvsLoad::kRequired, - std::string(kDataDir), - std::nullopt, - kDefaultMaxSnapshots); - ASSERT_TRUE(result); - std::unique_lock lock{result.value().kvs_mutex_}; - remove_key_result = result.value().RemoveKey("kvs"); - EXPECT_FALSE(remove_key_result); - EXPECT_EQ(static_cast(*remove_key_result.error()), ErrorCode::kMutexLockFailed); - CleanupEnvironment(); } @@ -1963,28 +1811,6 @@ TEST(kvs_snapshot_rotate, snapshot_rotate_failure_renaming_hash) CleanupEnvironment(); } -TEST(kvs_snapshot_rotate, snapshot_rotate_failure_mutex) -{ - PrepareEnvironment(); - - auto result{Kvs::Open(kInstanceId, - kSnapshotId, - KvsDefaults::kOptional, - KvsLoad::kOptional, - std::string(kDataDir), - std::nullopt, - kDefaultMaxSnapshots)}; - ASSERT_TRUE(result); - - // Mutex locked. - std::unique_lock lock{result.value().kvs_mutex_}; - auto rotate_result{result.value().SnapshotRotate()}; - EXPECT_FALSE(rotate_result); - EXPECT_EQ(static_cast(*rotate_result.error()), ErrorCode::kMutexLockFailed); - - CleanupEnvironment(); -} - TEST(kvs_flush, flush_success_data) { PrepareEnvironment(); @@ -2090,27 +1916,6 @@ TEST(kvs_flush, flush_sets_file_permissions) CleanupEnvironment(); } -TEST(kvs_flush, flush_failure_mutex) -{ - PrepareEnvironment(); - - auto result{Kvs::Open(kInstanceId, - kSnapshotId, - KvsDefaults::kOptional, - KvsLoad::kOptional, - std::string(kDataDir), - std::nullopt, - kDefaultMaxSnapshots)}; - ASSERT_TRUE(result); - - std::unique_lock lock{result.value().kvs_mutex_}; - auto flush_result{result.value().Flush()}; - EXPECT_FALSE(flush_result); - EXPECT_EQ(static_cast(*flush_result.error()), ErrorCode::kMutexLockFailed); - - CleanupEnvironment(); -} - TEST(kvs_flush, flush_failure_rotate_snapshots) { // Root ignores file permissions, so flush would succeed. @@ -2340,27 +2145,6 @@ TEST(kvs_snapshot_restore, snapshot_restore_failure_open_json) CleanupEnvironment(); } -TEST(kvs_snapshot_restore, snapshot_restore_failure_mutex) -{ - PrepareEnvironment(); - - auto result{Kvs::Open(kInstanceId, - kSnapshotId, - KvsDefaults::kOptional, - KvsLoad::kOptional, - std::string(kDataDir), - std::nullopt, - kDefaultMaxSnapshots)}; - ASSERT_TRUE(result); - - std::unique_lock lock{result.value().kvs_mutex_}; - auto restore_result{result.value().SnapshotRestore(1)}; - EXPECT_FALSE(restore_result); - EXPECT_EQ(static_cast(*restore_result.error()), ErrorCode::kMutexLockFailed); - - CleanupEnvironment(); -} - TEST(kvs_snapshot_restore, snapshot_restore_failure_snapshot_count) { PrepareEnvironment(); @@ -2631,28 +2415,6 @@ TEST(kvs_remove_all_keys, remove_all_keys_keeps_defaults) CleanupEnvironment(); } -TEST(kvs_remove_all_keys, remove_all_keys_failure) -{ - PrepareEnvironment(); - - // Mutex locked. - auto kvs{Kvs::Open(kInstanceId, - kSnapshotId, - KvsDefaults::kRequired, - KvsLoad::kRequired, - std::string(kDataDir), - std::nullopt, - kDefaultMaxSnapshots)}; - ASSERT_TRUE(kvs); - - std::unique_lock lock{kvs.value().kvs_mutex_}; - auto result{kvs.value().RemoveAllKeys()}; - EXPECT_FALSE(result); - EXPECT_EQ(static_cast(*result.error()), ErrorCode::kMutexLockFailed); - - CleanupEnvironment(); -} - TEST(kvs_discard_pending_changes, discard_reverts_to_state_at_open) { PrepareEnvironment(); @@ -2754,28 +2516,6 @@ TEST(kvs_discard_pending_changes, discard_without_changes_is_noop) CleanupEnvironment(); } -TEST(kvs_discard_pending_changes, discard_pending_changes_failure) -{ - PrepareEnvironment(); - - // Mutex locked. - auto kvs{Kvs::Open(kInstanceId, - kSnapshotId, - KvsDefaults::kRequired, - KvsLoad::kRequired, - std::string(kDataDir), - std::nullopt, - kDefaultMaxSnapshots)}; - ASSERT_TRUE(kvs); - - std::unique_lock lock{kvs.value().kvs_mutex_}; - auto result{kvs.value().DiscardPendingChanges()}; - EXPECT_FALSE(result); - EXPECT_EQ(static_cast(*result.error()), ErrorCode::kMutexLockFailed); - - CleanupEnvironment(); -} - TEST(kvs_discard_pending_changes, discard_without_persisted_file_yields_empty) { PrepareEnvironment();