Correct peer outgoing address for incomming peers#244
Merged
ikatson merged 8 commits intoikatson:mainfrom Oct 1, 2024
Merged
Conversation
ikatson
approved these changes
Sep 29, 2024
Owner
ikatson
left a comment
ikatson
left a comment
There was a problem hiding this comment.
Looks good to me, but please address the 3-state comment
Owner
|
You raise a very good point - that the "peers" map has SocketAddr as key, which doesn't make sense for incoming peers - as it promotes duplicates. Would be nice to fix it, but agree that it's a rather substantial refactor. |
Contributor
Author
|
Though not ideal still I think helps mainly in cases of "restarting" live torrent - by changing only files:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For live torrents peers are kept in DashMap with their SocketAddress as the key. Both outgoing and incoming peers are there.
For incoming peers SocketAddress key is the address of client, but it is used when retrying connection (in case of changing "only files" for instance). In these retries this address is used as peer listening address, which will not work. But we can have peer listening port and ip address from Extended Handshake, so this can be used instead, if available.
This PR is trying to implement this - only retry incoming peers if we know their listening address, otherwise keep them as not need (as we do not know where to connect them).
There is still an issue with the peers map, one peer can be there multiple times - once with outgoing listening SocketAddress and more times with client incoming SocketAddress - but to change this will require more significant change to logic - probably using Peer ID as key. So this is only partial fix, but it looks like having same peer in hash table does not cause problems.