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: use mesh peers instead of all peers for determining topic health #1150

Merged
merged 2 commits into from
Jul 3, 2024

Conversation

richard-ramos
Copy link
Member

Fixes #1131
I could not figure out the logic behind how gs.mesh is populated in go-lib2p2-pubsub, so in the meantime I disabled the test that check for connection changes

@status-im-auto
Copy link

status-im-auto commented Jul 2, 2024

Jenkins Builds

Click to see older builds (3)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 6a5d86f #1 2024-07-02 18:49:22 ~2 min nix-flake 📄log
✖️ 63e2cf6 #2 2024-07-03 19:26:13 ~14 sec nix-flake 📄log
✔️ 3fedd91 #3 2024-07-03 19:32:52 ~2 min nix-flake 📄log
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 5b3b9c7 #4 2024-07-03 19:39:52 ~1 min nix-flake 📄log
✔️ be0761e #5 2024-07-03 19:54:11 ~1 min nix-flake 📄log

Copy link
Collaborator

@chaitanyaprem chaitanyaprem left a comment

Choose a reason for hiding this comment

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

LGTM

@chaitanyaprem
Copy link
Collaborator

Looks like a datarace has been detected, better to address it before merging.

@richard-ramos
Copy link
Member Author

Hm, it seems that the way that go-libp2p-pubsub uses the mesh attribute in GossipSubRouter is prone to data races:

  • In the heartbeat() function which is executed by a separate goroutine heartBeatTimer()
  • As well as calls done by Pubsub to add/remove peers

@richard-ramos richard-ramos force-pushed the topic-health branch 2 times, most recently from 63e2cf6 to 3fedd91 Compare July 3, 2024 19:30
@richard-ramos
Copy link
Member Author

I modified go-libp2p-pubsub to implement something similar to ListPeers. That solved the data race

@richard-ramos richard-ramos merged commit 7c13021 into master Jul 3, 2024
12 checks passed
@richard-ramos richard-ramos deleted the topic-health branch July 3, 2024 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

feat: use gossipsub mesh size to report topic health for relay
3 participants