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

Customizable gossipsub backoff on unsubscribe #665

Merged
merged 3 commits into from
Dec 2, 2021
Merged

Conversation

Menduist
Copy link
Contributor

This is an alternative to status-im/nimbus-eth2#3025

It allows applications to choose a different backoff when we unsubcribe from a gossipsub topic.

Currently, if you unsubscribe - resubscribe from a topic in less than backoff seconds, all of your peers that were previously in your mesh are in your backoff, and won't allow you to graft them back. Setting the unsubcribeBackoff to a lower value allows you to alleviate this issue.

Note that it should still be >0 seconds, or some implementations (including ours) will ignore it. However, if it's less than BackoffSlackTime (2 seconds), it will effectively be a 0 seconds backoff.
It should not be an issue to have a 0 seconds backoff, since you're unsubscribed and peers should not attempt to graft you until you resub anyway. But maybe to avoid "race conditions", it's a good idea to keep a few seconds as backoff, just in case. Worst case scenario, a peer may attempt to graft you before receiving your unsub, and since this is not allowed, we'll prune him again with a bigger backoff.

arnetheduck
arnetheduck previously approved these changes Dec 2, 2021
@Menduist Menduist enabled auto-merge (squash) December 2, 2021 13:21
@zah
Copy link
Contributor

zah commented Dec 2, 2021

Please don't merge yet until we ship 1.5.5

@Menduist
Copy link
Contributor Author

Menduist commented Dec 2, 2021

Unstable will not be merged into master, and thus get bumped in nimbus, for the next few days/weeks, so 1.5.5 won't be affected anyway

Or you have reservation about this PR in general?

@zah
Copy link
Contributor

zah commented Dec 2, 2021

We haven't yet decided on the branching point from unstable that will go into the release. We avoid making commits to the testing and stable branches in preparation for releases.

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