Skip to content

Commit

Permalink
Converting functions on unchecked_map that return iterators to instea…
Browse files Browse the repository at this point in the history
…d use for_each with a functor to execute on each result. This prevents iteration details from being exposed externally.
  • Loading branch information
clemahieu committed Jun 8, 2022
1 parent 2fd16c7 commit b4ce6c4
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 87 deletions.
55 changes: 27 additions & 28 deletions nano/core_test/block_store.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -482,9 +482,11 @@ TEST (block_store, empty_bootstrap)
auto store = nano::make_store (logger, nano::unique_path (), nano::dev::constants);
ASSERT_TRUE (!store->init_error ());
auto transaction (store->tx_begin_read ());
auto begin (store->unchecked.begin (transaction));
auto end (store->unchecked.end ());
ASSERT_EQ (end, begin);
bool has_item = false;
store->unchecked.for_each (transaction, [&has_item] (nano::unchecked_key const & key, nano::unchecked_info const & info) {
has_item = true;
});
ASSERT_FALSE (has_item);
}

TEST (block_store, one_bootstrap)
Expand All @@ -495,17 +497,16 @@ TEST (block_store, one_bootstrap)
auto block1 (std::make_shared<nano::send_block> (0, 1, 2, nano::keypair ().prv, 4, 5));
auto transaction (store->tx_begin_write ());
store->unchecked.put (transaction, block1->hash (), block1);
auto begin (store->unchecked.begin (transaction));
auto end (store->unchecked.end ());
ASSERT_NE (end, begin);
auto hash1 (begin->first.key ());
ASSERT_EQ (block1->hash (), hash1);
auto blocks (store->unchecked.get (transaction, hash1));
std::vector<nano::hash_or_account> dependencies;
store->unchecked.for_each (transaction, [&dependencies] (nano::unchecked_key const & key, nano::unchecked_info const & info) {
dependencies.push_back (key.key ());
});
std::vector<nano::hash_or_account> expected{ block1->hash () };
ASSERT_EQ (dependencies, expected);
auto blocks (store->unchecked.get (transaction, block1->hash ()));
ASSERT_EQ (1, blocks.size ());
auto block2 (blocks[0].block);
ASSERT_EQ (*block1, *block2);
++begin;
ASSERT_EQ (end, begin);
}

TEST (block_store, unchecked_begin_search)
Expand Down Expand Up @@ -930,32 +931,30 @@ TEST (block_store, DISABLED_change_dupsort) // Unchecked is no longer dupsort ta
ASSERT_NE (send1->hash (), send2->hash ());
store.unchecked.put (transaction, send1->hash (), send1);
store.unchecked.put (transaction, send1->hash (), send2);
{
auto iterator1 (store.unchecked.begin (transaction));
++iterator1;
ASSERT_EQ (store.unchecked.end (), iterator1);
}
std::vector<nano::hash_or_account> dependencies1;
store.unchecked.for_each (transaction, [&dependencies1] (nano::unchecked_key const & key, nano::unchecked_info const & info) {
dependencies1.push_back (key.key ());
});
ASSERT_TRUE (dependencies1.empty ());
ASSERT_EQ (0, mdb_drop (store.env.tx (transaction), store.unchecked_handle, 0));
mdb_dbi_close (store.env, store.unchecked_handle);
ASSERT_EQ (0, mdb_dbi_open (store.env.tx (transaction), "unchecked", MDB_CREATE | MDB_DUPSORT, &store.unchecked_handle));
store.unchecked.put (transaction, send1->hash (), send1);
store.unchecked.put (transaction, send1->hash (), send2);
{
auto iterator1 (store.unchecked.begin (transaction));
++iterator1;
ASSERT_EQ (store.unchecked.end (), iterator1);
}
std::vector<nano::hash_or_account> dependencies2;
store.unchecked.for_each (transaction, [&dependencies2] (nano::unchecked_key const & key, nano::unchecked_info const & info) {
dependencies2.push_back (key.key ());
});
ASSERT_TRUE (dependencies2.empty ());
ASSERT_EQ (0, mdb_drop (store.env.tx (transaction), store.unchecked_handle, 1));
ASSERT_EQ (0, mdb_dbi_open (store.env.tx (transaction), "unchecked", MDB_CREATE | MDB_DUPSORT, &store.unchecked_handle));
store.unchecked.put (transaction, send1->hash (), send1);
store.unchecked.put (transaction, send1->hash (), send2);
{
auto iterator1 (store.unchecked.begin (transaction));
++iterator1;
ASSERT_NE (store.unchecked.end (), iterator1);
++iterator1;
ASSERT_EQ (store.unchecked.end (), iterator1);
}
std::vector<nano::hash_or_account> dependencies3;
store.unchecked.for_each (transaction, [&dependencies3] (nano::unchecked_key const & key, nano::unchecked_info const & info) {
dependencies3.push_back (key.key ());
});
ASSERT_EQ (1, dependencies3.size ());
}

TEST (block_store, state_block)
Expand Down
7 changes: 3 additions & 4 deletions nano/nano_node/entry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -437,14 +437,13 @@ int main (int argc, char * const * argv)
}

// Check all unchecked keys for matching frontier hashes. Indicates an issue with process_batch algorithm
for (auto i (node->store.unchecked.begin (transaction)), n (node->store.unchecked.end ()); i != n; ++i)
{
auto it = frontier_hashes.find (i->first.key ());
node->store.unchecked.for_each (transaction, [&frontier_hashes] (nano::unchecked_key const & key, nano::unchecked_info const & info) {
auto it = frontier_hashes.find (key.key ());
if (it != frontier_hashes.cend ())
{
std::cout << it->to_string () << "\n";
}
}
});
}
else if (vm.count ("debug_account_count"))
{
Expand Down
52 changes: 20 additions & 32 deletions nano/node/json_handler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4026,9 +4026,8 @@ void nano::json_handler::unchecked ()
{
boost::property_tree::ptree unchecked;
auto transaction (node.store.tx_begin_read ());
for (auto i (node.store.unchecked.begin (transaction)), n (node.store.unchecked.end ()); i != n && unchecked.size () < count; ++i)
{
nano::unchecked_info const & info (i->second);
node.store.unchecked.for_each (
transaction, [&unchecked, &json_block_l] (nano::unchecked_key const & key, nano::unchecked_info const & info) {
if (json_block_l)
{
boost::property_tree::ptree block_node_l;
Expand All @@ -4040,8 +4039,7 @@ void nano::json_handler::unchecked ()
std::string contents;
info.block->serialize_json (contents);
unchecked.put (info.block->hash ().to_string (), contents);
}
}
} }, [iterations = 0, count = count] () mutable { return iterations++ < count; });
response_l.add_child ("blocks", unchecked);
}
response_errors ();
Expand All @@ -4064,29 +4062,21 @@ void nano::json_handler::unchecked_get ()
if (!ec)
{
auto transaction (node.store.tx_begin_read ());
for (auto i (node.store.unchecked.begin (transaction)), n (node.store.unchecked.end ()); i != n; ++i)
{
nano::unchecked_key const & key (i->first);
if (key.hash == hash)
node.store.unchecked.for_each (
transaction, hash, [this, &hash, &json_block_l] (nano::unchecked_key const & key, nano::unchecked_info const & info) {
response_l.put ("modified_timestamp", std::to_string (info.modified));
if (json_block_l)
{
nano::unchecked_info const & info (i->second);
response_l.put ("modified_timestamp", std::to_string (info.modified));

if (json_block_l)
{
boost::property_tree::ptree block_node_l;
info.block->serialize_json (block_node_l);
response_l.add_child ("contents", block_node_l);
}
else
{
std::string contents;
info.block->serialize_json (contents);
response_l.put ("contents", contents);
}
break;
boost::property_tree::ptree block_node_l;
info.block->serialize_json (block_node_l);
response_l.add_child ("contents", block_node_l);
}
}
else
{
std::string contents;
info.block->serialize_json (contents);
response_l.put ("contents", contents);
} }, [iterations = 0] () mutable { return iterations++ < 1; });
if (response_l.empty ())
{
ec = nano::error_blocks::not_found;
Expand All @@ -4112,11 +4102,10 @@ void nano::json_handler::unchecked_keys ()
{
boost::property_tree::ptree unchecked;
auto transaction (node.store.tx_begin_read ());
for (auto i (node.store.unchecked.begin (transaction, nano::unchecked_key (key, 0))), n (node.store.unchecked.end ()); i != n && unchecked.size () < count; ++i)
{
node.store.unchecked.for_each (
transaction, key, [&unchecked, json_block_l] (nano::unchecked_key const & key, nano::unchecked_info const & info) {
boost::property_tree::ptree entry;
nano::unchecked_info const & info (i->second);
entry.put ("key", i->first.key ().to_string ());
entry.put ("key", key.key ().to_string ());
entry.put ("hash", info.block->hash ().to_string ());
entry.put ("modified_timestamp", std::to_string (info.modified));
if (json_block_l)
Expand All @@ -4131,8 +4120,7 @@ void nano::json_handler::unchecked_keys ()
info.block->serialize_json (contents);
entry.put ("contents", contents);
}
unchecked.push_back (std::make_pair ("", entry));
}
unchecked.push_back (std::make_pair ("", entry)); }, [&unchecked, &count] () { return unchecked.size () < count; });
response_l.add_child ("unchecked", unchecked);
}
response_errors ();
Expand Down
8 changes: 3 additions & 5 deletions nano/node/lmdb/lmdb.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -814,11 +814,9 @@ void nano::mdb_store::create_backup_file (nano::mdb_env & env_a, boost::filesyst
std::vector<nano::unchecked_info> nano::unchecked_mdb_store::get (nano::transaction const & transaction_a, nano::block_hash const & hash_a)
{
std::vector<nano::unchecked_info> result;
for (auto i (begin (transaction_a, nano::unchecked_key (hash_a, 0))), n (end ()); i != n && i->first.key () == hash_a; ++i)
{
nano::unchecked_info const & unchecked_info (i->second);
result.push_back (unchecked_info);
}
for_each (transaction_a, hash_a, [&result] (nano::unchecked_key const & key, nano::unchecked_info const & info) {
result.push_back (info);
});
return result;
}

Expand Down
9 changes: 3 additions & 6 deletions nano/node/node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -947,16 +947,13 @@ void nano::node::unchecked_cleanup ()
auto const now (nano::seconds_since_epoch ());
auto const transaction (store.tx_begin_read ());
// Max 1M records to clean, max 2 minutes reading to prevent slow i/o systems issues
for (auto i (store.unchecked.begin (transaction)), n (store.unchecked.end ()); i != n && cleaning_list.size () < 1024 * 1024 && nano::seconds_since_epoch () - now < 120; ++i)
{
nano::unchecked_key const & key (i->first);
nano::unchecked_info const & info (i->second);
store.unchecked.for_each (
transaction, [this, &digests, &cleaning_list, &now] (nano::unchecked_key const & key, nano::unchecked_info const & info) {
if ((now - info.modified) > static_cast<uint64_t> (config.unchecked_cutoff_time.count ()))
{
digests.push_back (network.publish_filter.hash (info.block));
cleaning_list.push_back (key);
}
}
} }, [iterations = 0, count = 1024 * 1024] () mutable { return iterations++ < count; });
}
if (!cleaning_list.empty ())
{
Expand Down
10 changes: 7 additions & 3 deletions nano/secure/store.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -743,15 +743,19 @@ class confirmation_height_store
class unchecked_store
{
public:
virtual ~unchecked_store () = default;
virtual void clear (nano::write_transaction const &) = 0;
virtual void put (nano::write_transaction const &, nano::unchecked_key const &, nano::unchecked_info const &) = 0;
virtual void put (nano::write_transaction const &, nano::block_hash const &, std::shared_ptr<nano::block> const &) = 0;
virtual std::vector<nano::unchecked_info> get (nano::transaction const &, nano::block_hash const &) = 0;
virtual bool exists (nano::transaction const & transaction_a, nano::unchecked_key const & unchecked_key_a) = 0;
virtual void del (nano::write_transaction const &, nano::unchecked_key const &) = 0;
virtual nano::store_iterator<nano::unchecked_key, nano::unchecked_info> begin (nano::transaction const &) const = 0;
virtual nano::store_iterator<nano::unchecked_key, nano::unchecked_info> begin (nano::transaction const &, nano::unchecked_key const &) const = 0;
virtual nano::store_iterator<nano::unchecked_key, nano::unchecked_info> end () const = 0;
virtual void for_each (
nano::transaction const & tx, std::function<void (nano::unchecked_key const &, nano::unchecked_info const &)> action, std::function<bool ()> predicate = [] () { return true; })
= 0;
virtual void for_each (
nano::transaction const & tx, nano::hash_or_account const & dependency, std::function<void (nano::unchecked_key const &, nano::unchecked_info const &)> action, std::function<bool ()> predicate = [] () { return true; })
= 0;
virtual size_t count (nano::transaction const &) = 0;
};

Expand Down
21 changes: 12 additions & 9 deletions nano/secure/store/unchecked_store_partial.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,19 +64,22 @@ class unchecked_store_partial : public unchecked_store
release_assert_success (store, status);
}

nano::store_iterator<nano::unchecked_key, nano::unchecked_info> end () const override
void for_each (
nano::transaction const & tx, std::function<void (nano::unchecked_key const &, nano::unchecked_info const &)> action, std::function<bool ()> predicate = [] () { return true; }) override
{
return nano::store_iterator<nano::unchecked_key, nano::unchecked_info> (nullptr);
for (auto i = store.template make_iterator<nano::unchecked_key, nano::unchecked_info> (tx, tables::unchecked), n = nano::store_iterator<nano::unchecked_key, nano::unchecked_info> (nullptr); predicate () && i != n; ++i)
{
action (i->first, i->second);
}
}

nano::store_iterator<nano::unchecked_key, nano::unchecked_info> begin (nano::transaction const & transaction_a) const override
void for_each (
nano::transaction const & tx, nano::hash_or_account const & dependency, std::function<void (nano::unchecked_key const &, nano::unchecked_info const &)> action, std::function<bool ()> predicate = [] () { return true; }) override
{
return store.template make_iterator<nano::unchecked_key, nano::unchecked_info> (transaction_a, tables::unchecked);
}

nano::store_iterator<nano::unchecked_key, nano::unchecked_info> begin (nano::transaction const & transaction_a, nano::unchecked_key const & key_a) const override
{
return store.template make_iterator<nano::unchecked_key, nano::unchecked_info> (transaction_a, tables::unchecked, nano::db_val<Val> (key_a));
for (auto i = store.template make_iterator<nano::unchecked_key, nano::unchecked_info> (tx, tables::unchecked, nano::unchecked_key{ dependency, 0 }), n = nano::store_iterator<nano::unchecked_key, nano::unchecked_info> (nullptr); predicate () && i->first.key () == dependency && i != n; ++i)
{
action (i->first, i->second);
}
}

size_t count (nano::transaction const & transaction_a) override
Expand Down

0 comments on commit b4ce6c4

Please sign in to comment.