Merge pull request #464 from evoskuil/master

Change restore to leave store open.
libbitcoin · May 9, 2024 · 6dc6134 · 6dc6134
2 parents f644e3d + a2a67a4
commit 6dc6134
Show file tree

Hide file tree

Showing 7 changed files with 84 additions and 59 deletions.
diff --git a/include/bitcoin/database/impl/query/archive.ipp b/include/bitcoin/database/impl/query/archive.ipp
@@ -435,6 +435,7 @@ typename CLASS::transactions_ptr CLASS::get_transactions(
 TEMPLATE
 size_t CLASS::get_candidate_size() const NOEXCEPT
 {
+    // If the store is not opened this will be a max_size loop.
     return get_candidate_size(get_top_candidate());
 }
 
@@ -451,6 +452,7 @@ size_t CLASS::get_candidate_size(size_t top) const NOEXCEPT
 TEMPLATE
 size_t CLASS::get_confirmed_size() const NOEXCEPT
 {
+    // If the store is not opened this will be a max_size loop.
     return get_confirmed_size(get_top_confirmed());
 }
 

diff --git a/include/bitcoin/database/impl/store.ipp b/include/bitcoin/database/impl/store.ipp
@@ -779,6 +779,7 @@ code CLASS::restore(const event_handler& handler) NOEXCEPT
     // Requires that the store is already flush locked (corrupted).
     if (!flush_lock_.is_locked())
     {
+        /* bool */ process_lock_.try_unlock();
         transactor_mutex_.unlock();
         return error::flush_lock;
     }
@@ -819,43 +820,56 @@ code CLASS::restore(const event_handler& handler) NOEXCEPT
         ec = error::missing_snapshot;
     }
 
-    const auto restore = [&handler](auto& storage, table_t table) NOEXCEPT
+    const auto restore = [&handler](auto& ec, auto& storage, table_t table) NOEXCEPT
     {
-        handler(event_t::restore_table, table);
-        return storage.restore();
+        if (!ec)
+        {
+            handler(event_t::restore_table, table);
+            if (!storage.restore())
+                ec = error::restore_table;
+        }
     };
 
     if (!ec)
-    {
         ec = open_load(handler);
 
-        if (!restore(header, table_t::header_table)) ec = error::restore_table;
-        if (!restore(input, table_t::input_table)) ec = error::restore_table;
-        if (!restore(output, table_t::output_table)) ec = error::restore_table;
-        if (!restore(point, table_t::point_table)) ec = error::restore_table;
-        if (!restore(puts, table_t::puts_table)) ec = error::restore_table;
-        if (!restore(spend, table_t::spend_table)) ec = error::restore_table;
-        if (!restore(tx, table_t::tx_table)) ec = error::restore_table;
-        if (!restore(txs, table_t::txs_table)) ec = error::restore_table;
-
-        if (!restore(candidate, table_t::candidate_table)) ec = error::restore_table;
-        if (!restore(confirmed, table_t::confirmed_table)) ec = error::restore_table;
-        if (!restore(strong_tx, table_t::strong_tx_table)) ec = error::restore_table;
-
-        if (!restore(validated_bk, table_t::validated_bk_table)) ec = error::restore_table;
-        if (!restore(validated_tx, table_t::validated_tx_table)) ec = error::restore_table;
-
-        if (!restore(address, table_t::address_table)) ec = error::restore_table;
-        if (!restore(neutrino, table_t::neutrino_table)) ec = error::restore_table;
-        ////if (!restore(bootstrap, table_t::bootstrap_table)) ec = error::restore_table;
-        ////if (!restore(buffer, table_t::buffer_table)) ec = error::restore_table;
+    if (!ec)
+    {
+        restore(ec, header, table_t::header_table);
+        restore(ec, input, table_t::input_table);
+        restore(ec, output, table_t::output_table);
+        restore(ec, point, table_t::point_table);
+        restore(ec, puts, table_t::puts_table);
+        restore(ec, spend, table_t::spend_table);
+        restore(ec, tx, table_t::tx_table);
+        restore(ec, txs, table_t::txs_table);
+
+        restore(ec, candidate, table_t::candidate_table);
+        restore(ec, confirmed, table_t::confirmed_table);
+        restore(ec, strong_tx, table_t::strong_tx_table);
+
+        restore(ec, validated_bk, table_t::validated_bk_table);
+        restore(ec, validated_tx, table_t::validated_tx_table);
+
+        restore(ec, address, table_t::address_table);
+        restore(ec, neutrino, table_t::neutrino_table);
+        ////restore(ec, bootstrap, table_t::bootstrap_table);
+        ////restore(ec, buffer, table_t::buffer_table);
+
+        if (ec)
+            /* code */ unload_close(handler);
+    }
 
-        if (!ec) ec = unload_close(handler);
+    if (ec)
+    {
+        // unlock errors override ec.
+        // on failure flush_lock is left in place (store corrupt).
+        // on success process and flush locks are held until close().
+        ////if (!flush_lock_.try_unlock()) ec = error::flush_unlock;
+        if (!process_lock_.try_unlock()) ec = error::process_unlock;
     }
 
-    // unlock errors override ec.
-    if (!flush_lock_.try_unlock()) ec = error::flush_unlock;
-    if (!process_lock_.try_unlock()) ec = error::process_unlock;
+    // store is open after successful restore but not otherwise.
     transactor_mutex_.unlock();
     return ec;
 }

diff --git a/include/bitcoin/database/store.hpp b/include/bitcoin/database/store.hpp
@@ -59,10 +59,10 @@ class store
     /// Open and load the set of tables, set locks.
     code open(const event_handler& handler) NOEXCEPT;
 
-    /// Snapshot the set of tables (from loaded).
+    /// Snapshot the set of tables (from loaded, leaves loaded).
     code snapshot(const event_handler& handler) NOEXCEPT;
 
-    /// Restore the most recent snapshot (from unloaded).
+    /// Restore the most recent snapshot (from unloaded, leave loaded).
     code restore(const event_handler& handler) NOEXCEPT;
 
     /// Unload and close the set of tables, clear locks.

diff --git a/install-cmake.sh b/install-cmake.sh
@@ -530,7 +530,7 @@ unpack_from_tarball()
     local COMPRESSION=$3
     local BUILD=$4
 
-    display_heading_message "Prepairing to aquire $ARCHIVE"
+    display_heading_message "Preparing to acquire $ARCHIVE"
 
     if [[ ! ($BUILD) ]]; then
         display_message "Skipping unpack of $ARCHIVE..."
@@ -576,7 +576,7 @@ build_from_tarball()
         return
     fi
 
-    display_heading_message "Prepairing to build $ARCHIVE"
+    display_heading_message "Preparing to build $ARCHIVE"
 
     # Because ICU tools don't know how to locate internal dependencies.
     if [[ ($ARCHIVE == "$ICU_ARCHIVE") ]]; then
@@ -634,7 +634,7 @@ create_from_github()
 
     FORK="$ACCOUNT/$REPO"
 
-    display_heading_message "Prepairing to aquire $FORK/$BRANCH"
+    display_heading_message "Preparing to acquire $FORK/$BRANCH"
 
     if [[ -d "$REPO" ]]; then
         if [[ true ]]; then
@@ -669,7 +669,7 @@ build_from_github()
     # Join generated and command line options.
     local CONFIGURATION=("${OPTIONS[@]}" "$@")
 
-    display_heading_message "Prepairing to build $REPO"
+    display_heading_message "Preparing to build $REPO"
 
     # Build the local repository clone.
     make_project_directory "$REPO" "$JOBS" "$TEST" "${CONFIGURATION[@]}"
@@ -737,7 +737,7 @@ build_from_github_cmake()
     # Join generated and command line options.
     local CONFIGURATION=("${OPTIONS[@]}" "$@")
 
-    display_heading_message "Prepairing to build $REPO"
+    display_heading_message "Preparing to build $REPO"
 
     # Build the local repository clone.
     cmake_project_directory "$REPO" "$JOBS" "$TEST" "${CONFIGURATION[@]}"
@@ -823,7 +823,7 @@ build_from_tarball_boost()
         return
     fi
 
-    display_heading_message "Prepairing to build $ARCHIVE"
+    display_heading_message "Preparing to build $ARCHIVE"
 
     local TARGET="build-$ARCHIVE"
 

diff --git a/install-cmakepresets.sh b/install-cmakepresets.sh
@@ -576,7 +576,7 @@ unpack_from_tarball()
     local COMPRESSION=$3
     local BUILD=$4
 
-    display_heading_message "Prepairing to aquire $ARCHIVE"
+    display_heading_message "Preparing to acquire $ARCHIVE"
 
     if [[ ! ($BUILD) ]]; then
         display_message "Skipping unpack of $ARCHIVE..."
@@ -622,7 +622,7 @@ build_from_tarball()
         return
     fi
 
-    display_heading_message "Prepairing to build $ARCHIVE"
+    display_heading_message "Preparing to build $ARCHIVE"
 
     # Because ICU tools don't know how to locate internal dependencies.
     if [[ ($ARCHIVE == "$ICU_ARCHIVE") ]]; then
@@ -680,7 +680,7 @@ create_from_github()
 
     FORK="$ACCOUNT/$REPO"
 
-    display_heading_message "Prepairing to aquire $FORK/$BRANCH"
+    display_heading_message "Preparing to acquire $FORK/$BRANCH"
 
     if [[ -d "$REPO" ]]; then
         if [[ true ]]; then
@@ -715,7 +715,7 @@ build_from_github()
     # Join generated and command line options.
     local CONFIGURATION=("${OPTIONS[@]}" "$@")
 
-    display_heading_message "Prepairing to build $REPO"
+    display_heading_message "Preparing to build $REPO"
 
     # Build the local repository clone.
     make_project_directory "$REPO" "$JOBS" "$TEST" "${CONFIGURATION[@]}"
@@ -791,7 +791,7 @@ build_from_github_cmake()
     # Join generated and command line options.
     local CONFIGURATION=("${OPTIONS[@]}" "$@")
 
-    display_heading_message "Prepairing to build $REPO"
+    display_heading_message "Preparing to build $REPO"
 
     # Build the local repository clone.
     cmake_project_directory "$REPO" "$PRESET" "$JOBS" "$TEST" "${CONFIGURATION[@]}"
@@ -877,7 +877,7 @@ build_from_tarball_boost()
         return
     fi
 
-    display_heading_message "Prepairing to build $ARCHIVE"
+    display_heading_message "Preparing to build $ARCHIVE"
 
     local TARGET="build-$ARCHIVE"
 

diff --git a/install.sh b/install.sh
@@ -473,7 +473,7 @@ unpack_from_tarball()
     local COMPRESSION=$3
     local BUILD=$4
 
-    display_heading_message "Prepairing to aquire $ARCHIVE"
+    display_heading_message "Preparing to acquire $ARCHIVE"
 
     if [[ ! ($BUILD) ]]; then
         display_message "Skipping unpack of $ARCHIVE..."
@@ -519,7 +519,7 @@ build_from_tarball()
         return
     fi
 
-    display_heading_message "Prepairing to build $ARCHIVE"
+    display_heading_message "Preparing to build $ARCHIVE"
 
     # Because ICU tools don't know how to locate internal dependencies.
     if [[ ($ARCHIVE == "$ICU_ARCHIVE") ]]; then
@@ -577,7 +577,7 @@ create_from_github()
 
     FORK="$ACCOUNT/$REPO"
 
-    display_heading_message "Prepairing to aquire $FORK/$BRANCH"
+    display_heading_message "Preparing to acquire $FORK/$BRANCH"
 
     if [[ -d "$REPO" ]]; then
         if [[ true ]]; then
@@ -612,7 +612,7 @@ build_from_github()
     # Join generated and command line options.
     local CONFIGURATION=("${OPTIONS[@]}" "$@")
 
-    display_heading_message "Prepairing to build $REPO"
+    display_heading_message "Preparing to build $REPO"
 
     # Build the local repository clone.
     make_project_directory "$REPO" "$JOBS" "$TEST" "${CONFIGURATION[@]}"
@@ -698,7 +698,7 @@ build_from_tarball_boost()
         return
     fi
 
-    display_heading_message "Prepairing to build $ARCHIVE"
+    display_heading_message "Preparing to build $ARCHIVE"
 
     local TARGET="build-$ARCHIVE"
 

diff --git a/test/store.cpp b/test/store.cpp
@@ -459,43 +459,48 @@ BOOST_AUTO_TEST_CASE(store__restore__process_locked__success)
 }
 #endif
 
-BOOST_AUTO_TEST_CASE(store__restore__no_flush_locked__flush_lock)
+BOOST_AUTO_TEST_CASE(store__restore__no_flush_lock__flush_lock)
 {
     settings configuration{};
     configuration.path = TEST_DIRECTORY;
     test::map_store instance{ configuration };
     BOOST_REQUIRE_EQUAL(instance.restore(events), error::flush_lock);
 }
 
-BOOST_AUTO_TEST_CASE(store__restore__process_lock_file__missing_snapshot)
+BOOST_AUTO_TEST_CASE(store__restore__missing_snapshot__missing_snapshot_locks)
 {
     settings configuration{};
     configuration.path = TEST_DIRECTORY;
     test::map_store instance{ configuration };
     BOOST_REQUIRE(test::create(instance.process_lock_file()));
     BOOST_REQUIRE(test::create(flush_lock_file(configuration.path)));
     BOOST_REQUIRE_EQUAL(instance.restore(events), error::missing_snapshot);
+    BOOST_REQUIRE(test::exists(instance.flush_lock_file()));
+    BOOST_REQUIRE(!test::exists(instance.process_lock_file()));
 }
 
-BOOST_AUTO_TEST_CASE(store__restore__failure__unlocks)
+BOOST_AUTO_TEST_CASE(store__restore__success__success_locks)
 {
     settings configuration{};
     configuration.path = TEST_DIRECTORY;
     test::map_store instance{ configuration };
     BOOST_REQUIRE(test::create(flush_lock_file(configuration.path)));
     BOOST_REQUIRE_NE(instance.restore(events), error::success);
-    BOOST_REQUIRE(!test::exists(instance.flush_lock_file()));
+    BOOST_REQUIRE(test::exists(instance.flush_lock_file()));
     BOOST_REQUIRE(!test::exists(instance.process_lock_file()));
     BOOST_REQUIRE(instance.transactor_mutex().try_lock());
 }
 
-BOOST_AUTO_TEST_CASE(store__restore__no_backups__missing_snapshot)
+BOOST_AUTO_TEST_CASE(store__restore__no_backups__missing_snapshot_locks)
 {
     settings configuration{};
     configuration.path = TEST_DIRECTORY;
     test::map_store instance{ configuration };
     BOOST_REQUIRE(test::create(flush_lock_file(configuration.path)));
     BOOST_REQUIRE_EQUAL(instance.restore_(), error::missing_snapshot);
+    BOOST_REQUIRE(test::exists(instance.flush_lock_file()));
+    BOOST_REQUIRE(!test::exists(instance.process_lock_file()));
+    BOOST_REQUIRE(instance.transactor_mutex().try_lock());
 }
 
 // The lock is process-exclusive in linux/macOS, globally in win32.
@@ -520,7 +525,7 @@ BOOST_AUTO_TEST_CASE(store__restore__primary_open__clear_directory)
 }
 #endif
 
-BOOST_AUTO_TEST_CASE(store__restore__primary_closed__restore_table)
+BOOST_AUTO_TEST_CASE(store__restore__primary_closed__open_failure)
 {
     settings configuration{};
     configuration.path = TEST_DIRECTORY;
@@ -534,15 +539,15 @@ BOOST_AUTO_TEST_CASE(store__restore__primary_closed__restore_table)
 
     // There are no backup index files to open.
     BOOST_REQUIRE(test::create(flush_lock_file(configuration.path)));
-    BOOST_REQUIRE_EQUAL(instance.restore_(), error::restore_table);
+    BOOST_REQUIRE_EQUAL(instance.restore_(), error::open_failure);
 
     // Rename /primary to /indexes and copy to /primary.
     ////BOOST_REQUIRE(!test::folder(configuration.path / schema::dir::primary));
     BOOST_REQUIRE(test::folder(configuration.path / schema::dir::primary));
     BOOST_REQUIRE(test::folder(configuration.path / schema::dir::heads));
 }
 
-BOOST_AUTO_TEST_CASE(store__restore__secondary_closed__restore_table)
+BOOST_AUTO_TEST_CASE(store__restore__secondary_closed__open_failure)
 {
     settings configuration{};
     configuration.path = TEST_DIRECTORY;
@@ -556,14 +561,14 @@ BOOST_AUTO_TEST_CASE(store__restore__secondary_closed__restore_table)
 
     // There are no backup index files to open.
     BOOST_REQUIRE(test::create(flush_lock_file(configuration.path)));
-    BOOST_REQUIRE_EQUAL(instance.restore_(), error::restore_table);
+    BOOST_REQUIRE_EQUAL(instance.restore_(), error::open_failure);
 
     // No primary, so rename /secondary to /indexes.
     BOOST_REQUIRE(!test::folder(configuration.path / schema::dir::secondary));
     BOOST_REQUIRE(test::folder(configuration.path / schema::dir::heads));
 }
 
-BOOST_AUTO_TEST_CASE(store__restore__primary_secondary_loaded__restore_table)
+BOOST_AUTO_TEST_CASE(store__restore__primary_secondary_loaded__open_failure)
 {
     settings configuration{};
     configuration.path = TEST_DIRECTORY;
@@ -578,7 +583,7 @@ BOOST_AUTO_TEST_CASE(store__restore__primary_secondary_loaded__restore_table)
 
     // There are no backup index files to open.
     BOOST_REQUIRE(test::create(flush_lock_file(configuration.path)));
-    BOOST_REQUIRE_EQUAL(instance.restore_(), error::restore_table);
+    BOOST_REQUIRE_EQUAL(instance.restore_(), error::open_failure);
 
     // Rename /primary to /indexes and copy to /primary.
     ////BOOST_REQUIRE(!test::folder(configuration.path / schema::dir::primary));
@@ -606,9 +611,13 @@ BOOST_AUTO_TEST_CASE(store__restore__snapshot__success_unlocks)
     BOOST_REQUIRE(test::folder(configuration.path / schema::dir::primary));
     ////BOOST_REQUIRE(!test::folder(configuration.path / schema::dir::primary));
 
+    // leaves loaded and unlocked
+    BOOST_REQUIRE(test::exists(instance.flush_lock_file()));
+    BOOST_REQUIRE(test::exists(instance.process_lock_file()));
+
+    BOOST_REQUIRE_EQUAL(instance.close(events), error::success);
     BOOST_REQUIRE(!test::exists(instance.flush_lock_file()));
     BOOST_REQUIRE(!test::exists(instance.process_lock_file()));
-    BOOST_REQUIRE(instance.transactor_mutex().try_lock());
 }
 
 BOOST_AUTO_TEST_SUITE_END()