Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

client/authority-discovery: Add option to disable querying #6354

Closed

Conversation

mxinden
Copy link
Contributor

@mxinden mxinden commented Jun 15, 2020

The authority discovery module (1) publishes a nodes address onto the
DHT and (2) queryies the DHT for the addresses of other authorities.

When a node is only allowed to connect to the configured reserved nodes
there is no benefit in discovering other authorities (2). To reduce the
unnecessary overhead this patch adds an option to disable (2) while
still running (1).

In the scenario above where a node can only connect to a small subset of
the nodes (reserved nodes) of the DHT the publishing of addresses (1) is
likely failing as the node can not place its addresses on the node
closest to its chain identity. For now the node will still try to
publish its addresses, hoping that (a) one of its reserved nodes is
close enough to accept the record and (b) the reserved node will
republish the record to other nodes.

polkadot companion: paritytech/polkadot#1262

Relates to #6031.

The authority discovery module (1) publishes a nodes address onto the
DHT and (2) queryies the DHT for the addresses of other authorities.

When a node is only allowed to connect to the configured reserved nodes
there is no benefit in discovering other authorities (2). To reduce the
unnecessary overhead this patch adds an option to disable (2) while
still running (1).

In the scenario above where a node can only connect to a small subset of
the nodes (reserved nodes) of the DHT the publishing of addresses (1) is
likely failing as the node can not place its addresses on the node
closest to its chain identity. For now the node will still try to
publish its addresses, hoping that (a) one of its reserved nodes is
close enough to accept the record and (b) the reserved node will
republish the record to other nodes.
@mxinden mxinden added A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. B2-breaksapi labels Jun 15, 2020
@mxinden mxinden requested a review from tomaka June 15, 2020 12:09
Comment on lines +31 to +32
/// Received dht value found event even though querying for authorities is disabled.
ReceivingDhtValueFoundEventWithQueryingDisabled,
Copy link
Contributor

Choose a reason for hiding this comment

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

This assumes that the authority discovery is the only code that will ever make DHT queries.
This cannot happen at the moment, but if another module calls get_value on the network service to make its own queries (unrelated to the authority discovery), these errors would be triggered.

@tomaka tomaka added A6-mustntgrumble B0-silent Changes should not be mentioned in any release notes and removed A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes labels Jun 30, 2020
@gavofyork
Copy link
Member

@mxinden needs resolving.

@bkchr
Copy link
Member

bkchr commented Jul 28, 2020

CC @mxinden

@mxinden
Copy link
Contributor Author

mxinden commented Jul 29, 2020

Closing here as the authority discovery is likely going through a larger change (#6595). Sorry for keeping this open so long.

@mxinden mxinden closed this Jul 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants