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

[Merged by Bors] - Make routing discovery more configurable and less spammy by default #5494

Closed
wants to merge 4 commits into from

Conversation

ivan4th
Copy link
Contributor

@ivan4th ivan4th commented Jan 25, 2024

Motivation

Enabling routing discovery advertisement causes too much of kad (DHT) traffic on the nodes with DHT server mode enabled (public IP)

Description

Add discovery-timings section in the p2p config for tuning routing discovery intervals and timeouts.
Less spammy defaults:

  • only start advertising after 1h of uptime (but still start discovering the peers behind NAT immediately)
  • use 1h TTL / re-advertise interval
  • use 1m interval for discovery / advertisement retries

Test Plan

Tested by running a node on mainnet and observing discovery of other peers

Add `discovery-timings` section in the p2p config for tuning routing
discovery intervals and timeouts.
Less spammy defaults:
* only start advertising after 1h of uptime (but still start
  discovering the peers behind NAT immediately)
* use 1h TTL / re-advertise interval
* use 1m interval for discovery / advertisement retries
Copy link

codecov bot commented Jan 25, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (4f8ba99) 77.6% compared to head (1375c32) 77.6%.
Report is 3 commits behind head on develop.

Files Patch % Lines
p2p/dhtdiscovery/discovery.go 93.9% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           develop   #5494   +/-   ##
=======================================
  Coverage     77.6%   77.6%           
=======================================
  Files          267     267           
  Lines        31068   31095   +27     
=======================================
+ Hits         24119   24142   +23     
- Misses        5420    5424    +4     
  Partials      1529    1529           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

p2p/dhtdiscovery/discovery.go Outdated Show resolved Hide resolved
@ivan4th
Copy link
Contributor Author

ivan4th commented Jan 25, 2024

bors merge

spacemesh-bors bot pushed a commit that referenced this pull request Jan 25, 2024
…5494)

## Motivation

Enabling routing discovery advertisement causes too much of kad (DHT) traffic on the nodes with DHT server mode enabled (public IP)



Co-authored-by: Ivan Shvedunov <[email protected]>
@spacemesh-bors
Copy link

Build failed:

@ivan4th
Copy link
Contributor Author

ivan4th commented Jan 25, 2024

bors merge

spacemesh-bors bot pushed a commit that referenced this pull request Jan 25, 2024
…5494)

## Motivation

Enabling routing discovery advertisement causes too much of kad (DHT) traffic on the nodes with DHT server mode enabled (public IP)



Co-authored-by: Ivan Shvedunov <[email protected]>
@spacemesh-bors
Copy link

Pull request successfully merged into develop.

Build succeeded:

@spacemesh-bors spacemesh-bors bot changed the title Make routing discovery more configurable and less spammy by default [Merged by Bors] - Make routing discovery more configurable and less spammy by default Jan 25, 2024
@spacemesh-bors spacemesh-bors bot closed this Jan 25, 2024
@spacemesh-bors spacemesh-bors bot deleted the feature/routing-disc-timings branch January 25, 2024 21:42
ivan4th added a commit that referenced this pull request Jan 25, 2024
…5494)

Enabling routing discovery advertisement causes too much of kad (DHT) traffic on the nodes with DHT server mode enabled (public IP)

Co-authored-by: Ivan Shvedunov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants