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: implement relay connectivity loop #642

Merged
merged 4 commits into from
Aug 15, 2023
Merged

Conversation

chaitanyaprem
Copy link
Collaborator

Description

Implemented a relay connectivity loop similar to https://github.com/waku-org/nwaku/pull/1482/files.

Changes

  • Relay connectivity loop
  • Updated relay peer connection target to have min of 10 outRelay connections ir-respective of max connections (similar to nwaku)
  • Fixed connection pruning logic to only use connectedPeer count and peers which are using relay protocol.

Tests

All unit tests passed.

@chaitanyaprem chaitanyaprem mentioned this pull request Aug 14, 2023
7 tasks
@status-im-auto
Copy link

status-im-auto commented Aug 14, 2023

Jenkins Builds

Click to see older builds (18)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ b66b13f #1 2023-08-14 11:47:51 ~1 min linux 📦deb
✔️ b66b13f #1 2023-08-14 11:48:25 ~1 min nix-flake 📄log
✔️ b66b13f #1 2023-08-14 11:49:20 ~2 min tests 📄log
✔️ b66b13f #1 2023-08-14 11:49:25 ~2 min tests 📄log
✔️ b66b13f #1 2023-08-14 11:50:23 ~3 min android 📦tgz
✔️ b66b13f #1 2023-08-14 11:51:16 ~4 min ios 📦tgz
✔️ 57fdd45 #2 2023-08-14 12:08:10 ~1 min linux 📦deb
✔️ 57fdd45 #2 2023-08-14 12:08:58 ~1 min nix-flake 📄log
✔️ 57fdd45 #2 2023-08-14 12:09:42 ~2 min tests 📄log
✔️ 57fdd45 #2 2023-08-14 12:09:52 ~2 min tests 📄log
✔️ 57fdd45 #2 2023-08-14 12:10:44 ~3 min android 📦tgz
✔️ 57fdd45 #2 2023-08-14 12:11:10 ~4 min ios 📦tgz
✔️ d17ce1f #3 2023-08-14 12:16:36 ~1 min linux 📦deb
✔️ d17ce1f #3 2023-08-14 12:17:04 ~1 min nix-flake 📄log
✔️ d17ce1f #3 2023-08-14 12:17:52 ~2 min tests 📄log
✔️ d17ce1f #3 2023-08-14 12:17:57 ~2 min tests 📄log
✔️ d17ce1f #3 2023-08-14 12:18:22 ~3 min android 📦tgz
✔️ d17ce1f #3 2023-08-14 12:19:12 ~4 min ios 📦tgz
Commit #️⃣ Finished (UTC) Duration Platform Result
e3a871c #4 2023-08-15 01:15:25 ~17 sec linux 📄log
✖️ e3a871c #4 2023-08-15 01:15:33 ~22 sec tests 📄log
✖️ e3a871c #4 2023-08-15 01:16:03 ~52 sec nix-flake 📄log
✖️ e3a871c #4 2023-08-15 01:16:35 ~1 min tests 📄log
e3a871c #4 2023-08-15 01:18:10 ~3 min android 📄log
e3a871c #4 2023-08-15 01:18:17 ~3 min ios 📄log
✔️ 3d7426f #5 2023-08-15 01:21:20 ~33 sec linux 📦deb
✔️ 3d7426f #5 2023-08-15 01:22:20 ~1 min tests 📄log
✔️ 3d7426f #5 2023-08-15 01:22:20 ~1 min tests 📄log
✔️ 3d7426f #5 2023-08-15 01:22:40 ~1 min nix-flake 📄log
✔️ 3d7426f #5 2023-08-15 01:24:18 ~3 min android 📦tgz
✔️ 3d7426f #5 2023-08-15 01:26:42 ~5 min ios 📦tgz

@chaitanyaprem chaitanyaprem marked this pull request as ready for review August 14, 2023 12:15
@richard-ramos richard-ramos changed the title feat: implement relay conenctivity loop feat: implement relay connectivity loop Aug 14, 2023
Copy link
Member

@richard-ramos richard-ramos left a comment

Choose a reason for hiding this comment

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

Cool PR! looking fwd to see it in action on desktop 🚀

Just left some minor comments.

waku/v2/peermanager/peer_connector.go Outdated Show resolved Hide resolved
waku/v2/peermanager/peer_connector.go Outdated Show resolved Hide resolved
waku/v2/peermanager/peer_connector.go Outdated Show resolved Hide resolved
@@ -73,40 +94,112 @@ func (pm *PeerManager) connectivityLoop(ctx context.Context) {
case <-ctx.Done():
return
case <-t.C:
pm.pruneInRelayConns()
pm.connectToRelayPeers()
Copy link
Member

Choose a reason for hiding this comment

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

In line 92, we probably need to add a defer t.Stop()

} else if direction == network.DirOutbound {
outPeers = append(outPeers, p)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Is it worth to add a pm.logger.Error line indicating the cause of error while attempting to retrieve the direction?

waku/v2/peermanager/peer_manager.go Outdated Show resolved Hide resolved
waku/v2/peermanager/peer_manager.go Outdated Show resolved Hide resolved
waku/v2/peermanager/peer_manager.go Outdated Show resolved Hide resolved
}
pm.logger.Info("Successfully disconnected connection towards peer",
Copy link
Member

Choose a reason for hiding this comment

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

It probably makes sense to either remove this log line, or change it into Debug, since this is what we expect should happen most of the time, it will probably introduce too much noise in the logs

Suggested change
pm.logger.Info("Successfully disconnected connection towards peer",
pm.logger.Info("Successfully disconnected connection towards peer",

waku/v2/peermanager/peer_manager.go Show resolved Hide resolved
@chaitanyaprem chaitanyaprem merged commit 06f027b into master Aug 15, 2023
2 checks passed
@chaitanyaprem chaitanyaprem deleted the feat/relay-conn-loop branch August 15, 2023 01:27
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