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

close stale rta connections #200

Open
wants to merge 11 commits into
base: rta-alpha4
Choose a base branch
from

Conversation

mbg033
Copy link
Contributor

@mbg033 mbg033 commented Dec 7, 2018

periodically (5s) closes in/out connections with state "state_before_handshake" if connections number exceeds max allowed (in-peers and out-peers)

erikd and others added 9 commits December 4, 2018 21:31
This rename is needed so that delete_in_connections can be added.
This is needed so that a max_in_connection_count can be added.
It was already possible to limit outgoing connections. One might want
to do this on home network connections with high bandwidth but low
usage caps.
Previously, the method name was printed as an exmpty string because
the input string had already been moved with `std::move`.
Previous code was unable to distingush between a connection error
and a communication error.
Original implementations could never have worked.
@mbg033 mbg033 force-pushed the merge/rta-alpha3-limit-incoming-connections branch from 41173a7 to 1b4324e Compare December 7, 2018 14:56
@mbg033 mbg033 force-pushed the merge/rta-alpha3-limit-incoming-connections branch from 1b4324e to efe66a5 Compare December 10, 2018 15:11
@jagerman
Copy link
Contributor

This PR seems like it is trying to paint over a bug.

We are repeatedly observing consistent excess connections that get established between the same two nodes are aren't used/don't handshake; this PR seems to do nothing to actually fix the source of the issue but rather just tries to deal with the symptom.

@mbg033
Copy link
Contributor Author

mbg033 commented Dec 12, 2018

We are repeatedly observing consistent excess connections that get established between the same two nodes are aren't used/don't handshake;

True, RTA tunnel connections don't use handshakes (while p2p cryptonote connections do). This way RTA tunnel connections will always have this state = state_before_handshake; Yes, it might be not really good to threat connections with the state = state_before_handshake as RTA tunnel connections, we'll probably introduce some type field in the future to make it explicit

this PR seems to do nothing to actually fix the source of the issue but rather just tries to deal with the symptom.

This is just "extra safety", tunnel connections supposed to be closed with "idle timer" implemented in #199

@mbg033 mbg033 changed the base branch from rta-alpha3 to rta-alpha4 December 26, 2018 07:30
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