From 986aa9218769d229bafc106676f5aae02d43fca2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20W=C3=B3jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Wed, 4 Jun 2025 12:40:52 +0200 Subject: [PATCH] Respect read only mode for lmdb databases (#4913) * Remember store read only status * Respect read only mode for lmdb databases --- nano/core_test/block_store.cpp | 4 ---- nano/core_test/node.cpp | 15 +-------------- nano/node/make_store.cpp | 6 ++++-- nano/node/node.cpp | 16 ++++++++-------- nano/node/node.hpp | 16 ++++++++-------- nano/secure/ledger.cpp | 5 +++++ nano/store/component.cpp | 10 ++++++++++ nano/store/component.hpp | 9 +++++++++ nano/store/lmdb/lmdb.cpp | 32 ++++++++++++++++++++++++++++---- nano/store/lmdb/lmdb.hpp | 11 ++++------- nano/store/lmdb/lmdb_env.cpp | 10 +++++++++- nano/store/lmdb/lmdb_env.hpp | 7 +++++++ nano/store/rocksdb/rocksdb.cpp | 18 ++++++++++++------ nano/store/rocksdb/rocksdb.hpp | 5 +++-- 14 files changed, 108 insertions(+), 56 deletions(-) diff --git a/nano/core_test/block_store.cpp b/nano/core_test/block_store.cpp index cf1e948b..5bc4f8a5 100644 --- a/nano/core_test/block_store.cpp +++ b/nano/core_test/block_store.cpp @@ -1640,10 +1640,6 @@ TEST (block_store, incompatible_version) { auto store = nano::make_store (logger, path, nano::dev::constants, true); ASSERT_TRUE (store->init_error ()); - - auto transaction = store->tx_begin_read (); - auto version_l = store->version.get (transaction); - ASSERT_EQ (version_l, std::numeric_limits::max ()); } } diff --git a/nano/core_test/node.cpp b/nano/core_test/node.cpp index df7fb4a9..6dad6dda 100644 --- a/nano/core_test/node.cpp +++ b/nano/core_test/node.cpp @@ -89,23 +89,10 @@ TEST (node, block_store_path_failure) ASSERT_TRUE (node->wallets.items.empty ()); } -#if defined(__clang__) && defined(__linux__) && CI -// Disable test due to instability with clang and actions -TEST (node_DeathTest, DISABLED_readonly_block_store_not_exist) -#else TEST (node_DeathTest, readonly_block_store_not_exist) -#endif { // This is a read-only node with no ledger file - if (nano::rocksdb_config::using_rocksdb_in_tests ()) - { - nano::inactive_node node (nano::unique_path (), nano::inactive_node_flag_defaults ()); - ASSERT_TRUE (node.node->init_error ()); - } - else - { - ASSERT_EXIT (nano::inactive_node node (nano::unique_path (), nano::inactive_node_flag_defaults ()), ::testing::ExitedWithCode (1), ""); - } + ASSERT_THROW (nano::inactive_node (nano::unique_path (), nano::inactive_node_flag_defaults ()), std::runtime_error); } TEST (node, password_fanout) diff --git a/nano/node/make_store.cpp b/nano/node/make_store.cpp index 19f86278..4d947f30 100644 --- a/nano/node/make_store.cpp +++ b/nano/node/make_store.cpp @@ -15,16 +15,18 @@ std::unique_ptr nano::make_store (nano::logger & logger, return node_config.database_backend; }; + nano::store::open_mode const mode = read_only ? nano::store::open_mode::read_only : nano::store::open_mode::read_write; + auto backend = decide_backend (); switch (backend) { case nano::database_backend::lmdb: { - return std::make_unique (logger, add_db_postfix ? path / "data.ldb" : path, constants, node_config.diagnostics_config.txn_tracking, node_config.block_processor_batch_max_time, node_config.lmdb_config, node_config.backup_before_upgrade); + return std::make_unique (logger, add_db_postfix ? path / "data.ldb" : path, constants, node_config.diagnostics_config.txn_tracking, node_config.block_processor_batch_max_time, node_config.lmdb_config, node_config.backup_before_upgrade, mode); } case nano::database_backend::rocksdb: { - return std::make_unique (logger, add_db_postfix ? path / "rocksdb" : path, constants, node_config.rocksdb_config, read_only); + return std::make_unique (logger, add_db_postfix ? path / "rocksdb" : path, constants, node_config.rocksdb_config, mode); } } diff --git a/nano/node/node.cpp b/nano/node/node.cpp index 0782b138..da87ba4b 100644 --- a/nano/node/node.cpp +++ b/nano/node/node.cpp @@ -96,6 +96,14 @@ nano::node::node (std::shared_ptr io_ctx_a, std::filesy logger{ *logger_impl }, stats_impl{ std::make_unique (logger, config.stats_config) }, stats{ *stats_impl }, + store_impl{ nano::make_store (logger, application_path_a, network_params.ledger, flags.read_only, true, config_a) }, + store{ *store_impl }, + wallets_store_impl{ std::make_unique (application_path_a / "wallets.ldb", config_a.lmdb_config) }, + wallets_store{ *wallets_store_impl }, + wallets_impl{ std::make_unique (wallets_store.init_error (), *this) }, + wallets{ *wallets_impl }, + ledger_impl{ std::make_unique (store, network_params.ledger, stats, logger, flags_a.generate_cache, config_a.representative_vote_weight_minimum.number ()) }, + ledger{ *ledger_impl }, runner_impl{ std::make_unique (io_ctx_shared, logger, config.io_threads) }, runner{ *runner_impl }, observers_impl{ std::make_unique () }, @@ -111,16 +119,8 @@ nano::node::node (std::shared_ptr io_ctx_a, std::filesy work{ work_a }, distributed_work_impl{ std::make_unique (*this) }, distributed_work{ *distributed_work_impl }, - store_impl{ nano::make_store (logger, application_path_a, network_params.ledger, flags.read_only, true, config_a) }, - store{ *store_impl }, unchecked_impl{ std::make_unique (config.max_unchecked_blocks, stats, flags.disable_block_processor_unchecked_deletion) }, unchecked{ *unchecked_impl }, - wallets_store_impl{ std::make_unique (application_path_a / "wallets.ldb", config_a.lmdb_config) }, - wallets_store{ *wallets_store_impl }, - wallets_impl{ std::make_unique (wallets_store.init_error (), *this) }, - wallets{ *wallets_impl }, - ledger_impl{ std::make_unique (store, network_params.ledger, stats, logger, flags_a.generate_cache, config_a.representative_vote_weight_minimum.number ()) }, - ledger{ *ledger_impl }, ledger_notifications_impl{ std::make_unique (config, stats, logger) }, ledger_notifications{ *ledger_notifications_impl }, outbound_limiter_impl{ std::make_unique (config) }, diff --git a/nano/node/node.hpp b/nano/node/node.hpp index a05f6e0f..aa8e82ee 100644 --- a/nano/node/node.hpp +++ b/nano/node/node.hpp @@ -105,6 +105,14 @@ public: nano::logger & logger; std::unique_ptr stats_impl; nano::stats & stats; + std::unique_ptr store_impl; + nano::store::component & store; + std::unique_ptr wallets_store_impl; + nano::wallets_store & wallets_store; + std::unique_ptr wallets_impl; + nano::wallets & wallets; + std::unique_ptr ledger_impl; + nano::ledger & ledger; std::unique_ptr runner_impl; nano::thread_runner & runner; std::unique_ptr observers_impl; @@ -120,16 +128,8 @@ public: nano::work_pool & work; std::unique_ptr distributed_work_impl; nano::distributed_work_factory & distributed_work; - std::unique_ptr store_impl; - nano::store::component & store; std::unique_ptr unchecked_impl; nano::unchecked_map & unchecked; - std::unique_ptr wallets_store_impl; - nano::wallets_store & wallets_store; - std::unique_ptr wallets_impl; - nano::wallets & wallets; - std::unique_ptr ledger_impl; - nano::ledger & ledger; std::unique_ptr ledger_notifications_impl; nano::ledger_notifications & ledger_notifications; std::unique_ptr outbound_limiter_impl; diff --git a/nano/secure/ledger.cpp b/nano/secure/ledger.cpp index 70793b07..ec0566c5 100644 --- a/nano/secure/ledger.cpp +++ b/nano/secure/ledger.cpp @@ -743,6 +743,11 @@ nano::ledger::ledger (nano::store::component & store_a, nano::ledger_constants & { initialize (generate_cache_flags_a); } + else + { + logger.error (nano::log::type::ledger, "Ledger initialization failed, store initialization error"); + throw std::runtime_error ("Ledger initialization failed, store initialization error"); + } } nano::ledger::~ledger () diff --git a/nano/store/component.cpp b/nano/store/component.cpp index ffe9c6e7..1af3aad7 100644 --- a/nano/store/component.cpp +++ b/nano/store/component.cpp @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -39,3 +40,12 @@ void nano::store::component::initialize (store::write_transaction const & transa rep_weight.put (transaction_a, constants.genesis->account (), std::numeric_limits::max ()); ledger_cache_a.rep_weights.representation_put (constants.genesis->account (), std::numeric_limits::max ()); } + +/* + * + */ + +std::string_view nano::store::to_string (open_mode mode) +{ + return nano::enum_util::name (mode); +} \ No newline at end of file diff --git a/nano/store/component.hpp b/nano/store/component.hpp index 232601e9..1de32ea5 100644 --- a/nano/store/component.hpp +++ b/nano/store/component.hpp @@ -20,6 +20,14 @@ namespace nano { namespace store { + enum class open_mode + { + read_only, + read_write + }; + + std::string_view to_string (open_mode mode); + /** * Store manager */ @@ -89,6 +97,7 @@ namespace store virtual std::string vendor_get () const = 0; virtual std::filesystem::path get_database_path () const = 0; + virtual nano::store::open_mode get_mode () const = 0; }; } // namespace store } // namespace nano diff --git a/nano/store/lmdb/lmdb.cpp b/nano/store/lmdb/lmdb.cpp index a1f40f4a..9fc1cb74 100644 --- a/nano/store/lmdb/lmdb.cpp +++ b/nano/store/lmdb/lmdb.cpp @@ -17,7 +17,7 @@ template class nano::store::typed_iterator; -nano::store::lmdb::component::component (nano::logger & logger_a, std::filesystem::path const & path_a, nano::ledger_constants & constants, nano::txn_tracking_config const & txn_tracking_config_a, std::chrono::milliseconds block_processor_batch_max_time_a, nano::lmdb_config const & lmdb_config_a, bool backup_before_upgrade_a) : +nano::store::lmdb::component::component (nano::logger & logger_a, std::filesystem::path const & path_a, nano::ledger_constants & constants, nano::txn_tracking_config const & txn_tracking_config_a, std::chrono::milliseconds block_processor_batch_max_time_a, nano::lmdb_config const & lmdb_config_a, bool backup_before_upgrade_a, nano::store::open_mode mode_a) : // clang-format off nano::store::component{ block_store, @@ -43,8 +43,9 @@ nano::store::lmdb::component::component (nano::logger & logger_a, std::filesyste version_store{ *this }, rep_weight_store{ *this }, database_path{ path_a }, + mode{ mode_a }, logger{ logger_a }, - env (error, path_a, nano::store::lmdb::env::options::make ().set_config (lmdb_config_a).set_use_no_mem_init (true)), + env (error, path_a, nano::store::lmdb::env::options::make ().set_config (lmdb_config_a).set_use_no_mem_init (true).set_read_only (mode_a == nano::store::open_mode::read_only)), mdb_txn_tracker (logger_a, txn_tracking_config_a, block_processor_batch_max_time_a), txn_tracking_enabled (txn_tracking_config_a.enable) { @@ -72,6 +73,15 @@ nano::store::lmdb::component::component (nano::logger & logger_a, std::filesyste // (can be a few minutes with the --fast_bootstrap flag for instance) if (!is_fully_upgraded) { + if (mode == nano::store::open_mode::read_only) + { + // Either following cases cannot run in read-only mode: + // a) there is no database yet, the access needs to be in write mode for it to be created; + // b) it will upgrade, and it is not possible to do it in read-only mode. + error = true; + return; + } + if (!is_fresh_db) { logger.info (nano::log::type::lmdb, "Upgrade in progress..."); @@ -88,12 +98,21 @@ nano::store::lmdb::component::component (nano::logger & logger_a, std::filesyste if (!error) { error |= do_upgrades (transaction, constants, needs_vacuuming); + if (error) + { + logger.error (nano::log::type::lmdb, "Failed to upgrade database: {}", database_path.string ()); + return; + } + else + { + logger.info (nano::log::type::lmdb, "Database upgraded successfully to version {}", version_current); + } } } if (needs_vacuuming) { - logger.info (nano::log::type::lmdb, "Ledger vaccum in progress..."); + logger.info (nano::log::type::lmdb, "Ledger vacuum in progress..."); auto vacuum_success = vacuum_after_upgrade (path_a, lmdb_config_a); if (vacuum_success) @@ -102,7 +121,7 @@ nano::store::lmdb::component::component (nano::logger & logger_a, std::filesyste } else { - logger.error (nano::log::type::lmdb, "Ledger vaccum failed"); + logger.error (nano::log::type::lmdb, "Ledger vacuum failed"); logger.error (nano::log::type::lmdb, "(Optional) Please ensure enough disk space is available for a copy of the database and try to vacuum after shutting down the node"); } } @@ -192,6 +211,11 @@ std::filesystem::path nano::store::lmdb::component::get_database_path () const return database_path; } +nano::store::open_mode nano::store::lmdb::component::get_mode () const +{ + return mode; +} + nano::store::lmdb::txn_callbacks nano::store::lmdb::component::create_txn_callbacks () const { nano::store::lmdb::txn_callbacks mdb_txn_callbacks; diff --git a/nano/store/lmdb/lmdb.hpp b/nano/store/lmdb/lmdb.hpp index 78bfac47..c2329040 100644 --- a/nano/store/lmdb/lmdb.hpp +++ b/nano/store/lmdb/lmdb.hpp @@ -26,12 +26,6 @@ #include -namespace nano -{ -class logging_mt; - -} - namespace nano::store::lmdb { /** @@ -63,12 +57,14 @@ private: friend class nano::store::lmdb::rep_weight; public: - component (nano::logger &, std::filesystem::path const &, nano::ledger_constants & constants, nano::txn_tracking_config const & txn_tracking_config_a = nano::txn_tracking_config{}, std::chrono::milliseconds block_processor_batch_max_time_a = std::chrono::milliseconds (5000), nano::lmdb_config const & lmdb_config_a = nano::lmdb_config{}, bool backup_before_upgrade = false); + component (nano::logger &, std::filesystem::path const &, nano::ledger_constants & constants, nano::txn_tracking_config const & txn_tracking_config_a = nano::txn_tracking_config{}, std::chrono::milliseconds block_processor_batch_max_time_a = std::chrono::milliseconds (5000), nano::lmdb_config const & lmdb_config_a = nano::lmdb_config{}, bool backup_before_upgrade = false, nano::store::open_mode = nano::store::open_mode::read_write); + store::write_transaction tx_begin_write () override; store::read_transaction tx_begin_read () const override; std::string vendor_get () const override; std::filesystem::path get_database_path () const override; + nano::store::open_mode get_mode () const override; void serialize_mdb_tracker (boost::property_tree::ptree &, std::chrono::milliseconds, std::chrono::milliseconds) override; @@ -81,6 +77,7 @@ public: private: bool error{ false }; std::filesystem::path const database_path; + nano::store::open_mode const mode; nano::logger & logger; public: diff --git a/nano/store/lmdb/lmdb_env.cpp b/nano/store/lmdb/lmdb_env.cpp index a6b0fab2..1c491c68 100644 --- a/nano/store/lmdb/lmdb_env.cpp +++ b/nano/store/lmdb/lmdb_env.cpp @@ -37,6 +37,7 @@ void nano::store::lmdb::env::init (bool & error_a, std::filesystem::path const & } auto status3 (mdb_env_set_mapsize (environment, map_size)); release_assert (success (status3), error_string (status3)); + // It seems if there's ever more threads than mdb_env_set_maxreaders has read slots available, we get failures on transaction creation unless MDB_NOTLS is specified // This can happen if something like 256 io_threads are specified in the node config // MDB_NORDAHEAD will allow platforms that support it to load the DB in memory as needed. @@ -55,14 +56,21 @@ void nano::store::lmdb::env::init (bool & error_a, std::filesystem::path const & environment_flags |= MDB_NOSYNC | MDB_WRITEMAP | MDB_MAPASYNC; } + if (options_a.read_only) + { + environment_flags |= MDB_RDONLY; + } + if (!memory_intensive_instrumentation () && options_a.use_no_mem_init) { environment_flags |= MDB_NOMEMINIT; } + auto status4 (mdb_env_open (environment, path_a.string ().c_str (), environment_flags, 00600)); if (!success (status4)) { - std::string message = "Could not open lmdb environment(" + std::to_string (status4) + "): " + mdb_strerror (status4); + std::string message = "Could not open lmdb environment: (" + std::to_string (status4) + ") " + mdb_strerror (status4); + nano::default_logger ().error (nano::log::type::lmdb, "{}", message); throw std::runtime_error (message); } release_assert (success (status4), error_string (status4)); diff --git a/nano/store/lmdb/lmdb_env.hpp b/nano/store/lmdb/lmdb_env.hpp index 3d71e178..09dd7d65 100644 --- a/nano/store/lmdb/lmdb_env.hpp +++ b/nano/store/lmdb/lmdb_env.hpp @@ -30,6 +30,12 @@ public: return *this; } + options & set_read_only (bool read_only_a) + { + read_only = read_only_a; + return *this; + } + options & set_use_no_mem_init (int use_no_mem_init_a) { use_no_mem_init = use_no_mem_init_a; @@ -52,6 +58,7 @@ public: private: bool use_no_mem_init{ false }; + bool read_only{ false }; nano::lmdb_config config; }; diff --git a/nano/store/rocksdb/rocksdb.cpp b/nano/store/rocksdb/rocksdb.cpp index dcb02bb9..4ef2c798 100644 --- a/nano/store/rocksdb/rocksdb.cpp +++ b/nano/store/rocksdb/rocksdb.cpp @@ -38,7 +38,7 @@ private: }; } -nano::store::rocksdb::component::component (nano::logger & logger_a, std::filesystem::path const & path_a, nano::ledger_constants & constants, nano::rocksdb_config const & rocksdb_config_a, bool open_read_only_a) : +nano::store::rocksdb::component::component (nano::logger & logger_a, std::filesystem::path const & path_a, nano::ledger_constants & constants, nano::rocksdb_config const & rocksdb_config_a, nano::store::open_mode mode_a) : // clang-format off nano::store::component{ block_store, @@ -64,6 +64,7 @@ nano::store::rocksdb::component::component (nano::logger & logger_a, std::filesy version_store{ *this }, rep_weight_store{ *this }, database_path{ path_a }, + mode{ mode_a }, logger{ logger_a }, constants{ constants }, rocksdb_config{ rocksdb_config_a }, @@ -126,11 +127,11 @@ nano::store::rocksdb::component::component (nano::logger & logger_a, std::filesy if (is_fully_upgraded) { - open (error, path_a, open_read_only_a, options, create_column_families ()); + open (error, path_a, (mode == nano::store::open_mode::read_only), options, create_column_families ()); return; } - if (open_read_only_a) + if (mode == nano::store::open_mode::read_only) { // Either following cases cannot run in read-only mode: // a) there is no database yet, the access needs to be in write mode for it to be created; @@ -141,7 +142,7 @@ nano::store::rocksdb::component::component (nano::logger & logger_a, std::filesy if (is_fresh_db) { - open (error, path_a, open_read_only_a, options, create_column_families ()); + open (error, path_a, (mode == nano::store::open_mode::read_only), options, create_column_families ()); if (!error) { version.put (tx_begin_write (), version_current); // It is fresh, someone needs to tell it its version. @@ -150,7 +151,7 @@ nano::store::rocksdb::component::component (nano::logger & logger_a, std::filesy } // The database is not upgraded, and it may not be compatible with the current column family set. - open (error, path_a, open_read_only_a, options, get_current_column_families (path_a.string (), options)); + open (error, path_a, (mode == nano::store::open_mode::read_only), options, get_current_column_families (path_a.string (), options)); if (!error) { logger.info (nano::log::type::rocksdb, "Upgrade in progress..."); @@ -446,6 +447,11 @@ std::filesystem::path nano::store::rocksdb::component::get_database_path () cons return database_path; } +nano::store::open_mode nano::store::rocksdb::component::get_mode () const +{ + return mode; +} + std::vector<::rocksdb::ColumnFamilyDescriptor> nano::store::rocksdb::component::get_single_column_family (std::string cf_name) const { std::vector<::rocksdb::ColumnFamilyDescriptor> minimum_cf_set{ @@ -868,7 +874,7 @@ bool nano::store::rocksdb::component::copy_db (std::filesystem::path const & des // Open it so that it flushes all WAL files if (status.ok ()) { - nano::store::rocksdb::component rocksdb_store{ logger, destination_path.string (), constants, rocksdb_config, false }; + nano::store::rocksdb::component rocksdb_store{ logger, destination_path.string (), constants, rocksdb_config }; return !rocksdb_store.init_error (); } return false; diff --git a/nano/store/rocksdb/rocksdb.hpp b/nano/store/rocksdb/rocksdb.hpp index be1cfb7d..ed9131c3 100644 --- a/nano/store/rocksdb/rocksdb.hpp +++ b/nano/store/rocksdb/rocksdb.hpp @@ -26,7 +26,6 @@ namespace nano { -class logging_mt; class rocksdb_config; class rocksdb_block_store_tombstone_count_Test; } @@ -64,13 +63,14 @@ public: friend class nano::store::rocksdb::version; friend class nano::store::rocksdb::rep_weight; - explicit component (nano::logger &, std::filesystem::path const &, nano::ledger_constants & constants, nano::rocksdb_config const & = nano::rocksdb_config{}, bool open_read_only = false); + explicit component (nano::logger &, std::filesystem::path const &, nano::ledger_constants & constants, nano::rocksdb_config const & = nano::rocksdb_config{}, nano::store::open_mode = nano::store::open_mode::read_write); store::write_transaction tx_begin_write () override; store::read_transaction tx_begin_read () const override; std::string vendor_get () const override; std::filesystem::path get_database_path () const override; + nano::store::open_mode get_mode () const override; uint64_t count (store::transaction const & transaction_a, tables table_a) const override; @@ -93,6 +93,7 @@ public: private: bool error{ false }; std::filesystem::path const database_path; + nano::store::open_mode const mode; nano::logger & logger; nano::ledger_constants & constants; ::rocksdb::TransactionDB * transaction_db = nullptr;