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

[1.1.1 -> main] Revert "P2P: Fix duplication connection logic #1188

Merged
merged 3 commits into from
Feb 20, 2025

Conversation

heifner
Copy link
Member

@heifner heifner commented Feb 20, 2025

Reverts #1108

The simplified de-dup logic of #1108 allows for both sides to determine the other is duplicate if the connections happen at the same time. The previous more complicated logic does allow duplicate connections in about half the cases, but prevents the race-condition of both sides deciding the other is a duplicate.

I wonder with the new block-nack feature and future limit of connections if we should even attempt to remove duplicate connections. In one sense, it seems like if a user wants to configure both sides to connect to each other that we should just allow it. It is obviously an important connection. Also all the reconnects and determination of duplicate and disconnect is in itself wasteful.

Note the previous version support here is not really needed, but just kept this as a straight revert.

Merges release/1.1 into main including #1186

@heifner heifner added the OCI Work exclusive to OCI team label Feb 20, 2025
@heifner heifner merged commit 6264318 into main Feb 20, 2025
36 checks passed
@heifner heifner deleted the revert-GH-1108-p2p-dup-main branch February 20, 2025 19:07
@ericpassmore
Copy link
Contributor

Note:start
category: Other
component: P2P
summary: Revert simplified duplicate connection logic and revert to previous logic to prevent duplicate connections.
Note:end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCI Work exclusive to OCI team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants