Skip to content

Commit

Permalink
Don't initialize conf height table after a new account is opened
Browse files Browse the repository at this point in the history
  • Loading branch information
wezrule committed Aug 15, 2020
1 parent d656e87 commit f630d40
Show file tree
Hide file tree
Showing 11 changed files with 65 additions and 78 deletions.
10 changes: 5 additions & 5 deletions nano/core_test/confirmation_height.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,19 +129,19 @@ TEST (confirmation_height, multiple_accounts)
ASSERT_EQ (nano::process_result::progress, node->ledger.process (transaction, send6).code);
ASSERT_EQ (nano::process_result::progress, node->ledger.process (transaction, receive2).code);

// Check confirmation heights of all the accounts are uninitialized (0),
// Check confirmation heights of all the accounts (except genesis) are uninitialized (0),
// as we have any just added them to the ledger and not processed any live transactions yet.
nano::confirmation_height_info confirmation_height_info;
ASSERT_FALSE (node->store.confirmation_height_get (transaction, nano::dev_genesis_key.pub, confirmation_height_info));
ASSERT_EQ (1, confirmation_height_info.height);
ASSERT_EQ (nano::genesis_hash, confirmation_height_info.frontier);
ASSERT_FALSE (node->store.confirmation_height_get (transaction, key1.pub, confirmation_height_info));
ASSERT_TRUE (node->store.confirmation_height_get (transaction, key1.pub, confirmation_height_info));
ASSERT_EQ (0, confirmation_height_info.height);
ASSERT_EQ (nano::block_hash (0), confirmation_height_info.frontier);
ASSERT_FALSE (node->store.confirmation_height_get (transaction, key2.pub, confirmation_height_info));
ASSERT_TRUE (node->store.confirmation_height_get (transaction, key2.pub, confirmation_height_info));
ASSERT_EQ (0, confirmation_height_info.height);
ASSERT_EQ (nano::block_hash (0), confirmation_height_info.frontier);
ASSERT_FALSE (node->store.confirmation_height_get (transaction, key3.pub, confirmation_height_info));
ASSERT_TRUE (node->store.confirmation_height_get (transaction, key3.pub, confirmation_height_info));
ASSERT_EQ (0, confirmation_height_info.height);
ASSERT_EQ (nano::block_hash (0), confirmation_height_info.frontier);
}
Expand Down Expand Up @@ -281,7 +281,7 @@ TEST (confirmation_height, gap_bootstrap)
ASSERT_FALSE (node1.store.confirmation_height_get (transaction, nano::dev_genesis_key.pub, confirmation_height_info));
ASSERT_EQ (1, confirmation_height_info.height);
ASSERT_EQ (genesis.hash (), confirmation_height_info.frontier);
ASSERT_FALSE (node1.store.confirmation_height_get (transaction, destination.pub, confirmation_height_info));
ASSERT_TRUE (node1.store.confirmation_height_get (transaction, destination.pub, confirmation_height_info));
ASSERT_EQ (0, confirmation_height_info.height);
ASSERT_EQ (nano::block_hash (0), confirmation_height_info.frontier);
}
Expand Down
4 changes: 2 additions & 2 deletions nano/core_test/ledger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2990,7 +2990,7 @@ TEST (ledger, confirmation_height_not_updated)
ASSERT_EQ (genesis.hash (), confirmation_height_info.frontier);
nano::open_block open1 (send1.hash (), nano::genesis_account, key.pub, key.prv, key.pub, *pool.generate (key.pub));
ASSERT_EQ (nano::process_result::progress, ledger.process (transaction, open1).code);
ASSERT_FALSE (store->confirmation_height_get (transaction, key.pub, confirmation_height_info));
ASSERT_TRUE (store->confirmation_height_get (transaction, key.pub, confirmation_height_info));
ASSERT_EQ (0, confirmation_height_info.height);
ASSERT_EQ (nano::block_hash (0), confirmation_height_info.frontier);
}
Expand Down Expand Up @@ -3235,7 +3235,7 @@ TEST (ledger, can_vote)
.build_shared ();
ASSERT_EQ (nano::process_result::progress, ledger.process (transaction, *receive2).code);
ASSERT_FALSE (ledger.can_vote (transaction, *receive2));
ASSERT_FALSE (ledger.store.confirmation_height_get (transaction, key1.pub, height));
ASSERT_TRUE (ledger.store.confirmation_height_get (transaction, key1.pub, height));
height.height += 1;
ledger.store.confirmation_height_put (transaction, key1.pub, height);
ASSERT_FALSE (ledger.can_vote (transaction, *receive2));
Expand Down
3 changes: 0 additions & 3 deletions nano/lib/config.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,6 @@ std::string get_rpc_toml_config_path (boost::filesystem::path const & data_path)
std::string get_access_toml_config_path (boost::filesystem::path const & data_path);
std::string get_qtwallet_toml_config_path (boost::filesystem::path const & data_path);

/** Called by gtest_main to enforce dev network */
void force_nano_dev_network ();

/** Checks if we are running inside a valgrind instance */
bool running_within_valgrind ();

Expand Down
53 changes: 24 additions & 29 deletions nano/node/active_transactions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,7 @@ void nano::active_transactions::confirm_prioritized_frontiers (nano::transaction
if (!this->confirmation_height_processor.is_processing_block (info.head))
{
nano::confirmation_height_info confirmation_height_info;
error = this->node.store.confirmation_height_get (transaction_a, cementable_account.account, confirmation_height_info);
release_assert (!error);
this->node.store.confirmation_height_get (transaction_a, cementable_account.account, confirmation_height_info);

if (info.block_count > confirmation_height_info.height)
{
Expand Down Expand Up @@ -346,29 +345,27 @@ void nano::active_transactions::activate_dependencies (nano::unique_lock<std::mu
/* Insert first unconfirmed block (pessimistic) and bisect the chain (likelihood) */
auto const account (node.store.block_account_calculated (*block_l));
nano::confirmation_height_info conf_info_l;
if (!node.store.confirmation_height_get (transaction, account, conf_info_l))
node.store.confirmation_height_get (transaction, account, conf_info_l);
if (height_l > conf_info_l.height + 1)
{
if (height_l > conf_info_l.height + 1)
auto const successor_hash_l = first_unconfirmed (transaction, account, conf_info_l.frontier);
if (!confirmation_height_processor.is_processing_block (successor_hash_l))
{
auto const successor_hash_l = first_unconfirmed (transaction, account, conf_info_l.frontier);
if (!confirmation_height_processor.is_processing_block (successor_hash_l))
auto const successor_l = node.store.block_get (transaction, successor_hash_l);
debug_assert (successor_l != nullptr);
if (successor_l != nullptr)
{
auto const successor_l = node.store.block_get (transaction, successor_hash_l);
debug_assert (successor_l != nullptr);
if (successor_l != nullptr)
{
activate_l.emplace_back (successor_l, hash_l);
}
activate_l.emplace_back (successor_l, hash_l);
}
}
if (height_l > conf_info_l.height + 2)
}
if (height_l > conf_info_l.height + 2)
{
auto const jumps_l = std::min<uint64_t> (128, (height_l - conf_info_l.height) / 2);
auto const backtracked_l (node.ledger.backtrack (transaction, block_l, jumps_l));
if (backtracked_l != nullptr)
{
auto const jumps_l = std::min<uint64_t> (128, (height_l - conf_info_l.height) / 2);
auto const backtracked_l (node.ledger.backtrack (transaction, block_l, jumps_l));
if (backtracked_l != nullptr)
{
activate_l.emplace_back (backtracked_l, hash_l);
}
activate_l.emplace_back (backtracked_l, hash_l);
}
}
}
Expand Down Expand Up @@ -516,12 +513,13 @@ void nano::active_transactions::prioritize_frontiers_for_confirmation (nano::tra

auto i (wallet->store.begin (wallet_transaction, next_wallet_frontier_account));
auto n (wallet->store.end ());
nano::confirmation_height_info confirmation_height_info;
for (; i != n; ++i)
{
auto const & account (i->first);
if (!node.store.account_get (transaction_a, account, info) && !node.store.confirmation_height_get (transaction_a, account, confirmation_height_info))
if (!node.store.account_get (transaction_a, account, info))
{
nano::confirmation_height_info confirmation_height_info;
node.store.confirmation_height_get (transaction_a, account, confirmation_height_info);
// If it exists in normal priority collection delete from there.
auto it = priority_cementable_frontiers.find (account);
if (it != priority_cementable_frontiers.end ())
Expand Down Expand Up @@ -562,17 +560,15 @@ void nano::active_transactions::prioritize_frontiers_for_confirmation (nano::tra

auto i (node.store.latest_begin (transaction_a, next_frontier_account));
auto n (node.store.latest_end ());
nano::confirmation_height_info confirmation_height_info;
for (; i != n && !stopped; ++i)
{
auto const & account (i->first);
auto const & info (i->second);
if (priority_wallet_cementable_frontiers.find (account) == priority_wallet_cementable_frontiers.end ())
{
if (!node.store.confirmation_height_get (transaction_a, account, confirmation_height_info))
{
prioritize_account_for_confirmation (priority_cementable_frontiers, priority_cementable_frontiers_size, account, info, confirmation_height_info.height);
}
nano::confirmation_height_info confirmation_height_info;
node.store.confirmation_height_get (transaction_a, account, confirmation_height_info);
prioritize_account_for_confirmation (priority_cementable_frontiers, priority_cementable_frontiers_size, account, info, confirmation_height_info.height);
}
next_frontier_account = account.number () + 1;
if (timer.since_start () >= ledger_account_traversal_max_time_a)
Expand Down Expand Up @@ -780,9 +776,8 @@ nano::election_insertion_result nano::active_transactions::activate (nano::accou
if (!node.store.account_get (transaction, account_a, account_info))
{
nano::confirmation_height_info conf_info;
auto error = node.store.confirmation_height_get (transaction, account_a, conf_info);
debug_assert (!error);
if (!error && conf_info.height < account_info.block_count)
node.store.confirmation_height_get (transaction, account_a, conf_info);
if (conf_info.height < account_info.block_count)
{
debug_assert (conf_info.frontier != account_info.head);
auto hash = conf_info.height == 0 ? account_info.open_block : node.store.block_successor (transaction, conf_info.frontier);
Expand Down
2 changes: 1 addition & 1 deletion nano/node/blockprocessor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ void nano::block_processor::process_batch (nano::unique_lock<std::mutex> & lock_
{
auto scoped_write_guard = write_database_queue.wait (nano::writer::process_batch);
block_post_events post_events;
auto transaction (node.store.tx_begin_write ({ tables::accounts, tables::blocks, tables::frontiers, tables::pending, tables::unchecked }, { tables::confirmation_height }));
auto transaction (node.store.tx_begin_write ({ tables::accounts, tables::blocks, tables::frontiers, tables::pending, tables::unchecked }));
nano::timer<std::chrono::milliseconds> timer_l;
lock_a.lock ();
timer_l.start ();
Expand Down
16 changes: 4 additions & 12 deletions nano/node/confirmation_height_bounded.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,7 @@ void nano::confirmation_height_bounded::process ()
}
else
{
auto error = ledger.store.confirmation_height_get (transaction, account, confirmation_height_info);
(void)error;
debug_assert (!error);
ledger.store.confirmation_height_get (transaction, account, confirmation_height_info);
// This block was added to the confirmation height processor but is already confirmed
if (first_iter && confirmation_height_info.height >= block->sideband ().height && current == original_hash)
{
Expand Down Expand Up @@ -365,7 +363,7 @@ void nano::confirmation_height_bounded::cement_blocks (nano::write_guard & scope
#ifndef NDEBUG
// Extra debug checks
nano::confirmation_height_info confirmation_height_info;
debug_assert (!ledger.store.confirmation_height_get (transaction, account, confirmation_height_info));
ledger.store.confirmation_height_get (transaction, account, confirmation_height_info);
auto block (ledger.store.block_get (transaction, confirmed_frontier));
debug_assert (block != nullptr);
debug_assert (block->sideband ().height == confirmation_height_info.height + num_blocks_cemented);
Expand All @@ -377,16 +375,10 @@ void nano::confirmation_height_bounded::cement_blocks (nano::write_guard & scope
};

nano::confirmation_height_info confirmation_height_info;
error = ledger.store.confirmation_height_get (transaction, pending.account, confirmation_height_info);
if (error)
{
auto error_str = (boost::format ("Failed to read confirmation height for account %1% (bounded processor)") % pending.account.to_account ()).str ();
logger.always_log (error_str);
std::cerr << error_str << std::endl;
}
ledger.store.confirmation_height_get (transaction, pending.account, confirmation_height_info);

// Some blocks need to be cemented at least
if (!error && pending.top_height > confirmation_height_info.height)
if (pending.top_height > confirmation_height_info.height)
{
// The highest hash which will be cemented
nano::block_hash new_cemented_frontier;
Expand Down
12 changes: 3 additions & 9 deletions nano/node/confirmation_height_unbounded.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ void nano::confirmation_height_unbounded::process ()
else
{
nano::confirmation_height_info confirmation_height_info;
release_assert (!ledger.store.confirmation_height_get (read_transaction, account, confirmation_height_info));
ledger.store.confirmation_height_get (read_transaction, account, confirmation_height_info);
confirmation_height = confirmation_height_info.height;

// This block was added to the confirmation height processor but is already confirmed
Expand Down Expand Up @@ -347,15 +347,9 @@ void nano::confirmation_height_unbounded::cement_blocks (nano::write_guard & sco
{
auto & pending = pending_writes.front ();
nano::confirmation_height_info confirmation_height_info;
error = ledger.store.confirmation_height_get (transaction, pending.account, confirmation_height_info);
if (error)
{
auto error_str = (boost::format ("Failed to read confirmation height for account %1% when writing block %2% (unbounded processor)") % pending.account.to_account () % pending.hash.to_string ()).str ();
logger.always_log (error_str);
std::cerr << error_str << std::endl;
}
ledger.store.confirmation_height_get (transaction, pending.account, confirmation_height_info);
auto confirmation_height = confirmation_height_info.height;
if (!error && pending.height > confirmation_height)
if (pending.height > confirmation_height)
{
auto block = ledger.store.block_get (transaction, pending.hash);
debug_assert (network_params.network.is_dev_network () || block != nullptr);
Expand Down
5 changes: 1 addition & 4 deletions nano/node/json_handler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -596,10 +596,7 @@ void nano::json_handler::account_info ()
auto transaction (node.store.tx_begin_read ());
auto info (account_info_impl (transaction, account));
nano::confirmation_height_info confirmation_height_info;
if (node.store.confirmation_height_get (transaction, account, confirmation_height_info))
{
ec = nano::error_common::account_not_found;
}
node.store.confirmation_height_get (transaction, account, confirmation_height_info);
if (!ec)
{
response_l.put ("frontier", info.head.to_string ());
Expand Down
16 changes: 15 additions & 1 deletion nano/node/rocksdb/rocksdb.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,8 @@ uint64_t nano::rocksdb_store::count (nano::transaction const & transaction_a, ta
{
db->GetIntProperty (table_to_column_family (table_a), "rocksdb.estimate-num-keys", &sum);
}
// These should only be used in tests to check database consistency
// Accounts and blocks should only be used in tests and CLI commands to check database consistency
// otherwise there can be performance issues.
else if (table_a == tables::accounts)
{
debug_assert (network_constants ().is_dev_network ());
Expand All @@ -302,6 +303,7 @@ uint64_t nano::rocksdb_store::count (nano::transaction const & transaction_a, ta
}
else if (table_a == tables::blocks)
{
// This is also used in some CLI commands
for (auto i (blocks_begin (transaction_a)), n (blocks_end ()); i != n; ++i)
{
++sum;
Expand Down Expand Up @@ -385,6 +387,15 @@ rocksdb::Options nano::rocksdb_store::get_db_options () const
db_options.IncreaseParallelism (rocksdb_config.io_threads);
db_options.OptimizeLevelStyleCompaction ();

// The maximum number of threads that will concurrently perform a compaction job by breaking it into multiple,
// smaller ones that are run simultaneously. Can help L0 to L1 compaction
db_options.max_subcompactions = std::min (rocksdb_config.io_threads, 2u);

// Allows parallel writers to the memtables. We do not currently have any, and
// if enabled can only be used with the default skip list factory, and is not compatible
// or inplace updates.
db_options.allow_concurrent_memtable_write = false;

// Adds a separate write queue for memtable/WAL
db_options.enable_pipelined_write = true;

Expand Down Expand Up @@ -416,6 +427,9 @@ rocksdb::ColumnFamilyOptions nano::rocksdb_store::get_cf_options () const
rocksdb::ColumnFamilyOptions cf_options;
cf_options.table_factory = table_factory;

// Search for keys by hash instead of binary search
cf_options.memtable_factory.reset (rocksdb::NewHashSkipListRepFactory ());

// Number of files in level which triggers compaction. Size of L0 and L1 should be kept similar as this is the only compaction which is single threaded
cf_options.level0_file_num_compaction_trigger = 4;

Expand Down
8 changes: 6 additions & 2 deletions nano/secure/blockstore_partial.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ class block_store_partial : public block_store
uint64_t block_account_height (nano::transaction const & transaction_a, nano::block_hash const & hash_a) const override
{
auto block = block_get (transaction_a, hash_a);
debug_assert (block != nullptr);
return block->sideband ().height;
}

Expand Down Expand Up @@ -509,7 +508,6 @@ class block_store_partial : public block_store
void account_put (nano::write_transaction const & transaction_a, nano::account const & account_a, nano::account_info const & info_a) override
{
// Check we are still in sync with other tables
debug_assert (confirmation_height_exists (transaction_a, account_a));
nano::db_val<Val> info (info_a);
auto status = put (transaction_a, tables::accounts, account_a, info);
release_assert (success (status));
Expand Down Expand Up @@ -634,6 +632,12 @@ class block_store_partial : public block_store
nano::bufferstream stream (reinterpret_cast<uint8_t const *> (value.data ()), value.size ());
result = confirmation_height_info_a.deserialize (stream);
}
if (result)
{
confirmation_height_info_a.height = 0;
confirmation_height_info_a.frontier = 0;
}

return result;
}

Expand Down
Loading

0 comments on commit f630d40

Please sign in to comment.