Merge pull request #4131 from simpago/clear-unchecked-table

Add LMDB and RocksDB upgrades to v22: Remove unchecked table

nano/core_test/block_store.cpp

@@ -17,10 +17,10 @@
 
 #include <boost/filesystem.hpp>
 
+#include <cstdlib>
 #include <fstream>
 #include <unordered_set>
-
-#include <stdlib.h>
+#include <vector>
 
 using namespace std::chrono_literals;
 
@@ -2177,6 +2177,89 @@ namespace lmdb
 		auto transaction (store.tx_begin_read ());
 		ASSERT_LT (19, store.version.get (transaction));
 	}
+
+	TEST (mdb_block_store, upgrade_v21_v22)
+	{
+		if (nano::rocksdb_config::using_rocksdb_in_tests ())
+		{
+			// Don't test this in rocksdb mode
+			GTEST_SKIP ();
+		}
+
+		auto path (nano::unique_path ());
+		nano::logger_mt logger;
+		nano::stats stats;
+		auto const check_correct_state = [&] () {
+			nano::lmdb::store store (logger, path, nano::dev::constants);
+			auto transaction (store.tx_begin_write ());
+			ASSERT_EQ (store.version.get (transaction), store.version_current);
+			MDB_dbi unchecked_handle{ 0 };
+			ASSERT_EQ (MDB_NOTFOUND, mdb_dbi_open (store.env.tx (transaction), "unchecked", 0, &unchecked_handle));
+		};
+
+		// Testing current version doesn't contain the unchecked table
+		check_correct_state ();
+
+		// Setting the database to its 21st version state
+		{
+			nano::lmdb::store store (logger, path, nano::dev::constants);
+			auto transaction (store.tx_begin_write ());
+			store.version.put (transaction, 21);
+			MDB_dbi unchecked_handle{ 0 };
+			ASSERT_FALSE (mdb_dbi_open (store.env.tx (transaction), "unchecked", MDB_CREATE, &unchecked_handle));
+			ASSERT_EQ (store.version.get (transaction), 21);
+		}
+
+		// Testing the upgrade code worked
+		check_correct_state ();
+	}
+}
+
+namespace rocksdb
+{
+	TEST (rocksdb_block_store, upgrade_v21_v22)
+	{
+		if (!nano::rocksdb_config::using_rocksdb_in_tests ())
+		{
+			// Don't test this in LMDB mode
+			GTEST_SKIP ();
+		}
+
+		auto const path = nano::unique_path ();
+		nano::logger_mt logger;
+		nano::stats stats;
+		auto const check_correct_state = [&] () {
+			nano::rocksdb::store store (logger, path, nano::dev::constants);
+			auto transaction (store.tx_begin_write ());
+			ASSERT_EQ (store.version.get (transaction), store.version_current);
+			ASSERT_FALSE (store.column_family_exists ("unchecked"));
+		};
+
+		// Testing current version doesn't contain the unchecked table
+		check_correct_state ();
+
+		// Setting the database to its 21st version state
+		{
+			nano::rocksdb::store store (logger, path, nano::dev::constants);
+
+			// Create a column family for "unchecked"
+			::rocksdb::ColumnFamilyOptions new_cf_options;
+			::rocksdb::ColumnFamilyHandle * new_cf_handle;
+			::rocksdb::Status status = store.db->CreateColumnFamily (new_cf_options, "unchecked", &new_cf_handle);
+			store.handles.emplace_back (new_cf_handle);
+
+			// The new column family was created successfully, and 'new_cf_handle' now points to it.
+			ASSERT_TRUE (status.ok ());
+
+			// Rollback the database version number.
+			auto transaction (store.tx_begin_write ());
+			store.version.put (transaction, 21);
+			ASSERT_EQ (store.version.get (transaction), 21);
+		}
+
+		// Testing the upgrade code worked
+		check_correct_state ();
+	}
 }
 }
 

nano/core_test/ledger.cpp

@@ -5541,7 +5541,6 @@ TEST (ledger, migrate_lmdb_to_rocksdb)
 	boost::asio::ip::address_v6 address (boost::asio::ip::make_address_v6 ("::ffff:127.0.0.1"));
 	uint16_t port = 100;
 	nano::lmdb::store store{ logger, path / "data.ldb", nano::dev::constants };
-	nano::unchecked_map unchecked{ system.stats, false };
 	nano::ledger ledger{ store, system.stats, nano::dev::constants };
 	nano::work_pool pool{ nano::dev::network_params.network, std::numeric_limits<unsigned>::max () };
 
@@ -5556,7 +5555,7 @@ TEST (ledger, migrate_lmdb_to_rocksdb)
 										.build_shared ();
 
 	nano::endpoint_key endpoint_key (address.to_bytes (), port);
-	auto version = 99;
+	auto version = nano::store::version_current;
 
 	{
 		auto transaction = store.tx_begin_write ();
@@ -5582,7 +5581,6 @@ TEST (ledger, migrate_lmdb_to_rocksdb)
 	ASSERT_FALSE (error);
 
 	nano::rocksdb::store rocksdb_store{ logger, path / "rocksdb", nano::dev::constants };
-	nano::unchecked_map rocksdb_unchecked{ system.stats, false };
 	auto rocksdb_transaction (rocksdb_store.tx_begin_read ());
 
 	nano::pending_info pending_info{};

nano/node/CMakeLists.txt

@@ -136,7 +136,6 @@ add_library(
   lmdb/pruned_store.cpp
   lmdb/version_store.hpp
   lmdb/version_store.cpp
-  lmdb/unchecked_store.hpp
   lmdb/lmdb.hpp
   lmdb/lmdb.cpp
   lmdb/lmdb_env.hpp

nano/node/lmdb/lmdb.cpp

@@ -62,7 +62,6 @@ nano::lmdb::store::store (nano::logger_mt & logger_a, boost::filesystem::path co
 	peer_store{ *this },
 	confirmation_height_store{ *this },
 	final_vote_store{ *this },
-	unchecked_store{},
 	version_store{ *this },
 	logger (logger_a),
 	env (error, path_a, nano::mdb_env::options::make ().set_config (lmdb_config_a).set_use_no_mem_init (true)),
@@ -208,7 +207,6 @@ nano::mdb_txn_callbacks nano::lmdb::store::create_txn_callbacks () const
 void nano::lmdb::store::open_databases (bool & error_a, nano::transaction const & transaction_a, unsigned flags)
 {
 	error_a |= mdb_dbi_open (env.tx (transaction_a), "frontiers", flags, &frontier_store.frontiers_handle) != 0;
-	error_a |= mdb_dbi_open (env.tx (transaction_a), "unchecked", flags, &unchecked_store.unchecked_handle) != 0;
 	error_a |= mdb_dbi_open (env.tx (transaction_a), "online_weight", flags, &online_weight_store.online_weight_handle) != 0;
 	error_a |= mdb_dbi_open (env.tx (transaction_a), "meta", flags, &block_store.meta_handle) != 0;
 	error_a |= mdb_dbi_open (env.tx (transaction_a), "peers", flags, &peer_store.peers_handle) != 0;
@@ -303,6 +301,9 @@ bool nano::lmdb::store::do_upgrades (nano::write_transaction & transaction_a, na
 			upgrade_v20_to_v21 (transaction_a);
 			[[fallthrough]];
 		case 21:
+			upgrade_v21_to_v22 (transaction_a);
+			[[fallthrough]];
+		case 22:
 			break;
 		default:
 			logger.always_log (boost::str (boost::format ("The version of the ledger (%1%) is too high for this node") % version_l));
@@ -776,6 +777,16 @@ void nano::lmdb::store::upgrade_v20_to_v21 (nano::write_transaction const & tran
 	logger.always_log ("Finished creating new final_vote table");
 }
 
+void nano::lmdb::store::upgrade_v21_to_v22 (nano::write_transaction const & transaction_a)
+{
+	logger.always_log ("Preparing v21 to v22 database upgrade...");
+	MDB_dbi unchecked_handle{ 0 };
+	release_assert (!mdb_dbi_open (env.tx (transaction_a), "unchecked", MDB_CREATE, &unchecked_handle));
+	release_assert (!mdb_drop (env.tx (transaction_a), unchecked_handle, 1)); // del = 1, to delete it from the environment and close the DB handle.
+	version.put (transaction_a, 22);
+	logger.always_log ("Finished removing unchecked table");
+}
+
 /** Takes a filepath, appends '_backup_<timestamp>' to the end (but before any extension) and saves that file in the same directory */
 void nano::lmdb::store::create_backup_file (nano::mdb_env & env_a, boost::filesystem::path const & filepath_a, nano::logger_mt & logger_a)
 {
@@ -865,8 +876,6 @@ MDB_dbi nano::lmdb::store::table_to_dbi (tables table_a) const
 			return block_store.blocks_handle;
 		case tables::pending:
 			return pending_store.pending_handle;
-		case tables::unchecked:
-			return unchecked_store.unchecked_handle;
 		case tables::online_weight:
 			return online_weight_store.online_weight_handle;
 		case tables::meta:

nano/node/lmdb/lmdb.hpp

@@ -16,7 +16,6 @@
 #include <nano/node/lmdb/peer_store.hpp>
 #include <nano/node/lmdb/pending_store.hpp>
 #include <nano/node/lmdb/pruned_store.hpp>
-#include <nano/node/lmdb/unchecked_store.hpp>
 #include <nano/node/lmdb/version_store.hpp>
 #include <nano/secure/common.hpp>
 #include <nano/secure/versioning.hpp>
@@ -57,7 +56,6 @@ namespace lmdb
 		nano::lmdb::peer_store peer_store;
 		nano::lmdb::pending_store pending_store;
 		nano::lmdb::pruned_store pruned_store;
-		nano::lmdb::unchecked_store unchecked_store;
 		nano::lmdb::version_store version_store;
 
 		friend class nano::lmdb::account_store;
@@ -69,7 +67,6 @@ namespace lmdb
 		friend class nano::lmdb::peer_store;
 		friend class nano::lmdb::pending_store;
 		friend class nano::lmdb::pruned_store;
-		friend class nano::lmdb::unchecked_store;
 		friend class nano::lmdb::version_store;
 
 	public:
@@ -136,6 +133,7 @@ namespace lmdb
 		void upgrade_v18_to_v19 (nano::write_transaction const &);
 		void upgrade_v19_to_v20 (nano::write_transaction const &);
 		void upgrade_v20_to_v21 (nano::write_transaction const &);
+		void upgrade_v21_to_v22 (nano::write_transaction const &);
 
 		std::shared_ptr<nano::block> block_get_v18 (nano::transaction const & transaction_a, nano::block_hash const & hash_a) const;
 		nano::mdb_val block_raw_get_v18 (nano::transaction const & transaction_a, nano::block_hash const & hash_a, nano::block_type & type_a) const;
@@ -188,6 +186,7 @@ namespace lmdb
 		friend class mdb_block_store_upgrade_v18_v19_Test;
 		friend class mdb_block_store_upgrade_v19_v20_Test;
 		friend class mdb_block_store_upgrade_v20_v21_Test;
+		friend class mdb_block_store_upgrade_v21_v22_Test;
 		friend class block_store_DISABLED_change_dupsort_Test;
 		friend void write_sideband_v14 (nano::lmdb::store &, nano::transaction &, nano::block const &, MDB_dbi);
 		friend void write_sideband_v15 (nano::lmdb::store &, nano::transaction &, nano::block const &);

nano/node/node.cpp

@@ -441,7 +441,6 @@ nano::node::node (boost::asio::io_context & io_ctx_a, boost::filesystem::path co
 			// Drop unchecked blocks if initial bootstrap is completed
 			if (!flags.disable_unchecked_drop && !use_bootstrap_weight && !flags.read_only)
 			{
-				auto const transaction (store.tx_begin_write ({ tables::unchecked }));
 				unchecked.clear ();
 				logger.always_log ("Dropping unchecked blocks");
 			}
@@ -996,7 +995,6 @@ void nano::node::unchecked_cleanup ()
 	while (!cleaning_list.empty ())
 	{
 		std::size_t deleted_count (0);
-		auto const transaction (store.tx_begin_write ({ tables::unchecked }));
 		while (deleted_count++ < 2 * 1024 && !cleaning_list.empty ())
 		{
 			auto key (cleaning_list.front ());