Skip to content

Commit

Permalink
Fix usage of store del operations when iterating
Browse files Browse the repository at this point in the history
  • Loading branch information
pwojcikdev committed Dec 22, 2024
1 parent 50dd9ac commit e79f4a8
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 9 deletions.
26 changes: 19 additions & 7 deletions nano/node/online_reps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -141,21 +141,28 @@ void nano::online_reps::trim_trended (nano::store::write_transaction const & tra
auto const now = std::chrono::system_clock::now ();
auto const cutoff = now - config.network_params.node.weight_cutoff;

std::deque<nano::store::online_weight::iterator::value_type> to_remove;

for (auto it = ledger.store.online_weight.begin (transaction); it != ledger.store.online_weight.end (transaction); ++it)
{
auto tstamp = nano::from_seconds_since_epoch (it->first);
if (tstamp < cutoff)
{
stats.inc (nano::stat::type::online_reps, nano::stat::detail::trim_trend);
ledger.store.online_weight.del (transaction, it->first);
to_remove.push_back (*it);
}
else
{
// Entries are ordered by timestamp, so break early
break;
break; // Entries are ordered by timestamp, so break early
}
}

// Remove entries after iterating to avoid iterator invalidation
for (auto const & entry : to_remove)
{
ledger.store.online_weight.del (transaction, entry.first);
}

// Ensure that all remaining entries are within the expected range
debug_assert (verify_consistency (transaction, now, cutoff));
}
Expand All @@ -166,26 +173,31 @@ void nano::online_reps::sanitize_trended (nano::store::write_transaction const &
auto const cutoff = now - config.network_params.node.weight_cutoff;

size_t removed_old = 0, removed_future = 0;
std::deque<nano::store::online_weight::iterator::value_type> to_remove;

for (auto it = ledger.store.online_weight.begin (transaction); it != ledger.store.online_weight.end (transaction); ++it)
{
auto tstamp = nano::from_seconds_since_epoch (it->first);
if (tstamp < cutoff)
{
stats.inc (nano::stat::type::online_reps, nano::stat::detail::sanitize_old);
// TODO: Ensure it's OK to delete entry with the same key as the current iterator
ledger.store.online_weight.del (transaction, it->first);
to_remove.push_back (*it);
++removed_old;
}
else if (tstamp > now)
{
stats.inc (nano::stat::type::online_reps, nano::stat::detail::sanitize_future);
// TODO: Ensure it's OK to delete entry with the same key as the current iterator
ledger.store.online_weight.del (transaction, it->first);
to_remove.push_back (*it);
++removed_future;
}
}

// Remove entries after iterating to avoid iterator invalidation
for (auto const & entry : to_remove)
{
ledger.store.online_weight.del (transaction, entry.first);
}

logger.debug (nano::log::type::online_reps, "Sanitized online weight trend, remaining entries: {}, removed: {} (old: {}, future: {})",
ledger.store.online_weight.count (transaction),
removed_old + removed_future,
Expand Down
11 changes: 9 additions & 2 deletions nano/node/peer_history.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,21 +104,28 @@ void nano::peer_history::run_one ()
auto const now = std::chrono::system_clock::now ();
auto const cutoff = now - config.erase_cutoff;

std::deque<nano::store::peer::iterator::value_type> to_remove;

for (auto it = store.peer.begin (transaction); it != store.peer.end (transaction); ++it)
{
auto const [endpoint, timestamp_millis] = *it;
auto timestamp = nano::from_milliseconds_since_epoch (timestamp_millis);
if (timestamp > now || timestamp < cutoff)
{
// TODO: Ensure it's OK to delete entry with the same key as the current iterator
store.peer.del (transaction, endpoint);
to_remove.push_back (*it);

stats.inc (nano::stat::type::peer_history, nano::stat::detail::erased);
logger.debug (nano::log::type::peer_history, "Erased peer: {} (not seen for {}s)",
fmt::streamed (endpoint.endpoint ()),
nano::log::seconds_delta (timestamp));
}
}

// Remove entries after iterating to avoid iterator invalidation
for (auto const & entry : to_remove)
{
store.peer.del (transaction, entry.first);
}
}

std::vector<nano::endpoint> nano::peer_history::peers () const
Expand Down

0 comments on commit e79f4a8

Please sign in to comment.