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.

Sign up for GitHub

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

Jump to bottom

Support multiple work peers in the same host #2477

Merged
merged 5 commits into from
Jan 17, 2020
Merged

Support multiple work peers in the same host #2477

merged 5 commits into from
Jan 17, 2020

Conversation

guilhermelawless
Copy link
Contributor

@guilhermelawless guilhermelawless commented Jan 13, 2020 •
edited
Loading

This adds some more robust tests for distributed_work, which was lacking a fake peer to test from. The fake peer can act as a good (fast), slow (only replies after a while) or malicious (replies with wrong data) peer.

A few issues came up, all minor and now fixed:

  • (commit 1) node.work_generation_enabled was being called with the predefined work_peers even though there are secondary work peers that can be used.
  • (commit 2) only one peer was supported for each host. There doesn't seem to be any reason for this, so I simply made outstanding a vector and supporting changes.
  • (commit 3) cancels were not being sent on an "end of stream", which was a bug with the fake work peer; with this PR, any peer that has been reached (written to) and not responded erroneously (malicious) will receive a cancel. I think this adds some more guarantees of energy savings, with marginal costs.

I placed the fake work peer in nano/core_test/fakes/ but up to other suggestions

@guilhermelawless guilhermelawless added bug functionality quality improvements This item indicates the need for or supplies a better way to implement existing functionality unit test Related to a new, changed or fixed unit test labels Jan 13, 2020
@guilhermelawless guilhermelawless added this to the V21.0 milestone Jan 13, 2020
@guilhermelawless guilhermelawless self-assigned this Jan 13, 2020
@guilhermelawless guilhermelawless changed the title Support multiple work peers in same address Support multiple work peers in the same address Jan 14, 2020
@guilhermelawless guilhermelawless changed the title Support multiple work peers in the same address Support multiple work peers in the same host Jan 14, 2020
@guilhermelawless guilhermelawless merged commit eca1ec8 into nanocurrency:develop Jan 17, 2020
@guilhermelawless guilhermelawless deleted the distributed-work/peer-tests branch January 17, 2020 22:44
wezrule pushed a commit to wezrule/raiblocks that referenced this pull request Jan 22, 2020
@guilhermelawless @wezrule
Support multiple work peers in the same host (nanocurrency#2477)
4b439c4 
* Correct check for peers when creating work

* Support multiple work peers in the same address

* Send cancels despite errors to account for non-conforming implementations

* Add tests using a fake work peer, acting as good, malicious or slow

* Comment
wezrule pushed a commit to wezrule/raiblocks that referenced this pull request Jan 22, 2020
@guilhermelawless @wezrule
Support multiple work peers in the same host (nanocurrency#2477)
815c24e 
* Correct check for peers when creating work

* Support multiple work peers in the same address

* Send cancels despite errors to account for non-conforming implementations

* Add tests using a fake work peer, acting as good, malicious or slow

* Comment
wezrule added a commit that referenced this pull request Jan 24, 2020
@wezrule @clemahieu
@guilhermelawless @argakiig @SergiySW @cryptocode
Node telemetry (#2446)
1edb8c6 
* Node telemetry

* Add genesis block

* Don't checkif at the end of the stream with UDP to support compatibility with different message versions

* Add single request and optional blocking

* Formatting

* Use nano locks

* Fix clang build

* Out of date comments

* Update some names of classes/functions/variables

* Fix for disconnected peer channels & some cleanup

* Remove unnecessary lock requirement in active_transactions::lock. (#2475)

* Remove unnecessarry lock requirement in active_transactions::lock.
This also removes some code-smell around passing in a bool to determine if the lock should actually be acquired.

* Using a timeout counter in line with other test idioms.

* Confirmation solicitor revamp (#2472)

* Optional minimum version when querying representatives from crawler

* Revamping confirmation_solicitor to mimick previous active_transactions behavior

* Use a time-based approach to throttle confirmation requests and block flooding

* Addressing Wesley review

* Remove unusued node.hpp include (thanks wes)

* Simplify logic by using unordered_map::operator[] which calls the default constructor if not found

* Split solicitor add into broadcast+add and bring back the logic to  active_transactions

This brings back rate-limitting logic and modifying election variables to active_transactions only. Timings are also slightly adjusted:

- Only 2 requests required before starting to flood blocks
- Timings for test network

* Rename flag

* Only broadcast OR request confirmation in the same loop for the same election

* Enclose lambda in clang-format off

* Beta network reset #2 (#2476)

* Delete rep_weights_beta.bin

* New genesis

* Change header_magic_number to one never used before

* use v1.1 for actions-aws-cli (#2486)

* The start of CLI tests (#2403)

* The start of CLI tests

* Add needed header file after merge

* Serg review comments

* Websocket bootstrap subscription (#2471)

* Bootstrap attempt ID
* Websocket bootstrap subscription

* Change processed blocks factor for requeued pulls (#2437)

From `processed blocks / 10000` to `processed blocks / 4096` for better processing of largest chains in case of failures

* Upgrade confirmation height table to include cemented frontier (#2481)

* Upgrade confirmation height table to include cemented frontier

* Stein review comments

* Add //clang-format block around lambda

* Update comment (thanks Gui)

* Upgrade to v17 instead

* Update comments

* Request aggregator (#2485)

* Request aggregator

Adds a class that runs in a new thread to pool confirmation requests by endpoint.
This allows a reduction of bandwidth, vote generation, and moves some vote generation out of the I/O threads.

* Use a constant for confirm_ack_hashes_max

* Small code simplification

* Use const transaction

* Disable clang-format for lambdas

* Add missing deadlines and update deadline before poll block

* Use a scoped lock_guard pattern and initialize start in-class

* Misc. fixes and documentation

* use clang-format-8 (#2490)

There is some funkiness with clang-format-9 and lambdas that are not
honored

* Formatting fix so clangformat 8 applies cleanly (#2491)

* Support multiple work peers in the same host (#2477)

* Correct check for peers when creating work

* Support multiple work peers in the same address

* Send cancels despite errors to account for non-conforming implementations

* Add tests using a fake work peer, acting as good, malicious or slow

* Comment

* Formatting

* Fix multiple response error in RPC

* Formatting

* Add extra error handling when specifying port and not address with the RPC (and vice versa)

* Formatting

* Serg comments

* Formatting

* Mask not needed

* Missed file check in for mask removal

* Fix test failures

* Gui suggestions

* Formatting

* Fix assert with test on Windows and some cleanup

* Lower number of nodes in node_telemetry.many_nodes when using sanitizers

Co-authored-by: clemahieu <clemahieu@gmail.com>
Co-authored-by: Guilherme Lawless <guilherme.lawless@gmail.com>
Co-authored-by: Russel Waters <vaelstrom@gmail.com>
Co-authored-by: Sergey Kroshnin <sergiysw@gmail.com>
Co-authored-by: cryptocode <stein@nano.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@argakiig argakiig argakiig approved these changes

@cryptocode cryptocode cryptocode approved these changes

Assignees

@guilhermelawless guilhermelawless

Labels
bug functionality quality improvements This item indicates the need for or supplies a better way to implement existing functionality unit test Related to a new, changed or fixed unit test
Projects
None yet
Milestone
V21.0
Development

Successfully merging this pull request may close these issues.
3 participants
@guilhermelawless @argakiig @cryptocode