Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tsan: Fix race in repcrawler and remove redundant weight computation #1881

Merged
merged 1 commit into from
Apr 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 7 additions & 11 deletions nano/node/repcrawler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,9 @@ void nano::rep_crawler::start ()
void nano::rep_crawler::ongoing_crawl ()
{
auto now (std::chrono::steady_clock::now ());
query (get_crawl_targets ());
auto sufficient_weight (total_weight_internal () > node.config.online_weight_minimum.number ());
auto total_weight_l (total_weight ());
query (get_crawl_targets (total_weight_l));
auto sufficient_weight (total_weight_l > node.config.online_weight_minimum.number ());
// If online weight drops below minimum, reach out to preconfigured peers
if (!sufficient_weight)
{
Expand All @@ -53,14 +54,14 @@ void nano::rep_crawler::ongoing_crawl ()
});
}

std::vector<std::shared_ptr<nano::transport::channel>> nano::rep_crawler::get_crawl_targets ()
std::vector<std::shared_ptr<nano::transport::channel>> nano::rep_crawler::get_crawl_targets (nano::uint128_t total_weight_a)
{
std::unordered_set<std::shared_ptr<nano::transport::channel>> channels;
constexpr size_t conservative_count = 10;
constexpr size_t aggressive_count = 40;

// Crawl more aggressively if we lack sufficient total peer weight.
bool sufficient_weight (total_weight_internal () > node.config.online_weight_minimum.number ());
bool sufficient_weight (total_weight_a > node.config.online_weight_minimum.number ());
uint16_t required_peer_count = sufficient_weight ? conservative_count : aggressive_count;
std::lock_guard<std::mutex> lock (probable_reps_mutex);

Expand Down Expand Up @@ -139,8 +140,9 @@ bool nano::rep_crawler::response (std::shared_ptr<nano::transport::channel> chan
return updated;
}

nano::uint128_t nano::rep_crawler::total_weight_internal ()
nano::uint128_t nano::rep_crawler::total_weight ()
{
std::lock_guard<std::mutex> lock (probable_reps_mutex);
nano::uint128_t result (0);
for (auto i (probable_reps.get<tag_weight> ().begin ()), n (probable_reps.get<tag_weight> ().end ()); i != n; ++i)
{
Expand All @@ -157,12 +159,6 @@ nano::uint128_t nano::rep_crawler::total_weight_internal ()
return result;
}

nano::uint128_t nano::rep_crawler::total_weight ()
{
std::lock_guard<std::mutex> lock (probable_reps_mutex);
return total_weight_internal ();
}

std::vector<nano::representative> nano::rep_crawler::representatives_by_weight ()
{
std::vector<nano::representative> result;
Expand Down
7 changes: 2 additions & 5 deletions nano/node/repcrawler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,18 +122,15 @@ class rep_crawler
/** Called continuously to crawl for representatives */
void ongoing_crawl ();

/** Returns a list of endpoints to crawl */
std::vector<std::shared_ptr<nano::transport::channel>> get_crawl_targets ();
/** Returns a list of endpoints to crawl. The total weight is passed in to avoid computing it twice. */
std::vector<std::shared_ptr<nano::transport::channel>> get_crawl_targets (nano::uint128_t total_weight_a);

/** When a rep request is made, this is called to update the last-request timestamp. */
void on_rep_request (std::shared_ptr<nano::transport::channel> channel_a);

/** Protects the probable_reps container */
std::mutex probable_reps_mutex;

/** Get total available weight from representatives (must be called with a lock on probable_reps_mutex) */
nano::uint128_t total_weight_internal ();

/** Probable representatives */
probably_rep_t probable_reps;
};
Expand Down