Move routing table exchange to another thread #5089
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mm-near
reviewed
Oct 27, 2021
mm-near
reviewed
Oct 28, 2021
pmnoxx
requested review from
bowenwang1996,
chefsale,
frol,
khorolets and
mfornet
as code owners
3 tasks
matklad
reviewed
Oct 28, 2021
matklad
reviewed
Oct 28, 2021
mm-near
reviewed
Oct 28, 2021
mm-near
reviewed
Oct 28, 2021
|
I moved out some of the changes to: #5097 |
3 tasks
3 tasks
This was
linked to
issues
Oct 29, 2021
|
@mm-near If you think it will make reviewing easier, I can extract the refactoring mentioned in summary to a separate change. |
|
@mm-near @bowenwang1996 I added summary to the first post in the PR, I hope it helps. |
near-bulldozer bot
pushed a commit
that referenced
this pull request
Nov 7, 2021
We should refactor network code, before #5089 - This refactoring doesn't contain any code changes, only methods/attributes are renamed to better names - All changes are extracted from #5089, this was done to keep that PR smaller. This will cause merge conflicts, but it's better to extract syntax only changes to make #5089 easier to review. Here is design doc for changes planned (https://docs.google.com/document/d/1FV3oyMRo52EQAgYroVd6Ws-5-ylkLSTdMOstY-IdL-M/edit?usp=sharing)
This was referenced Nov 7, 2021
pmnoxx
added
A-network
near-bulldozer bot
pushed a commit
that referenced
this pull request
Nov 8, 2021
Make `RoutingTableActor` use Context instead of `SyncContext`. `SyncContext` is meant to be used in situations, where someone wants to start multiple instances of actors, each one separate thread. We only have one instance, and the code wouldn't work with multiple instances, so we are changing `SyncContext` to `Context`, to make reading code less confusion. Additional changes: - Add `edge_verifier_pool` to `RoutingTableActor` (for #5089) - Rename `routing_table_pool` to `routing_table_addr` Changes are extracted from #5089 to make it easier to review.
near-bulldozer bot
pushed a commit
that referenced
this pull request
Nov 9, 2021
Add new features to routing table actor. - do edge pruning (not used yet) - store all edges in `edges_info` - `StartRoutingTableSync` (handling first message of protocol_feature_routing_exchange_algorithm) (not yet used) - `RoutingTableUpdate` (updating routing table and maybe pruning edges) (not yet used) - `AddVerifiedEdges` - adds verified edges and updates stats (Similar to `AddEdges`, but does more) (not yet used) This is done to split #5089 into smaller pieces. This should be reviewed after #5159, as this PR also includes changes in that request.
|
@mm-near Can you take a look? This PR is much smaller after splitting it 6 times. |
mm-near
approved these changes
Nov 9, 2021
|
@chefsale @frol @khorolets Can you take a look? |
|
@chefsale @frol @khorolets Can you take a look? |
chefsale
approved these changes
Nov 9, 2021
pmnoxx
deleted the
piotr-move-routing-table-exchange-to-another-thread-pt2
branch
pmnoxx
added a commit
that referenced
this pull request
Nov 20, 2021
Make `RoutingTableActor` use Context instead of `SyncContext`. `SyncContext` is meant to be used in situations, where someone wants to start multiple instances of actors, each one separate thread. We only have one instance, and the code wouldn't work with multiple instances, so we are changing `SyncContext` to `Context`, to make reading code less confusion. Additional changes: - Add `edge_verifier_pool` to `RoutingTableActor` (for #5089) - Rename `routing_table_pool` to `routing_table_addr` Changes are extracted from #5089 to make it easier to review.
pmnoxx
added a commit
that referenced
this pull request
Nov 20, 2021
Add new features to routing table actor. - do edge pruning (not used yet) - store all edges in `edges_info` - `StartRoutingTableSync` (handling first message of protocol_feature_routing_exchange_algorithm) (not yet used) - `RoutingTableUpdate` (updating routing table and maybe pruning edges) (not yet used) - `AddVerifiedEdges` - adds verified edges and updates stats (Similar to `AddEdges`, but does more) (not yet used) This is done to split #5089 into smaller pieces. This should be reviewed after #5159, as this PR also includes changes in that request.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In this PR we will creating copy of routing table to another thread.
This is related to #4859
Summary:
PeerManagerno longer has copy ofedges_info, instead it only stores list of edges to peers that it is/was connected tolocal_edges_infoPeerManagerthat were stored inedges_infopeer_forwardingis done inRoutingTableActor,PeerManagergets it from another thread every second / tests now useRoutingTableActorinstead ofRoutingTablestructpub raw_graph: Graph,and a few other data structures toRoutingTableActoradv_get_routing_table_newfrom jsonrpc server. Now, routing table is only stored in one place, so this method is no longer neededCloses #4859