Fix backing store switching issues

libraries/chain/combined_database.cpp

@@ -235,6 +235,19 @@ namespace eosio { namespace chain {
          kv_undo_stack(std::make_unique<eosio::session::undo_stack<rocks_db_type>>(*kv_database)),
          kv_snapshot_batch_threashold(cfg.persistent_storage_mbytes_batch * 1024 * 1024)  {}
 
+   void combined_database::check_backing_store_setting(bool clean_startup) {
+      if (backing_store != db.get<kv_db_config_object>().backing_store) {   
+         EOS_ASSERT(clean_startup, database_move_kv_disk_exception,
+                   "Existing state indicates a different backing store is in use; use resync, replay, or restore from snapshot to switch backing store");
+         db.modify(db.get<kv_db_config_object>(), [this](auto& cfg) { cfg.backing_store = backing_store; });
+      }
+
+      if (backing_store == backing_store_type::ROCKSDB)
+         ilog("using rocksdb for backing store");
+      else
+         ilog("using chainbase for backing store");
+   }
+
    void combined_database::destroy(const fc::path& p) {
       if( !fc::is_directory( p ) )
           return;
@@ -245,28 +258,6 @@ namespace eosio { namespace chain {
       rocks_db_type::destroy((p / "chain-kv").string());
    }
 
-   void combined_database::check_backing_store_setting() {
-      switch (backing_store) {
-      case backing_store_type::CHAINBASE:
-         EOS_ASSERT(db.get<kv_db_config_object>().backing_store == backing_store_type::CHAINBASE, database_move_kv_disk_exception,
-                    "Chainbase indicates that RocksDB is in use; resync, replay, or restore from snapshot to switch back to chainbase");
-         break;
-      case backing_store_type::ROCKSDB:
-         if (db.get<kv_db_config_object>().backing_store != backing_store_type::ROCKSDB) {
-            auto& idx = db.get_index<kv_index, by_kv_key>();
-            auto  it  = idx.lower_bound(std::make_tuple(name{}, std::string_view{}));
-            EOS_ASSERT(it == idx.end(), database_move_kv_disk_exception,
-                     "Chainbase already contains KV entries; use resync, replay, or snapshot to move these to "
-                     "rocksdb");
-            db.modify(db.get<kv_db_config_object>(), [](auto& cfg) { cfg.backing_store = backing_store_type::ROCKSDB; });
-         }
-      }
-      if (backing_store == backing_store_type::ROCKSDB)
-         ilog("using rocksdb for backing store");
-      else
-         ilog("using chainbase for backing store");
-   }
-
    void combined_database::set_revision(uint64_t revision) {
       db.set_revision(revision);
 
@@ -386,7 +377,7 @@ namespace eosio { namespace chain {
       });
 
       db.create<kv_db_config_object>([](auto&) {});
-      check_backing_store_setting();
+      check_backing_store_setting(true);
 
       { /// load and upgrade the block header state
          block_header_state head_header_state;

libraries/chain/controller.cpp

@@ -502,7 +502,7 @@ struct controller_impl {
          const auto hash = calculate_integrity_hash();
          ilog( "database initialized with hash: ${hash}", ("hash", hash) );
 
-         init(check_shutdown);
+         init(check_shutdown, true);
       } catch (boost::interprocess::bad_alloc& e) {
          elog( "db storage not configured to have enough storage for the provided snapshot, please increase and retry snapshot" );
          throw e;
@@ -548,7 +548,7 @@ struct controller_impl {
       } else {
          blog.reset( genesis, head->block, packed_transaction::cf_compression_type::none );
       }
-      init(check_shutdown);
+      init(check_shutdown, true);
    }
 
    void startup(std::function<void()> shutdown, std::function<bool()> check_shutdown) {
@@ -581,7 +581,7 @@ struct controller_impl {
       }
       head = fork_db.head();
 
-      init(check_shutdown);
+      init(check_shutdown, false);
    }
 
 
@@ -598,7 +598,7 @@ struct controller_impl {
       return header_itr;
    }
 
-   void init(std::function<bool()> check_shutdown) {
+   void init(std::function<bool()> check_shutdown, bool clean_startup) {
       uint32_t lib_num = (blog.head() ? blog.head()->block_num() : fork_db.root()->block_num);
 
       auto header_itr = validate_db_version( db );
@@ -618,7 +618,7 @@ struct controller_impl {
          });
       }
 
-      kv_db.check_backing_store_setting();
+      kv_db.check_backing_store_setting( clean_startup );
 
       // At this point head != nullptr && fork_db.head() != nullptr && fork_db.root() != nullptr.
       // Furthermore, fork_db.root()->block_num <= lib_num.

libraries/chain/include/eosio/chain/combined_database.hpp

@@ -93,10 +93,9 @@ namespace eosio { namespace chain {
       combined_database& operator=(const combined_database& copy) = delete;
 
       // Save the backing_store setting to the chainbase in order to detect
-      // when this setting is switched from chainbase to rocksdb, in which
-      // case, check that no KV entries already exist in the chainbase.
-      // Otherwise, they would become unreachable.
-      void check_backing_store_setting();
+      // when this setting is switched between chainbase and rocksdb.
+      // If existing state is not clean, switching is not allowed.
+      void check_backing_store_setting(bool clean_startup);
 
       static combined_session make_no_op_session() { return combined_session(); }
 

libraries/chain/include/eosio/chain/exceptions.hpp

@@ -331,7 +331,7 @@ namespace eosio { namespace chain {
       FC_DECLARE_DERIVED_EXCEPTION( database_revision_mismatch_exception, database_exception,
                                     3060006, "Chainbase and chain-kv databases are at different revisions" )
       FC_DECLARE_DERIVED_EXCEPTION( database_move_kv_disk_exception, database_exception,
-                                    3060007, "Chainbase already contains eosio.kvdisk entries; use resync, replay, or snapshot to move these to rocksdb" )
+                                    3060007, "Cannot change backing store when existing state has already stored data in a different backing store; use resync, replay, or snapshot to move these to the new backing store" )
       FC_DECLARE_DERIVED_EXCEPTION( kv_rocksdb_bad_value_size_exception, database_exception,
                                     3060008, "The size of value returned from RocksDB is less than payer's size" )
       FC_DECLARE_DERIVED_EXCEPTION( bad_composite_key_exception, database_exception,

libraries/chain_kv/include/b1/session/session.hpp

@@ -279,27 +279,18 @@ void session<Parent>::prime_cache_() {
       }
    };
 
-   std::visit(overloaded{ [&](session<Parent>* p) {
-                            auto begin = std::begin(p->m_cache);
-                            auto end   = std::end(p->m_cache);
-                            if (begin == end) {
-                               return;
-                            }
-                            update(begin->first, begin->second.value);
-                            --end;
-                            update(end->first, end->second.value);
-                         },
-                          [&](Parent* p) {
-                             auto begin = std::begin(*p);
-                             auto end   = std::end(*p);
-                             if (begin == end) {
-                                return;
-                             }
-                             update(begin.key(), (*begin).second.value());
-                             --end;
-                             update(end.key(), (*end).second.value());
-                          } },
-              m_parent);
+   std::visit(
+         [&](auto* p) {
+            auto begin = std::begin(*p);
+            auto end   = std::end(*p);
+            if (begin == end) {
+               return;
+            }
+            update(begin.key(), (*begin).second.value());
+            --end;
+            update(end.key(), (*end).second.value());
+         },
+         m_parent);
 }
 
 template <typename Parent>

libraries/testing/include/eosio/testing/tester.hpp

@@ -432,8 +432,6 @@ namespace eosio { namespace testing {
             return {cfg, gen};
          }
 
-         void restart_with_backing_store(chain::backing_store_type backing_store);
-
       protected:
          signed_block_ptr _produce_block( fc::microseconds skip_time, bool skip_pending_trxs );
          signed_block_ptr _produce_block( fc::microseconds skip_time, bool skip_pending_trxs,

libraries/testing/tester.cpp

@@ -1195,13 +1195,6 @@ namespace eosio { namespace testing {
       preactivate_protocol_features( preactivations );
    }
 
-   void base_tester::restart_with_backing_store(chain::backing_store_type backing_store) {
-      auto cfg = get_config();
-      close(); // clean up chain so no dirty db error
-      cfg.backing_store = backing_store;
-      init(cfg); // enable new config
-   }
-
    tester::tester(const std::function<void(controller&)>& control_setup, setup_policy policy, db_read_mode read_mode) {
       auto def_conf            = default_config(tempdir);
       def_conf.first.read_mode = read_mode;

unittests/api_tests.cpp

@@ -1655,7 +1655,6 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(more_deferred_transaction_tests, TYPE_T, backing_s
    TYPE_T bs_type;
    fc::temp_directory tempdir;
    validating_tester chain( tempdir, true, {bs_type} );
-   chain.restart_with_backing_store(bs_type);
    chain.execute_setup_policy( setup_policy::preactivate_feature_and_new_bios );
 
    const auto& pfm = chain.control->get_protocol_feature_manager();

unittests/kv_tests.cpp

@@ -39,7 +39,7 @@ FC_REFLECT(itparam, (db)(count)(erase))
 
 class kv_tester : public tester {
  public:
-   kv_tester() {
+   kv_tester(backing_store_type backing_store) : tester(setup_policy::full, db_read_mode::SPECULATIVE, std::optional<uint32_t>{}, std::optional<uint32_t>{}, backing_store) {
       produce_blocks(2);
 
       create_accounts({ "kvtest"_n, "kvtest1"_n, "kvtest2"_n, "kvtest3"_n, "kvtest4"_n });
@@ -690,87 +690,86 @@ class kv_tester : public tester {
    abi_serializer sys_abi_ser;
 };
 
+class kv_chainbase_tester : public kv_tester {
+ public:
+   kv_chainbase_tester() : kv_tester(backing_store_type::CHAINBASE) { }
+};
+
 class kv_rocksdb_tester : public kv_tester {
  public:
-   kv_rocksdb_tester() {
-      // Switch configuration option to RocksDB
-      close();
-      auto cfg = get_config();
-      cfg.backing_store = eosio::chain::backing_store_type::ROCKSDB;
-      init(cfg);
-   }
+   kv_rocksdb_tester() : kv_tester(backing_store_type::ROCKSDB) { }
 };
 
 BOOST_AUTO_TEST_SUITE(kv_tests)
 
-BOOST_FIXTURE_TEST_CASE(kv_basic, kv_tester) try {
+BOOST_FIXTURE_TEST_CASE(kv_basic, kv_chainbase_tester) try {
    test_kv_basic_common();
 }
 FC_LOG_AND_RETHROW()
 
-BOOST_FIXTURE_TEST_CASE(kv_scan, kv_tester) try {
+BOOST_FIXTURE_TEST_CASE(kv_scan, kv_chainbase_tester) try {
    test_kv_scan_common();
 }
 FC_LOG_AND_RETHROW()
 
-BOOST_FIXTURE_TEST_CASE(kv_scanrev, kv_tester) try {
+BOOST_FIXTURE_TEST_CASE(kv_scanrev, kv_chainbase_tester) try {
    test_kv_scanrev_common();
 }
 FC_LOG_AND_RETHROW()
 
-BOOST_FIXTURE_TEST_CASE(kv_scanrev2, kv_tester) try { //
+BOOST_FIXTURE_TEST_CASE(kv_scanrev2, kv_chainbase_tester) try { //
    test_kv_scanrev2_common();
 }
 FC_LOG_AND_RETHROW()
 
-BOOST_FIXTURE_TEST_CASE(kv_iterase, kv_tester) try { //
+BOOST_FIXTURE_TEST_CASE(kv_iterase, kv_chainbase_tester) try { //
    test_kv_iterase_common();
 }
 FC_LOG_AND_RETHROW()
 
-BOOST_FIXTURE_TEST_CASE(kv_ram_usage, kv_tester) try { //
+BOOST_FIXTURE_TEST_CASE(kv_ram_usage, kv_chainbase_tester) try { //
    test_kv_ram_usage_common();
 }
 FC_LOG_AND_RETHROW()
 
-BOOST_FIXTURE_TEST_CASE(kv_resource_limit, kv_tester) try { //
+BOOST_FIXTURE_TEST_CASE(kv_resource_limit, kv_chainbase_tester) try { //
    test_kv_resource_limit_common();
 }
 FC_LOG_AND_RETHROW()
 
-BOOST_FIXTURE_TEST_CASE(kv_key_value_limit, kv_tester) try { //
+BOOST_FIXTURE_TEST_CASE(kv_key_value_limit, kv_chainbase_tester) try { //
    test_kv_key_value_limit_common();
 }
 FC_LOG_AND_RETHROW()
 
 constexpr name databases[] = { "eosio.kvram"_n };
 
-BOOST_DATA_TEST_CASE_F(kv_tester, kv_inc_dec_usage, bdata::make(databases), db) try { //
+BOOST_DATA_TEST_CASE_F(kv_chainbase_tester, kv_inc_dec_usage, bdata::make(databases), db) try { //
    test_kv_inc_dec_usage();
 }
 FC_LOG_AND_RETHROW()
 
-BOOST_DATA_TEST_CASE_F(kv_tester, kv_inc_usage_and_limit, bdata::make(databases), db) try { //
+BOOST_DATA_TEST_CASE_F(kv_chainbase_tester, kv_inc_usage_and_limit, bdata::make(databases), db) try { //
    test_kv_inc_usage_and_limit();
 }
 FC_LOG_AND_RETHROW()
 
-BOOST_DATA_TEST_CASE_F(kv_tester, kv_dec_limit_and_usage, bdata::make(databases), db) try { //
+BOOST_DATA_TEST_CASE_F(kv_chainbase_tester, kv_dec_limit_and_usage, bdata::make(databases), db) try { //
    test_kv_dec_limit_and_usage();
 }
 FC_LOG_AND_RETHROW()
 
-BOOST_DATA_TEST_CASE_F(kv_tester, get_data, bdata::make(databases), db) try { //
+BOOST_DATA_TEST_CASE_F(kv_chainbase_tester, get_data, bdata::make(databases), db) try { //
    test_get_data();
 }
 FC_LOG_AND_RETHROW()
 
-BOOST_DATA_TEST_CASE_F(kv_tester, other_contract, bdata::make(databases), db) try { //
+BOOST_DATA_TEST_CASE_F(kv_chainbase_tester, other_contract, bdata::make(databases), db) try { //
    test_other_contract();
 }
 FC_LOG_AND_RETHROW()
 
-BOOST_FIXTURE_TEST_CASE(max_iterators, kv_tester) try { //
+BOOST_FIXTURE_TEST_CASE(max_iterators, kv_chainbase_tester) try { //
    test_max_iterators();
 }
 FC_LOG_AND_RETHROW()
@@ -847,7 +846,7 @@ BOOST_FIXTURE_TEST_CASE(max_iterators_rocksdb, kv_rocksdb_tester) try { //
 }
 FC_LOG_AND_RETHROW()
 
-BOOST_DATA_TEST_CASE_F(kv_tester, undo, bdata::make(databases) * bdata::make({false, true}), db, rocks) try { //
+BOOST_DATA_TEST_CASE_F(kv_chainbase_tester, undo, bdata::make(databases) * bdata::make({false, true}), db, rocks) try { //
    test_undo();
 }
 FC_LOG_AND_RETHROW()

unittests/snapshot_tests.cpp

@@ -609,10 +609,7 @@ static const char kv_snapshot_bios[] = R"=====(
 BOOST_AUTO_TEST_CASE_TEMPLATE(test_kv_snapshot, SNAPSHOT_SUITE, snapshot_suites) {
    for (backing_store_type origin_backing_store : { backing_store_type::CHAINBASE, backing_store_type::ROCKSDB }) {
       for (backing_store_type resulting_backing_store: { backing_store_type::CHAINBASE, backing_store_type::ROCKSDB }) {
-         tester chain;
-
-         // Set backing_store for save snapshot
-         chain.restart_with_backing_store(origin_backing_store);
+         tester chain {setup_policy::full, db_read_mode::SPECULATIVE, std::optional<uint32_t>{}, std::optional<uint32_t>{}, origin_backing_store};
 
          chain.create_accounts({"manager"_n});
          auto get_ext = [](unsigned i) {