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

Use TCP instead of UDP #3

Open
dbadoy opened this issue Nov 27, 2022 · 1 comment
Open

Use TCP instead of UDP #3

dbadoy opened this issue Nov 27, 2022 · 1 comment

Comments

@dbadoy
Copy link
Member

dbadoy commented Nov 27, 2022

UDP is sufficient If the communication is simply sending a PullRequest to a random peer. However, this library's PullResponse payload size is unpredictable and will likely exceed the safe from packet fragmentation UDP size of 508 bytes(The minimum size of IPv4 datagram every device has to be able to receive(i.e. MTU) is 576 byte. And IP header(with fully option field) is 60 byte, UDP header is 8 byte. so I guess safety maximum UDP packet size from packet fragmentation is 508(576 - 60 - 8) byte).

The current implementation cuts packets at the application level and sends them over UDP to avoid packet fragmentation.
I realized there is no clear reason to use UDP When I asked myself Why use UDP and not TCP?.

All communication in the library will use TCP instead of UDP.

@dbadoy dbadoy linked a pull request Nov 28, 2022 that will close this issue
@dbadoy
Copy link
Member Author

dbadoy commented Nov 29, 2022

However, I still don't know if preventing packet fragmentation is absolutely necessary or if it has a significant performance impact. The MTU of modern routers is stable, and if a packet is lost, it is likely to be recovered on the next pull request.

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 a pull request may close this issue.

1 participant