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

Make static dials subject to the maxActiveDials limit. #1202

Merged
merged 1 commit into from
Oct 30, 2020

Conversation

oneeman
Copy link
Contributor

@oneeman oneeman commented Oct 30, 2020

Description

In #1192, we pulled in the the new dial scheduler from upstream, but modified it to exempt static dials from the limits it put on them (which were set as maxDialPeers and maxActiveDials), because we rely on static dials for the connections between validators/proxies and their validator/proxy peers. This PR modifies that to make them respect the maxActiveDials limit. So, while static dialed connections will still be exempt from maxDialPeers (and therefore from maxPeers), the establishing of those connections will not be allowed to push the number of active dials being attempted past its limit (whose default value was set to 50 upstream).

Note that, prior to the new dial scheduler (i.e. in celo-blockchain 1.1.0 and earlier and in the corresponding code upstream), the equivalent to maxActiveDial (called maxActiveDialTasks) was 16, and static dials were subject to it (though not to maxDialPeers), so even after this PR we will still have a considerably larger limit than before it. See the value at

maxActiveDialTasks = 16

Tested

Updated the unit tests involved, as well as re-added a test from upstream (though modified to account for the fact that we exempt static dials from maxDialPeers), which checks that maxActiveDials is respected and that the peers to dial are selected at random from d.staticPool.

Backwards compatibility

Doesn't affect the protocol, so there are no backwards compatibility concerns.

@trianglesphere
Copy link
Contributor

looks good to me

@oneeman oneeman merged commit b1508ea into master Oct 30, 2020
@oneeman oneeman deleted the oneeman/static-dial-max-active-dials-limit branch October 30, 2020 18:13
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.

None yet

2 participants