Skip to content
New issue

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

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

Already on GitHub? # to your account

Separate vote_router from active_transactions #4607

Merged
merged 2 commits into from
May 8, 2024

Conversation

clemahieu
Copy link
Contributor

This change is to simplify the active_transactions class which has many responsibilities around election management. This change separates the responsibility of routing votes to their respective elections from other responsibilities inside active_transactions.

The mapping hash -> election in nano::active_transactions::blocks is moved into a new class nano::vote_router. Vote processing logic is moved from nano::active_transactions::vote to nano::vote_router::vote.

nano::vote_router changes the behavior from active_transactions::blocks in that it does not own the lifetime of elections. Only a weak_ptr to election is held and a background thread cleans up orphaned routes periodically.

nano::vote_code and nano::vote_source are moved into the vote_router.hpp file and forward declared when possible.

Copy link
Contributor

@pwojcikdev pwojcikdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really cool. I can already see how this can be improved further with read/write locks to allow for faster multithreaded vote processing (no need for an exclusive mutex when we're not modifying the mapping). I struggled to get it done in the past when election routing was part of the AEC, but this makes it much easier.

std::unique_lock lock{ mutex };
for (auto const & [hash, _] : election.blocks ())
{
elections.erase (hash);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Election object can hold a limited number of blocks and some can be dropped during its lifetime. While the cleanup thread should still clean up the remaining hashes, at first sight this might be confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this directly helps but I renamed these to connect/disconnect to represent the looser ownership nature.

pwojcikdev
pwojcikdev previously approved these changes May 7, 2024
@clemahieu clemahieu merged commit 0f1fadc into nanocurrency:develop May 8, 2024
25 of 26 checks passed
@qwahzi qwahzi added this to the V27 milestone May 21, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants