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

feat(peer-tracker): add minAmount of peers tracked in tracker #139

Merged
merged 2 commits into from
Dec 22, 2023

Conversation

walldiss
Copy link
Member

Adds minPeerTrackerSizeBeforeGC setting to peer tracker, that specifies the min amount of peers before the peerTracker starts removing peers.

@codecov-commenter
Copy link

codecov-commenter commented Dec 19, 2023

Codecov Report

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

Comparison is base (28ff21c) 63.60% compared to head (30b2efd) 63.71%.

Files Patch % Lines
p2p/peer_tracker.go 94.28% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #139      +/-   ##
==========================================
+ Coverage   63.60%   63.71%   +0.11%     
==========================================
  Files          39       39              
  Lines        3366     3382      +16     
==========================================
+ Hits         2141     2155      +14     
- Misses       1060     1061       +1     
- Partials      165      166       +1     

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

@Wondertan
Copy link
Member

@vgonkivs and I discussed completely removing GC as a solution and significantly simplifying the exchange. Unfortunately, we didn't note other essential details, so I am happy to merge this as a quick fix with the proper one coming afterward. Likely when @vgonkivs is back

renaynay
renaynay previously approved these changes Dec 20, 2023
p2p/peer_tracker.go Outdated Show resolved Hide resolved
@walldiss walldiss requested a review from renaynay December 21, 2023 03:49
Copy link
Member

@renaynay renaynay left a comment

Choose a reason for hiding this comment

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

This is good as a quick fix.

@renaynay renaynay merged commit 6afe95f into main Dec 22, 2023
2 checks passed
@renaynay renaynay deleted the min-amount-of-peers-in-tracker branch December 22, 2023 09:11
ramin added a commit to celestiaorg/celestia-node that referenced this pull request Jan 2, 2024
Bumps go-header to the latest RC to pull in the fix to the `peerTracker`
in celestiaorg/go-header#139. It's suspected
that the `peerTracker`'s aggressive scoring mechanism is causing header
syncing to stall after periods of poor connectivity.

---------

Co-authored-by: ramin <[email protected]>
renaynay added a commit to renaynay/celestia-node that referenced this pull request Jan 15, 2024
Bumps go-header to the latest RC to pull in the fix to the `peerTracker`
in celestiaorg/go-header#139. It's suspected
that the `peerTracker`'s aggressive scoring mechanism is causing header
syncing to stall after periods of poor connectivity.

---------

Co-authored-by: ramin <[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.

5 participants