-
Notifications
You must be signed in to change notification settings - Fork 1.1k
chore(memory-connection-limits): fix tests #6134
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that we should change the behavior here.
max_allowed_bytes
strongly implies that the set number is still allowed; it's the same also e.g. when setting a max_*
in ConnectionLimits
.
We could instead simply change the docs to
/// New inbound and outbound connections will be denied when the threshold is exceeded.
3f1f46b
to
e2fbcc2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that we should change the behavior here.
max_allowed_bytes strongly implies that the set number is still allowed; it's the same also e.g. when setting a max_* in ConnectionLimits.We could instead simply change the docs to
/// New inbound and outbound connections will be denied when the threshold is exceeded.
My rational was to give users what they had already been expecting from the doc, but yeah you are right, I missed the max_
and the fact that updating the doc is easier than updating the code.
Thanks Elena
…bp2p into fix-connection-memory-limits
e2fbcc2
to
bc71415
Compare
Description
while addressing the flakiness of the tests I noticed the memory limit was not accurate according to the documentation, documentation mentions the limit is reached not greater than.
This PR fixes that.
To address the flakiness due to now connections being probably a little bit smaller in memory footprint I gave room for a connection threshold in the tests.