Skip to content

feat(share/discovery): persist discovered peers #2210

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

feat(share/discovery): persist discovered peers #2210

wants to merge 2 commits into from

Conversation

Wondertan
Copy link
Member

@Wondertan Wondertan commented May 15, 2023 •
edited
Loading

Closes #2208

  • Integrate option
  • Load peers into peer manager

@Wondertan Wondertan added the area:shares Shares and samples label May 15, 2023
@Wondertan Wondertan requested a review from renaynay as a code owner May 15, 2023 21:12
@Wondertan Wondertan self-assigned this May 15, 2023
@Wondertan Wondertan added the kind:feat Attached to feature PRs label May 15, 2023
@distractedm1nd
Copy link
Contributor

distractedm1nd commented May 16, 2023 •
edited
Loading

I'm not convinced this is necessary outside of the context of saving peers for bootstrapper usage, as I have never seen issues finding peers very quickly once we join the network. In fact, until other errors are fixed, this will probably give the disadvantage that we save bad peers (which is why restarting helps so often)

@Wondertan
Copy link
Member Author

Wondertan commented May 16, 2023 •
edited
Loading

I'm not convinced this is necessary outside of the context of saving peers for bootstrapper usage, as I have never seen issues finding peers very quickly once we join the network. In fact, until other errors are fixed, this will probably give the disadvantage that we save bad peers (which is why restarting helps so often)

Think of this in conjunction with Discard, which filters unhealthy peers. Overtime, discovery will persist only healthiest/closest peers without a need to rediscover

vgonkivs
vgonkivs reviewed May 16, 2023
View reviewed changes
share/availability/discovery/discovery_test.go
@@ -109,6 +162,18 @@ func (t *testnet) discovery(opts ...Option) *Discovery {
return disc
}

func (t *testnet) borkenDiscovery(opts ...Option) *Discovery {
Copy link
Member

Choose a reason for hiding this comment

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

typo brokenDiscovery

Copy link
Member Author

Choose a reason for hiding this comment

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

It was intentional 😈

@Wondertan
Copy link
Member Author

Closing until becomes relevant

@Wondertan Wondertan closed this Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vgonkivs vgonkivs vgonkivs left review comments

@renaynay renaynay Awaiting requested review from renaynay renaynay is a code owner

@distractedm1nd distractedm1nd Awaiting requested review from distractedm1nd

@walldiss walldiss Awaiting requested review from walldiss walldiss is a code owner

Assignees

@Wondertan Wondertan

Labels
area:shares Shares and samples kind:feat Attached to feature PRs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

share/discovery: persist discovered peers
3 participants
@Wondertan @distractedm1nd @vgonkivs