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] - p2p: add spread to the discovery advertisement interval #5815

Closed
wants to merge 4 commits into from

Conversation

ivan4th
Copy link
Contributor

@ivan4th ivan4th commented Apr 8, 2024

Motivation

Need to lessen DHT overload from the routing discovery mechanism

Description

Default to 2h +/- 1h discovery advertisement interval

Test Plan

Verified on a mainnet node

Default to 2h +/- 1h discovery advertisement interval
Copy link

codecov bot commented Apr 8, 2024

Codecov Report

Attention: Patch coverage is 82.35294% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 80.2%. Comparing base (4cd82fd) to head (8d5f0e6).

Files Patch % Lines
p2p/host.go 71.4% 1 Missing and 1 partial ⚠️
p2p/dhtdiscovery/discovery.go 88.8% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           develop   #5815     +/-   ##
=========================================
- Coverage     80.2%   80.2%   -0.1%     
=========================================
  Files          286     286             
  Lines        29804   29815     +11     
=========================================
- Hits         23930   23919     -11     
- Misses        4219    4240     +21     
- Partials      1655    1656      +1     

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

backup, bootnodes []peer.AddrInfo
advertiseDelay time.Duration
advertiseInterval time.Duration
advertiseIntervalSpread time.Duration
Copy link
Member

Choose a reason for hiding this comment

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

Instead of making this configurable would it make sense to instead have it a relative amount, e.g. 1%?

@pigmej
Copy link
Member

pigmej commented Apr 8, 2024

bors merge

spacemesh-bors bot pushed a commit that referenced this pull request Apr 8, 2024
## Motivation

Need to lessen DHT overload from the routing discovery mechanism
@spacemesh-bors
Copy link

spacemesh-bors bot commented Apr 8, 2024

Build failed (retrying...):

  • systest-status

spacemesh-bors bot pushed a commit that referenced this pull request Apr 8, 2024
## Motivation

Need to lessen DHT overload from the routing discovery mechanism
@spacemesh-bors
Copy link

spacemesh-bors bot commented Apr 8, 2024

Build failed:

  • systest-status

@ivan4th
Copy link
Contributor Author

ivan4th commented Apr 9, 2024

bors merge

@ivan4th
Copy link
Contributor Author

ivan4th commented Apr 9, 2024

systests timed out with rather unclear reason, retrying

spacemesh-bors bot pushed a commit that referenced this pull request Apr 9, 2024
## Motivation

Need to lessen DHT overload from the routing discovery mechanism
@spacemesh-bors
Copy link

spacemesh-bors bot commented Apr 9, 2024

Timed out.

@ivan4th
Copy link
Contributor Author

ivan4th commented Apr 9, 2024

Timed out on this job: CI / build-tools (go-spacemesh, m2-pro) (push)

@ivan4th
Copy link
Contributor Author

ivan4th commented Apr 9, 2024

bors merge

spacemesh-bors bot pushed a commit that referenced this pull request Apr 9, 2024
## Motivation

Need to lessen DHT overload from the routing discovery mechanism
@spacemesh-bors
Copy link

spacemesh-bors bot commented Apr 9, 2024

Build failed (retrying...):

@ivan4th
Copy link
Contributor Author

ivan4th commented Apr 9, 2024

Test flake on Windows:

    grpcserver_test.go:759: 
        	Error Trace:	D:/a/go-spacemesh/go-spacemesh/api/grpcserver/grpcserver_test.go:759
        	Error:      	An error is expected but got nil.
        	Test:       	TestSmesherService/PostSetupStatusStream
    logger.go:146: 2024-04-09T15:31:53.496Z	INFO	grpc	finished streaming call with code OK	{"grpc.start_time": "2024-04-09T15:31:53Z", "system": "grpc", "span.kind": "server", "grpc.service": "spacemesh.v1.SmesherService", "grpc.method": "PostSetupStatusStream", "peer.address": "127.0.0.1:65112", "grpc.code": "OK", "grpc.time_ms": 50.828}
    logger.go:146: 2024-04-09T15:31:53.498Z	INFO	grpc	stopping the grpc server
--- FAIL: TestSmesherService/PostSetupStatusStream (1.11s)

@fasmat
Copy link
Member

fasmat commented Apr 9, 2024

bors merge

@spacemesh-bors
Copy link

spacemesh-bors bot commented Apr 9, 2024

Merge conflict.

@ivan4th ivan4th force-pushed the feature/disc-adv-spread branch from e34ac34 to cff8b4e Compare April 9, 2024 18:16
@ivan4th ivan4th force-pushed the feature/disc-adv-spread branch from cff8b4e to 8d5f0e6 Compare April 9, 2024 18:17
@ivan4th
Copy link
Contributor Author

ivan4th commented Apr 9, 2024

bors merge

spacemesh-bors bot pushed a commit that referenced this pull request Apr 9, 2024
## Motivation

Need to lessen DHT overload from the routing discovery mechanism
@spacemesh-bors
Copy link

spacemesh-bors bot commented Apr 9, 2024

Pull request successfully merged into develop.

Build succeeded:

@spacemesh-bors spacemesh-bors bot changed the title p2p: add spread to the discovery advertisement interval [Merged by Bors] - p2p: add spread to the discovery advertisement interval Apr 9, 2024
@spacemesh-bors spacemesh-bors bot closed this Apr 9, 2024
@spacemesh-bors spacemesh-bors bot deleted the feature/disc-adv-spread branch April 9, 2024 19:49
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.

3 participants