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

[v17] Replay votes in response to a confirm_req for an active block #1409

Merged

Conversation

Projects
None yet
3 participants
@SergiySW
Copy link
Collaborator

commented Nov 27, 2018

Port of #1382

Fixes #1393

@SergiySW SergiySW requested a review from PlasmaPower Nov 27, 2018

@SergiySW SergiySW added this to the V17.0 milestone Nov 27, 2018

@SergiySW SergiySW force-pushed the SergiySW:confirm-req-active-replay-17 branch from b363acf to 9970618 Nov 27, 2018

@PlasmaPower

This comment has been minimized.

Copy link
Contributor

commented Nov 27, 2018

IIRC compute_rep_votes is never called, so we can get rid of that function entirely.

I was hoping that V17 would be a good opportunity to do some kind of LRU cache instead of tying it to active_transactions, but this is still a lot better than nothing, and we can improve it later if we need to. 👍

@SergiySW

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 27, 2018

If you have LRU cache in mind, we can merge this in 17.0 & not merge to master, wait for cache I guess
@rkeene ?

@SergiySW SergiySW force-pushed the SergiySW:confirm-req-active-replay-17 branch from 20fe710 to a42a577 Nov 27, 2018

@rkeene

This comment has been minimized.

Copy link
Contributor

commented Nov 27, 2018

That's fine -- getting it fixed in V17.0 and doing something better in V18+ works

@rkeene rkeene added the enhancement label Nov 27, 2018

Regenerate votes for long unconfirmed blocks
Useful for tests & largest representatives

@SergiySW SergiySW force-pushed the SergiySW:confirm-req-active-replay-17 branch from a8fc0e8 to 2cf9fa9 Nov 27, 2018

Add recursive_mutex to foreach_representative
To prevent node.unlock_search assert
@@ -49,6 +49,16 @@ void rai::vote_generator::send (std::unique_lock<std::mutex> & lock_a)
auto transaction (node.store.tx_begin_read ());
node.wallets.foreach_representative (transaction, [this, &hashes_l, &transaction](rai::public_key const & pub_a, rai::raw_key const & prv_a) {
auto vote (this->node.store.vote_generate (transaction, pub_a, prv_a, hashes_l));
std::unique_lock<std::mutex> lock (node.active.mutex);

This comment has been minimized.

Copy link
@rkeene

rkeene Nov 28, 2018

Contributor

MSVC is failing to compile this:

c:\projects\myproject\rai\node\voting.cpp(52): error C2143: syntax error: missing ')' before '.'
c:\projects\myproject\rai\node\voting.cpp(52): error C3484: syntax error: expected '->' before the return type
c:\projects\myproject\rai\node\voting.cpp(52): error C3613: missing return type after '->' ('int' assumed)
c:\projects\myproject\rai\node\voting.cpp(52): error C3646: 'active': unknown override specifier
c:\projects\myproject\rai\node\voting.cpp(52): error C3551: if a trailing return type is used then the leading return type shall be the single type-specifier 'auto' (not 'std::unique_lock<std::mutex>')
c:\projects\myproject\rai\node\voting.cpp(52): error C2059: syntax error: '.'
c:\projects\myproject\rai\node\voting.cpp(52): error C2059: syntax error: ')'

This comment has been minimized.

Copy link
@PlasmaPower

PlasmaPower Nov 28, 2018

Contributor

We've seen this type of problem before, it needs to be this->node. I wonder if we can enable a warning for this in GCC.

@rkeene rkeene merged commit 62e3a40 into nanocurrency:releases/v17 Nov 29, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

SergiySW added a commit to SergiySW/raiblocks that referenced this pull request Jan 2, 2019

Add recursive_mutex to foreach_representative
To prevent node.unlock_search assert

Initially part of v17 nanocurrency#1409

SergiySW added a commit that referenced this pull request Jan 28, 2019

Calculate votes for local representatives in block_confirm (#1534)
* Add recursive_mutex to foreach_representative

To prevent node.unlock_search assert

Initially part of v17 #1409

* Calculate votes for local representatives in block_confirm

* Use vote_generator

* Fix

* Make vote_generator public

* Improve node.unlock_search test

* Formatting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.