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: handle discovery startup errors properly #5888

Closed
wants to merge 4 commits into from

Conversation

ivan4th
Copy link
Contributor

@ivan4th ivan4th commented Apr 25, 2024

Motivation

There was no error checking for DHT discovery startup, yet it could fail e.g. during LevelDB corruption:

Error: init services: error starting discovery: open leveldb at /opt/go-spacemesh/go-spacemesh-0/p2p: leveldb/journal: block/chunk corrupted: chunk length overflows block (1270 bytes) [file=000247.log]

In this case, error was being swallowed and DHT not started, breaking acquisition of new peers.

Description

This PR adds error checking for DHT startup and also fixes panics that were happening if app startup was interrupted this early.

Test Plan

Verified on a mainnet node

TODO

  • Explain motivation or link existing issue(s)
  • Test changes and document test plan
  • Update changelog as needed

Copy link

codecov bot commented Apr 25, 2024

Codecov Report

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

Project coverage is 80.6%. Comparing base (a8d47f2) to head (c8e9850).

Files Patch % Lines
p2p/dhtdiscovery/discovery.go 60.0% 2 Missing and 2 partials ⚠️
p2p/upgrade.go 0.0% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           develop   #5888     +/-   ##
=========================================
- Coverage     80.6%   80.6%   -0.1%     
=========================================
  Files          285     285             
  Lines        29512   29521      +9     
=========================================
  Hits         23801   23801             
- Misses        4122    4128      +6     
- Partials      1589    1592      +3     

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

p2p/upgrade.go Outdated Show resolved Hide resolved
Co-authored-by: Bartosz Różański <[email protected]>
@ivan4th
Copy link
Contributor Author

ivan4th commented Apr 29, 2024

bors merge

@spacemesh-bors
Copy link

Merge conflict.

@ivan4th
Copy link
Contributor Author

ivan4th commented Apr 29, 2024

bors merge

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

There was no error checking for DHT discovery startup, yet it could fail e.g. during LevelDB corruption:
```
Error: init services: error starting discovery: open leveldb at /opt/go-spacemesh/go-spacemesh-0/p2p: leveldb/journal: block/chunk corrupted: chunk length overflows block (1270 bytes) [file=000247.log]
```
In this case, error was being swallowed and DHT not started, breaking acquisition of new peers.



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 p2p: handle discovery startup errors properly [Merged by Bors] - p2p: handle discovery startup errors properly Apr 29, 2024
@spacemesh-bors spacemesh-bors bot closed this Apr 29, 2024
@spacemesh-bors spacemesh-bors bot deleted the fix/disc-startup-errors branch April 29, 2024 13:41
ivan4th added a commit that referenced this pull request Apr 29, 2024
There was no error checking for DHT discovery startup, yet it could fail e.g. during LevelDB corruption:
```
Error: init services: error starting discovery: open leveldb at /opt/go-spacemesh/go-spacemesh-0/p2p: leveldb/journal: block/chunk corrupted: chunk length overflows block (1270 bytes) [file=000247.log]
```
In this case, error was being swallowed and DHT not started, breaking acquisition of new peers.

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