LMDB exception normalization with mock client (#1414)

This is the final PR for exception normalization. Previous PRs are #1411, #1360, #1344, #1304, #1297 and #1285 #### Reference Issues/PRs Previously #1285 only normalized `KeyNotFoundException` and `DuplicateKeyException` correctly. As mentioned in [this comment](#1285 (comment)), we need to normalize other lmdb specific errors too. This PR does that. A mock client is also created to simulate the lmdb errors which aren't easily produce-able with real lmdb but can occur. The ErrorCode list has been updated in `error_code.hpp`. Previously, all the storage error codes were just sequential. This required that each time a new error was added for a specific storage, all the other error codes needed to be changed as we want to assign sequential error codes to the same storage. We now leave 10 error codes for each storage which allows us to easily add new error codes for a storage without having to change all the others. `::lmdb::map_full_error` is now normalized. When this error occurs, lmdb throws `LMDBMapFullException` which is child of `StorageException` #### What does this implement or fix? #### Any other comments? #### Checklist <details> <summary> Checklist for code changes... </summary> - [ ] Have you updated the relevant docstrings, documentation and copyright notice? - [ ] Is this contribution tested against [all ArcticDB's features](../docs/mkdocs/docs/technical/contributing.md)? - [ ] Do all exceptions introduced raise appropriate [error messages](https://docs.arcticdb.io/error_messages/)? - [ ] Are API changes highlighted in the PR description? - [ ] Is the PR labelled as enhancement or bug so it appears in autogenerated release notes? </details>
man-group · Mar 14, 2024 · 882e73e · 882e73e
1 parent 01e890c
commit 882e73e
Show file tree

Hide file tree

Showing 14 changed files with 625 additions and 118 deletions.
diff --git a/cpp/arcticdb/CMakeLists.txt b/cpp/arcticdb/CMakeLists.txt
@@ -272,12 +272,21 @@ set(arcticdb_srcs
         storage/library.hpp
         storage/library_index.hpp
         storage/library_manager.hpp
+        storage/storage_mock_client.hpp
+        storage/azure/azure_client_wrapper.hpp
+        storage/azure/azure_mock_client.hpp
+        storage/azure/azure_real_client.hpp
         storage/azure/azure_storage.hpp
+        storage/lmdb/lmdb_client_wrapper.hpp
+        storage/lmdb/lmdb_mock_client.hpp
+        storage/lmdb/lmdb_real_client.hpp
         storage/lmdb/lmdb_storage.hpp
         storage/memory/memory_storage.hpp
         storage/memory/memory_storage.cpp
         storage/mongo/mongo_client.hpp
         storage/mongo/mongo_instance.hpp
+        storage/mongo/mongo_client_wrapper.hpp
+        storage/mongo/mongo_mock_client.hpp
         storage/mongo/mongo_storage.hpp
         storage/object_store_utils.hpp
         storage/file/file_store.hpp
@@ -433,10 +442,15 @@ set(arcticdb_srcs
         storage/failure_simulation.cpp
         storage/library_manager.cpp
         storage/azure/azure_storage.cpp
+        storage/azure/azure_real_client.cpp
+        storage/azure/azure_mock_client.cpp
+        storage/lmdb/lmdb_mock_client.cpp
+        storage/lmdb/lmdb_real_client.cpp
         storage/lmdb/lmdb_storage.cpp
         storage/file/mapped_file_storage.cpp
         storage/mongo/mongo_client.cpp
         storage/mongo/mongo_instance.cpp
+        storage/mongo/mongo_mock_client.cpp
         storage/mongo/mongo_storage.cpp
         storage/s3/nfs_backed_storage.cpp
         storage/s3/s3_api.cpp
@@ -445,15 +459,6 @@ set(arcticdb_srcs
         storage/s3/s3_storage.cpp
         storage/s3/s3_storage_tool.cpp
         storage/storage_factory.cpp
-        storage/azure/azure_client_wrapper.hpp
-        storage/azure/azure_real_client.hpp
-        storage/azure/azure_real_client.cpp
-        storage/azure/azure_mock_client.hpp
-        storage/azure/azure_mock_client.cpp
-        storage/storage_mock_client.hpp
-        storage/mongo/mongo_client_wrapper.hpp
-        storage/mongo/mongo_mock_client.hpp
-        storage/mongo/mongo_mock_client.cpp
         stream/aggregator.cpp
         stream/append_map.cpp
         stream/piloted_clock.cpp

diff --git a/cpp/arcticdb/python/python_module.cpp b/cpp/arcticdb/python/python_module.cpp
@@ -207,7 +207,7 @@ void register_error_code_ecosystem(py::module& m, py::exception<arcticdb::Arctic
 
     static py::exception<InternalException> internal_exception(m, "InternalException", compat_exception.ptr());
     static py::exception<StorageException> storage_exception(m, "StorageException", compat_exception.ptr());
-    static py::exception<::lmdb::map_full_error> lmdb_map_full_error(m, "LmdbMapFullError", storage_exception.ptr());
+    static py::exception<LMDBMapFullException> lmdb_map_full_exception(m, "LmdbMapFullError", storage_exception.ptr());
     static py::exception<UserInputException> user_input_exception(m, "UserInputException", compat_exception.ptr());
 
     py::register_exception_translator([](std::exception_ptr p) {
@@ -219,13 +219,13 @@ void register_error_code_ecosystem(py::module& m, py::exception<arcticdb::Arctic
             user_input_exception(e.what());
         } catch (const arcticdb::InternalException& e){
             internal_exception(e.what());
-        } catch (const ::lmdb::map_full_error& e) {
+        } catch (const LMDBMapFullException& e) {
             std::string msg = fmt::format("E5003: LMDB map is full. Close and reopen your LMDB backed Arctic instance with a "
                                           "larger map size. For example to open `/tmp/a/b/` with a map size of 5GB, "
                                           "use `Arctic(\"lmdb:///tmp/a/b?map_size=5GB\")`. Also see the "
                                           "[LMDB documentation](http://www.lmdb.tech/doc/group__mdb.html#gaa2506ec8dab3d969b0e609cd82e619e5). "
-                                          "LMDB info: code=[{}] origin=[{}] message=[{}]", e.code(), e.origin(), e.what());
-            lmdb_map_full_error(msg.c_str());
+                                          "LMDB info: message=[{}]", e.what());
+            lmdb_map_full_exception(msg.c_str());
         } catch (const StorageException& e) {
             storage_exception(e.what());
         } catch (const py::stop_iteration &e){

diff --git a/cpp/arcticdb/storage/lmdb/lmdb_client_wrapper.hpp b/cpp/arcticdb/storage/lmdb/lmdb_client_wrapper.hpp
@@ -0,0 +1,65 @@
+/* Copyright 2024 Man Group Operations Limited
+ *
+ * Use of this software is governed by the Business Source License 1.1 included in the file licenses/BSL.txt.
+ *
+ * As of the Change Date specified in that file, in accordance with the Business Source License, use of this software will be governed by the Apache License, version 2.0.
+ */
+
+#pragma once
+
+#include <arcticdb/codec/segment.hpp>
+#include <arcticdb/entity/atom_key.hpp>
+#include <arcticdb/entity/variant_key.hpp>
+#include <arcticdb/storage/storage_utils.hpp>
+
+// LMDB++ is using `std::is_pod` in `lmdb++.h`, which is deprecated as of C++20.
+// See: https://github.com/drycpp/lmdbxx/blob/0b43ca87d8cfabba392dfe884eb1edb83874de02/lmdb%2B%2B.h#L1068
+// See: https://en.cppreference.com/w/cpp/types/is_pod
+// This suppresses the warning.
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+#ifdef ARCTICDB_USING_CONDA
+#include <lmdb++.h>
+#else
+#include <third_party/lmdbxx/lmdb++.h>
+#endif
+#pragma GCC diagnostic pop
+
+
+namespace arcticdb::storage::lmdb {
+
+class LmdbClientWrapper {
+public:
+    virtual bool exists(
+            const std::string& db_name,
+            std::string& path,
+            ::lmdb::txn& txn,
+            ::lmdb::dbi& dbi) const = 0;
+
+    virtual std::optional<Segment> read(
+            const std::string& db_name,
+            std::string& path,
+            ::lmdb::txn& txn,
+            ::lmdb::dbi& dbi) const = 0;
+
+    virtual void write(
+            const std::string& db_name,
+            std::string& path,
+            Segment&& segment,
+            ::lmdb::txn& txn,
+            ::lmdb::dbi& dbi,
+            int64_t overwrite_flag) = 0;
+
+    virtual bool remove(const std::string& db_name, std::string& path, ::lmdb::txn& txn, ::lmdb::dbi& dbi) = 0;
+
+    virtual std::vector<VariantKey> list(
+            const std::string& db_name,
+            const std::string& prefix,
+            ::lmdb::txn& txn,
+            ::lmdb::dbi& dbi,
+            KeyType key_type) const = 0;
+
+    virtual ~LmdbClientWrapper() = default;
+};
+
+} // namespace arcticdb::storage::lmdb
diff --git a/cpp/arcticdb/storage/lmdb/lmdb_mock_client.cpp b/cpp/arcticdb/storage/lmdb/lmdb_mock_client.cpp
@@ -0,0 +1,135 @@
+/* Copyright 2024 Man Group Operations Limited
+ *
+ * Use of this software is governed by the Business Source License 1.1 included in the file licenses/BSL.txt.
+ *
+ * As of the Change Date specified in that file, in accordance with the Business Source License, use of this software will be governed by the Apache License, version 2.0.
+ */
+
+#include <arcticdb/storage/lmdb/lmdb_mock_client.hpp>
+
+#include <arcticdb/codec/segment.hpp>
+#include <arcticdb/entity/atom_key.hpp>
+#include <arcticdb/entity/serialized_key.hpp>
+#include <arcticdb/util/string_utils.hpp>
+
+
+namespace arcticdb::storage::lmdb {
+
+std::string MockLmdbClient::get_failure_trigger(
+        const std::string& path,
+        StorageOperation operation_to_fail,
+        int error_code) {
+    return fmt::format("{}#Failure_{}_{}", path, operation_to_string(operation_to_fail), error_code);
+}
+
+std::string_view lmdb_operation_string(StorageOperation operation) {
+    switch (operation) {
+        case StorageOperation::READ:
+            return "mdb_get";
+        case StorageOperation::WRITE:
+            return "mdb_put";
+        case StorageOperation::DELETE:
+            return "mdb_del";
+        case StorageOperation::LIST:
+            return "mdb_cursor_get";
+        case StorageOperation::EXISTS:
+            return "mdb_get";
+        default:
+            return "unknown";
+    }
+}
+
+void raise_if_has_failure_trigger(const LmdbKey& key, StorageOperation operation) {
+    auto path = key.path_;
+    auto failure_string_for_operation = "#Failure_" + operation_to_string(operation) + "_";
+    auto position = path.rfind(failure_string_for_operation);
+
+    if (position == std::string::npos) {
+        return;
+    }
+
+    int error_code = 0;
+    try {
+        auto start = position + failure_string_for_operation.size();
+        error_code = stoi(path.substr(start));
+        auto error_message = fmt::format("Simulated Error, message: operation {}, error code {}",
+                                         operation_to_string(operation), error_code);
+    } catch (std::exception&) {
+        return;
+    }
+
+    if (error_code != 0) {
+        ::lmdb::error::raise(lmdb_operation_string(operation).data(), error_code);
+    }
+}
+
+void raise_key_exists_error(std::string_view lmdb_op) {
+    ::lmdb::error::raise(lmdb_op.data(), MDB_KEYEXIST);
+}
+
+bool MockLmdbClient::has_key(const LmdbKey& key) const {
+    return lmdb_contents_.find(key) != lmdb_contents_.end();
+}
+
+bool MockLmdbClient::exists(const std::string& db_name, std::string& path, ::lmdb::txn&, ::lmdb::dbi&) const {
+    LmdbKey key = {db_name, path};
+    raise_if_has_failure_trigger(key, StorageOperation::EXISTS);
+
+    return has_key(key);
+}
+
+std::optional<Segment> MockLmdbClient::read(const std::string& db_name, std::string& path, ::lmdb::txn&, ::lmdb::dbi&) const {
+    LmdbKey key = {db_name, path};
+    raise_if_has_failure_trigger(key, StorageOperation::READ);
+
+    if (!has_key(key)) {
+        return std::nullopt;
+    }
+
+    return lmdb_contents_.at(key);
+}
+
+void MockLmdbClient::write(const std::string& db_name, std::string& path, arcticdb::Segment&& segment,
+                           ::lmdb::txn&, ::lmdb::dbi&, int64_t) {
+    LmdbKey key = {db_name, path};
+    raise_if_has_failure_trigger(key, StorageOperation::WRITE);
+
+    if(has_key(key)) {
+        raise_key_exists_error(lmdb_operation_string(StorageOperation::WRITE));
+    } else {
+        lmdb_contents_.insert({key, segment});
+    }
+}
+
+bool MockLmdbClient::remove(const std::string& db_name, std::string& path, ::lmdb::txn&, ::lmdb::dbi&) {
+    LmdbKey key = {db_name, path};
+    raise_if_has_failure_trigger(key, StorageOperation::DELETE);
+
+    if (!has_key(key)) {
+        return false;
+    }
+
+    lmdb_contents_.erase(key);
+    return true;
+}
+
+std::vector<VariantKey> MockLmdbClient::list(const std::string& db_name, const std::string& prefix, ::lmdb::txn&,
+                                             ::lmdb::dbi&, KeyType key_type) const {
+    std::vector<VariantKey> found_keys;
+
+    for (const auto& [key, segment] : lmdb_contents_) {
+        if (key.db_name_ == db_name && util::string_starts_with(prefix, key.path_)) {
+            raise_if_has_failure_trigger(key, StorageOperation::LIST);
+
+            auto k = variant_key_from_bytes(
+                    reinterpret_cast<const uint8_t *>(key.path_.data()),
+                    key.path_.size(),
+                    key_type);
+            found_keys.push_back(k);
+        }
+    }
+
+    return found_keys;
+}
+
+} // namespace arcticdb::storage::lmdb
diff --git a/cpp/arcticdb/storage/lmdb/lmdb_mock_client.hpp b/cpp/arcticdb/storage/lmdb/lmdb_mock_client.hpp
@@ -0,0 +1,78 @@
+/* Copyright 2024 Man Group Operations Limited
+ *
+ * Use of this software is governed by the Business Source License 1.1 included in the file licenses/BSL.txt.
+ *
+ * As of the Change Date specified in that file, in accordance with the Business Source License, use of this software will be governed by the Apache License, version 2.0.
+ */
+
+#pragma once
+
+#include <arcticdb/storage/lmdb/lmdb_client_wrapper.hpp>
+
+#include <arcticdb/codec/segment.hpp>
+#include <arcticdb/entity/atom_key.hpp>
+#include <arcticdb/entity/variant_key.hpp>
+#include <arcticdb/storage/storage_mock_client.hpp>
+#include <arcticdb/storage/storage_utils.hpp>
+
+
+namespace arcticdb::storage::lmdb {
+
+struct LmdbKey {
+    std::string db_name_;
+    std::string path_;
+
+    bool operator==(const LmdbKey& other) const {
+        return std::pair(db_name_, path_) == std::pair(other.db_name_, other.path_);
+    }
+};
+
+struct LmdbKeyHash {
+    std::size_t operator()(const LmdbKey& k) const {
+        return std::hash<std::pair<std::string, std::string>>{}(std::pair(k.db_name_, k.path_));
+    }
+};
+
+class MockLmdbClient : public LmdbClientWrapper {
+public:
+    static std::string get_failure_trigger(
+            const std::string& path,
+            StorageOperation operation_to_fail,
+            int error_code);
+
+    bool exists(
+            const std::string& db_name,
+            std::string& path,
+            ::lmdb::txn& txn,
+            ::lmdb::dbi& dbi) const override;
+
+    std::optional<Segment> read(
+            const std::string& db_name,
+            std::string& path,
+            ::lmdb::txn& txn,
+            ::lmdb::dbi& dbi) const override;
+
+    void write(
+            const std::string& db_name,
+            std::string& path,
+            Segment&& segment,
+            ::lmdb::txn& txn,
+            ::lmdb::dbi& dbi,
+            int64_t overwrite_flag) override;
+
+    bool remove(const std::string& db_name, std::string& path, ::lmdb::txn& txn, ::lmdb::dbi& dbi) override;
+
+    std::vector<VariantKey> list(
+            const std::string& db_name,
+            const std::string& prefix,
+            ::lmdb::txn& txn,
+            ::lmdb::dbi& dbi,
+            KeyType key_type) const override;
+
+private:
+    std::unordered_map<LmdbKey, Segment, LmdbKeyHash> lmdb_contents_;
+
+    bool has_key(const LmdbKey& key) const;
+};
+
+} // namespace arcticdb::storage::lmdb