Skip to content
This repository has been archived by the owner on Mar 5, 2024. It is now read-only.

sync_peer option to specify preferred nodes #1319

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

joecaswell
Copy link
Contributor

adds blockchain sync_peers configuration option, accepts a list of string multi-addresses

@joecaswell joecaswell force-pushed the jc/sync-peer-configuration-option branch from cbad321 to 08574df Compare April 29, 2022 18:40
State#state{sync_pid = Pid, sync_ref = Ref}
end.

get_configured_or_random_peer(SwarmTID) ->
case get_configured_sync_peer(SwarmTID) of
undefined-> get_random_peer(SwarmTID);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
undefined-> get_random_peer(SwarmTID);
undefined -> get_random_peer(SwarmTID);

[] ->
lager:debug("No sync_peers configured"),
undefined;
ConfiguredPeers ->
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ConfiguredPeers ->
ConfiguredPeers when is_list(ConfiguredPeers) ->

Copy link
Contributor Author

@joecaswell joecaswell Apr 29, 2022

Choose a reason for hiding this comment

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

As long as we're validating, we should check [P | _] when is_list(P) to make sure it is a list of strings, also should gracefully fail back to random so the node will still try to sync if the config is pooched.

Comment on lines 927 to 928
{Left, Right} = lists:split(rand:uniform(length(ConfiguredPeers)), ConfiguredPeers),
get_configured_sync_peer(SwarmTID, Peerbook, Right ++ Left)
Copy link
Contributor

Choose a reason for hiding this comment

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

is this "cutting the deck" of configured peers so to speak? just a cheaper version of shuffling the list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, exactly. We only need one peer, the rest of the list is only used if the chosen one fails to connect.

Copy link
Contributor

Choose a reason for hiding this comment

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

slick solution 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants