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

refactor the AutoRelay code #1240

Merged
merged 5 commits into from
Nov 16, 2021
Merged

Conversation

marten-seemann
Copy link
Contributor

This refactor introduces 3 improvements:

  • fixes a long-standing TODO to run relay discovery in a separate Go routine, thereby not blocking the main loop for up to 2.5 minutes at a time
  • we now use the EvtPeerConnectednessChanged to detect disconnects from relays
  • we immediately acquire a reservation if we detect that we connect to a static relay (before we'd have to wait until the refresh timer triggered a refresh of relays), thereby deflaking the 0182 sharness test in go-ipfs

Copy link
Contributor

@vyzo vyzo left a comment

Choose a reason for hiding this comment

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

just a small nit, other than that LGTM

evt := ev.(event.EvtPeerConnectednessChanged)
switch evt.Connectedness {
case network.Connected:
// If we just connect to one of our static relays, get a reservation immediately.
Copy link
Contributor

Choose a reason for hiding this comment

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

this might make discovery strategy refactoring tricky, but i guess it is fine for now.

if atomic.CompareAndSwapInt32(&ar.findRelaysRunning, 0, 1) {
go func() {
ar.findRelays(ctx)
atomic.StoreInt32(&ar.findRelaysRunning, 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

defer this?

@marten-seemann marten-seemann merged commit f7e7cde into master Nov 16, 2021
@marten-seemann marten-seemann deleted the autorelay-check-new-conns branch November 16, 2021 11:07
@aschmahmann aschmahmann mentioned this pull request Dec 1, 2021
80 tasks
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