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

Mix: Add connection maintenance #928

Merged
merged 5 commits into from
Dec 9, 2024
Merged

Mix: Add connection maintenance #928

merged 5 commits into from
Dec 9, 2024

Conversation

youngjoon-lee
Copy link
Contributor

1. What does this PR implement?

For spam protection and resiliency, the Tier 1 design contains the Connection Maintenance, which measures frequencies of several incoming messages, and close/open connections to maintain the reasonable quality of delivery.
How frequencies are measured is defined in the Frequencies Measurement. How to close/open connections is defined in here.

This PR implements all mentioned above.

Plus, this implementation provides an option to disable it. Ideally, all nodes should enable connection maintenance. However, we are not sure what much frequency we should expect yet. If we set the threshold too high or small, the protocol would close too many healthy connections than intended. The estimation is not simple even though we have the persistent transmission, because of the immediate forwarding.
I will run tests soon with multiple (virtual) nodes to measure the actual frequencies, so that we can provide the recommended parameters to set for the connection maintenance.

2. Does the code have enough context to be clearly understood?

Yes

3. Who are the specification authors and who is accountable for this PR?

@youngjoon-lee @madxor

4. Is the specification accurate and complete?

Yes

5. Does the implementation introduce changes in the specification?

No

Checklist

Warning

Do not merge the PR if any of the following is missing:

  • 1. Description added.
  • 2. Context and links to Specification document(s) added.
  • 3. Main contact(s) (developers and specification authors) added
  • 4. Implementation and Specification are 100% in sync including changes. This is critical.
  • 5. Link PR to a specific milestone.

@youngjoon-lee youngjoon-lee added this to the Iteration 8 milestone Dec 2, 2024
@youngjoon-lee youngjoon-lee self-assigned this Dec 2, 2024
nomos-mix/core/src/conn_maintenance.rs Outdated Show resolved Hide resolved
nomos-mix/network/src/behaviour.rs Outdated Show resolved Hide resolved
nomos-mix/network/src/behaviour.rs Outdated Show resolved Hide resolved
nomos-services/mix/src/backends/libp2p.rs Outdated Show resolved Hide resolved
nomos-mix/network/src/behaviour.rs Outdated Show resolved Hide resolved
@youngjoon-lee youngjoon-lee marked this pull request as draft December 3, 2024 02:24
@youngjoon-lee youngjoon-lee marked this pull request as ready for review December 4, 2024 08:33
Copy link
Collaborator

@danielSanchezQ danielSanchezQ left a comment

Choose a reason for hiding this comment

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

i think it is much clearer now! Thanks for the extra effort!

nomos-mix/network/src/behaviour.rs Show resolved Hide resolved
@youngjoon-lee youngjoon-lee merged commit 59726d0 into master Dec 9, 2024
5 checks passed
@youngjoon-lee youngjoon-lee deleted the mix-conn-monitor branch December 9, 2024 07:31
@youngjoon-lee youngjoon-lee linked an issue Dec 16, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Connection monitoring & maintenance
2 participants